Let PacketSource::NextPacket() return an std::unique_ptr
The return type of PacketSource::NextPacket() is changed from a naked
pointer to an std::uniqe_ptr. The interface contract was and still is
that the ownership is passed from the callee to the caller, but a
unique_ptr makes this explicit.
BUG=webrtc:2692
Review-Url: https://codereview.webrtc.org/2005873002
Cr-Commit-Position: refs/heads/master@{#12884}
diff --git a/webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.cc b/webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.cc
index dc8111d..025a0e9 100644
--- a/webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.cc
+++ b/webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.cc
@@ -154,7 +154,7 @@
void AcmReceiveTestOldApi::Run() {
for (std::unique_ptr<Packet> packet(packet_source_->NextPacket()); packet;
- packet.reset(packet_source_->NextPacket())) {
+ packet = packet_source_->NextPacket()) {
// Pull audio until time to insert packet.
while (clock_.TimeInMilliseconds() < packet->time_ms()) {
AudioFrame output_frame;
diff --git a/webrtc/modules/audio_coding/acm2/acm_send_test_oldapi.cc b/webrtc/modules/audio_coding/acm2/acm_send_test_oldapi.cc
index 3a89a77..84db491 100644
--- a/webrtc/modules/audio_coding/acm2/acm_send_test_oldapi.cc
+++ b/webrtc/modules/audio_coding/acm2/acm_send_test_oldapi.cc
@@ -73,13 +73,13 @@
return codec_registered_ = true;
}
-Packet* AcmSendTestOldApi::NextPacket() {
+std::unique_ptr<Packet> AcmSendTestOldApi::NextPacket() {
assert(codec_registered_);
if (filter_.test(static_cast<size_t>(payload_type_))) {
// This payload type should be filtered out. Since the payload type is the
// same throughout the whole test run, no packet at all will be delivered.
// We can just as well signal that the test is over by returning NULL.
- return NULL;
+ return nullptr;
}
// Insert audio and process until one packet is produced.
while (clock_.TimeInMilliseconds() < test_duration_ms_) {
@@ -101,7 +101,7 @@
}
}
// Test ended.
- return NULL;
+ return nullptr;
}
// This method receives the callback from ACM when a new packet is produced.
@@ -122,7 +122,7 @@
return 0;
}
-Packet* AcmSendTestOldApi::CreatePacket() {
+std::unique_ptr<Packet> AcmSendTestOldApi::CreatePacket() {
const size_t kRtpHeaderSize = 12;
size_t allocated_bytes = last_payload_vec_.size() + kRtpHeaderSize;
uint8_t* packet_memory = new uint8_t[allocated_bytes];
@@ -147,10 +147,10 @@
memcpy(packet_memory + kRtpHeaderSize,
&last_payload_vec_[0],
last_payload_vec_.size());
- Packet* packet =
- new Packet(packet_memory, allocated_bytes, clock_.TimeInMilliseconds());
- assert(packet);
- assert(packet->valid_header());
+ std::unique_ptr<Packet> packet(
+ new Packet(packet_memory, allocated_bytes, clock_.TimeInMilliseconds()));
+ RTC_DCHECK(packet);
+ RTC_DCHECK(packet->valid_header());
return packet;
}
diff --git a/webrtc/modules/audio_coding/acm2/acm_send_test_oldapi.h b/webrtc/modules/audio_coding/acm2/acm_send_test_oldapi.h
index 938e39e..c752878 100644
--- a/webrtc/modules/audio_coding/acm2/acm_send_test_oldapi.h
+++ b/webrtc/modules/audio_coding/acm2/acm_send_test_oldapi.h
@@ -44,10 +44,8 @@
// Registers an external send codec. Returns true on success, false otherwise.
bool RegisterExternalCodec(AudioEncoder* external_speech_encoder);
- // Returns the next encoded packet. Returns NULL if the test duration was
- // exceeded. Ownership of the packet is handed over to the caller.
// Inherited from PacketSource.
- Packet* NextPacket() override;
+ std::unique_ptr<Packet> NextPacket() override;
// Inherited from AudioPacketizationCallback.
int32_t SendData(FrameType frame_type,
@@ -63,9 +61,8 @@
static const int kBlockSizeMs = 10;
// Creates a Packet object from the last packet produced by ACM (and received
- // through the SendData method as a callback). Ownership of the new Packet
- // object is transferred to the caller.
- Packet* CreatePacket();
+ // through the SendData method as a callback).
+ std::unique_ptr<Packet> CreatePacket();
SimulatedClock clock_;
std::unique_ptr<AudioCodingModule> acm_;
diff --git a/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc b/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
index 470f690..d30a63c 100644
--- a/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
+++ b/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
@@ -1149,17 +1149,13 @@
remove(output_file_name.c_str());
}
- // Returns a pointer to the next packet. Returns NULL if the source is
- // depleted (i.e., the test duration is exceeded), or if an error occurred.
// Inherited from test::PacketSource.
- test::Packet* NextPacket() override {
- // Get the next packet from AcmSendTest. Ownership of |packet| is
- // transferred to this method.
- test::Packet* packet = send_test_->NextPacket();
+ std::unique_ptr<test::Packet> NextPacket() override {
+ auto packet = send_test_->NextPacket();
if (!packet)
return NULL;
- VerifyPacket(packet);
+ VerifyPacket(packet.get());
// TODO(henrik.lundin) Save the packet to file as well.
// Pass it on to the caller. The caller becomes the owner of |packet|.
@@ -1480,9 +1476,9 @@
ASSERT_TRUE(send_test_->acm());
send_test_->acm()->SetBitRate(target_bitrate_bps);
int nr_bytes = 0;
- while (test::Packet* next_packet = send_test_->NextPacket()) {
+ while (std::unique_ptr<test::Packet> next_packet =
+ send_test_->NextPacket()) {
nr_bytes += next_packet->payload_length_bytes();
- delete next_packet;
}
EXPECT_EQ(expected_total_bits, nr_bytes * 8);
}
@@ -1577,7 +1573,8 @@
sampling_freq_hz_ * kTestDurationMs / (frame_size_samples_ * 1000);
int nr_bytes_before = 0, nr_bytes_after = 0;
int packet_counter = 0;
- while (test::Packet* next_packet = send_test_->NextPacket()) {
+ while (std::unique_ptr<test::Packet> next_packet =
+ send_test_->NextPacket()) {
if (packet_counter == nr_packets / 2)
send_test_->acm()->SetBitRate(target_bitrate_bps);
if (packet_counter < nr_packets / 2)
@@ -1585,7 +1582,6 @@
else
nr_bytes_after += next_packet->payload_length_bytes();
packet_counter++;
- delete next_packet;
}
EXPECT_EQ(expected_before_switch_bits, nr_bytes_before * 8);
EXPECT_EQ(expected_after_switch_bits, nr_bytes_after * 8);
@@ -1733,7 +1729,7 @@
}
// Inherited from test::PacketSource.
- test::Packet* NextPacket() override {
+ std::unique_ptr<test::Packet> NextPacket() override {
// Check if it is time to terminate the test. The packet source is of type
// ConstantPcmPacketSource, which is infinite, so we must end the test
// "manually".
diff --git a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc
index 4a6f505..24a8774 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc
@@ -349,7 +349,7 @@
(output_sample_rate_ / 1000))));
}
// Get next packet.
- packet_.reset(rtp_source_->NextPacket());
+ packet_ = rtp_source_->NextPacket();
}
// Get audio from NetEq.
@@ -387,7 +387,7 @@
gen_ref ? webrtc::test::OutputPath() + "neteq_rtcp_stats.dat" : "";
ResultSink rtcp_stats(rtcp_out_file);
- packet_.reset(rtp_source_->NextPacket());
+ packet_ = rtp_source_->NextPacket();
int i = 0;
while (packet_) {
std::ostringstream ss;
diff --git a/webrtc/modules/audio_coding/neteq/tools/constant_pcm_packet_source.cc b/webrtc/modules/audio_coding/neteq/tools/constant_pcm_packet_source.cc
index 5a9f79f..fbbd468 100644
--- a/webrtc/modules/audio_coding/neteq/tools/constant_pcm_packet_source.cc
+++ b/webrtc/modules/audio_coding/neteq/tools/constant_pcm_packet_source.cc
@@ -35,7 +35,7 @@
RTC_CHECK_EQ(2U, encoded_len);
}
-Packet* ConstantPcmPacketSource::NextPacket() {
+std::unique_ptr<Packet> ConstantPcmPacketSource::NextPacket() {
RTC_CHECK_GT(packet_len_bytes_, kHeaderLenBytes);
uint8_t* packet_memory = new uint8_t[packet_len_bytes_];
// Fill the payload part of the packet memory with the pre-encoded value.
@@ -43,8 +43,8 @@
packet_memory[kHeaderLenBytes + i] = encoded_sample_[i % 2];
WriteHeader(packet_memory);
// |packet| assumes ownership of |packet_memory|.
- Packet* packet =
- new Packet(packet_memory, packet_len_bytes_, next_arrival_time_ms_);
+ std::unique_ptr<Packet> packet(
+ new Packet(packet_memory, packet_len_bytes_, next_arrival_time_ms_));
next_arrival_time_ms_ += payload_len_samples_ / samples_per_ms_;
return packet;
}
diff --git a/webrtc/modules/audio_coding/neteq/tools/constant_pcm_packet_source.h b/webrtc/modules/audio_coding/neteq/tools/constant_pcm_packet_source.h
index 6972303..d1f8877 100644
--- a/webrtc/modules/audio_coding/neteq/tools/constant_pcm_packet_source.h
+++ b/webrtc/modules/audio_coding/neteq/tools/constant_pcm_packet_source.h
@@ -31,9 +31,7 @@
int sample_rate_hz,
int payload_type);
- // Returns a pointer to the next packet. Will never return NULL. That is,
- // the source is infinite.
- Packet* NextPacket() override;
+ std::unique_ptr<Packet> NextPacket() override;
private:
void WriteHeader(uint8_t* packet_memory);
diff --git a/webrtc/modules/audio_coding/neteq/tools/neteq_rtpplay.cc b/webrtc/modules/audio_coding/neteq/tools/neteq_rtpplay.cc
index 84e866e..bee9730 100644
--- a/webrtc/modules/audio_coding/neteq/tools/neteq_rtpplay.cc
+++ b/webrtc/modules/audio_coding/neteq/tools/neteq_rtpplay.cc
@@ -493,7 +493,7 @@
replacement_audio.reset(new int16_t[input_frame_size_timestamps]);
payload_mem_size_bytes = 2 * input_frame_size_timestamps;
payload.reset(new uint8_t[payload_mem_size_bytes]);
- next_packet.reset(file_source->NextPacket());
+ next_packet = file_source->NextPacket();
assert(next_packet);
next_packet_available = true;
}
@@ -566,9 +566,9 @@
}
// Get next packet from file.
- Packet* temp_packet = file_source->NextPacket();
+ std::unique_ptr<Packet> temp_packet = file_source->NextPacket();
if (temp_packet) {
- packet.reset(temp_packet);
+ packet = std::move(temp_packet);
if (replace_payload) {
// At this point |packet| contains the packet *after* |next_packet|.
// Swap Packet objects between |packet| and |next_packet|.
@@ -586,6 +586,7 @@
next_input_time_ms = std::numeric_limits<int64_t>::max();
packet_available = false;
}
+ RTC_DCHECK(!temp_packet); // Must have transferred to another variable.
}
// Check if it is time to get output audio.
diff --git a/webrtc/modules/audio_coding/neteq/tools/packet_source.h b/webrtc/modules/audio_coding/neteq/tools/packet_source.h
index 804a94d..3ec576d 100644
--- a/webrtc/modules/audio_coding/neteq/tools/packet_source.h
+++ b/webrtc/modules/audio_coding/neteq/tools/packet_source.h
@@ -12,6 +12,7 @@
#define WEBRTC_MODULES_AUDIO_CODING_NETEQ_TOOLS_PACKET_SOURCE_H_
#include <bitset>
+#include <memory>
#include "webrtc/base/constructormagic.h"
#include "webrtc/modules/audio_coding/neteq/tools/packet.h"
@@ -26,9 +27,9 @@
PacketSource() : use_ssrc_filter_(false), ssrc_(0) {}
virtual ~PacketSource() {}
- // Returns a pointer to the next packet. Returns NULL if the source is
- // depleted, or if an error occurred.
- virtual Packet* NextPacket() = 0;
+ // Returns next packet. Returns nullptr if the source is depleted, or if an
+ // error occurred.
+ virtual std::unique_ptr<Packet> NextPacket() = 0;
virtual void FilterOutPayloadType(uint8_t payload_type) {
filter_.set(payload_type, true);
diff --git a/webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.cc b/webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.cc
index 9192839..517458b 100644
--- a/webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.cc
+++ b/webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.cc
@@ -39,7 +39,7 @@
return parser_->RegisterRtpHeaderExtension(type, id);
}
-Packet* RtcEventLogSource::NextPacket() {
+std::unique_ptr<Packet> RtcEventLogSource::NextPacket() {
while (rtp_packet_index_ < parsed_stream_.GetNumberOfEvents()) {
if (parsed_stream_.GetEventType(rtp_packet_index_) ==
ParsedRtcEventLog::RTP_EVENT) {
@@ -54,9 +54,9 @@
uint8_t* packet_header = new uint8_t[header_length];
parsed_stream_.GetRtpHeader(rtp_packet_index_, nullptr, nullptr,
packet_header, nullptr, nullptr);
- Packet* packet = new Packet(packet_header, header_length, packet_length,
- static_cast<double>(timestamp_us) / 1000,
- *parser_.get());
+ std::unique_ptr<Packet> packet(new Packet(
+ packet_header, header_length, packet_length,
+ static_cast<double>(timestamp_us) / 1000, *parser_.get()));
if (packet->valid_header()) {
// Check if the packet should not be filtered out.
if (!filter_.test(packet->header().payloadType) &&
@@ -69,9 +69,6 @@
<< " has an invalid header and will be ignored."
<< std::endl;
}
- // The packet has either an invalid header or needs to be filtered out,
- // so it can be deleted.
- delete packet;
}
}
rtp_packet_index_++;
diff --git a/webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.h b/webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.h
index ad7add1..71bf841 100644
--- a/webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.h
+++ b/webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.h
@@ -38,9 +38,7 @@
// Registers an RTP header extension and binds it to |id|.
virtual bool RegisterRtpHeaderExtension(RTPExtensionType type, uint8_t id);
- // Returns a pointer to the next packet. Returns NULL if end of file was
- // reached.
- Packet* NextPacket() override;
+ std::unique_ptr<Packet> NextPacket() override;
// Returns the timestamp of the next audio output event, in milliseconds. The
// maximum value of int64_t is returned if there are no more audio output
diff --git a/webrtc/modules/audio_coding/neteq/tools/rtp_analyze.cc b/webrtc/modules/audio_coding/neteq/tools/rtp_analyze.cc
index 0735b4c..74c64e0 100644
--- a/webrtc/modules/audio_coding/neteq/tools/rtp_analyze.cc
+++ b/webrtc/modules/audio_coding/neteq/tools/rtp_analyze.cc
@@ -107,7 +107,7 @@
int cycles = -1;
std::unique_ptr<webrtc::test::Packet> packet;
while (true) {
- packet.reset(file_source->NextPacket());
+ packet = file_source->NextPacket();
if (!packet.get()) {
// End of file reached.
break;
diff --git a/webrtc/modules/audio_coding/neteq/tools/rtp_file_source.cc b/webrtc/modules/audio_coding/neteq/tools/rtp_file_source.cc
index 039e1fa..9ca48e9 100644
--- a/webrtc/modules/audio_coding/neteq/tools/rtp_file_source.cc
+++ b/webrtc/modules/audio_coding/neteq/tools/rtp_file_source.cc
@@ -55,7 +55,7 @@
return parser_->RegisterRtpHeaderExtension(type, id);
}
-Packet* RtpFileSource::NextPacket() {
+std::unique_ptr<Packet> RtpFileSource::NextPacket() {
while (true) {
RtpPacket temp_packet;
if (!rtp_reader_->NextPacket(&temp_packet)) {
@@ -80,7 +80,7 @@
// This payload type should be filtered out. Continue to the next packet.
continue;
}
- return packet.release();
+ return packet;
}
}
diff --git a/webrtc/modules/audio_coding/neteq/tools/rtp_file_source.h b/webrtc/modules/audio_coding/neteq/tools/rtp_file_source.h
index b02e16a..1b98d6c 100644
--- a/webrtc/modules/audio_coding/neteq/tools/rtp_file_source.h
+++ b/webrtc/modules/audio_coding/neteq/tools/rtp_file_source.h
@@ -44,9 +44,7 @@
// Registers an RTP header extension and binds it to |id|.
virtual bool RegisterRtpHeaderExtension(RTPExtensionType type, uint8_t id);
- // Returns a pointer to the next packet. Returns NULL if end of file was
- // reached, or if a the data was corrupt.
- Packet* NextPacket() override;
+ std::unique_ptr<Packet> NextPacket() override;
private:
static const int kFirstLineLength = 40;