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}

webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.cc

@@ -154,7 +154,7 @@ int AcmReceiveTestOldApi::RegisterExternalReceiveCodec(
 
 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;

webrtc/modules/audio_coding/acm2/acm_send_test_oldapi.cc

@@ -73,13 +73,13 @@ bool AcmSendTestOldApi::RegisterExternalCodec(
   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 @@ Packet* AcmSendTestOldApi::NextPacket() {
     }
   }
   // Test ended.
-  return NULL;
+  return nullptr;
 }
 
 // This method receives the callback from ACM when a new packet is produced.
@@ -122,7 +122,7 @@ int32_t AcmSendTestOldApi::SendData(
   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 @@ Packet* AcmSendTestOldApi::CreatePacket() {
   memcpy(packet_memory + kRtpHeaderSize,
          &last_payload_vec_[0],
          last_payload_vec_.size());
-  Packet* packet =
+  std::unique_ptr<Packet> packet(
-      new Packet(packet_memory, allocated_bytes, clock_.TimeInMilliseconds());
+      new Packet(packet_memory, allocated_bytes, clock_.TimeInMilliseconds()));
-  assert(packet);
+  RTC_DCHECK(packet);
-  assert(packet->valid_header());
+  RTC_DCHECK(packet->valid_header());
   return packet;
 }
 

webrtc/modules/audio_coding/acm2/acm_send_test_oldapi.h

@@ -44,10 +44,8 @@ class AcmSendTestOldApi : public AudioPacketizationCallback,
   // 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 @@ class AcmSendTestOldApi : public AudioPacketizationCallback,
   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
+  // through the SendData method as a callback).
-  // object is transferred to the caller.
+  std::unique_ptr<Packet> CreatePacket();
-  Packet* CreatePacket();
 
   SimulatedClock clock_;
   std::unique_ptr<AudioCodingModule> acm_;

webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc

@@ -1149,17 +1149,13 @@ class AcmSenderBitExactnessOldApi : public ::testing::Test,
     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 {
+  std::unique_ptr<test::Packet> NextPacket() override {
-    // Get the next packet from AcmSendTest. Ownership of |packet| is
+    auto packet = send_test_->NextPacket();
-    // transferred to this method.
-    test::Packet* 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 @@ class AcmSetBitRateOldApi : public ::testing::Test {
     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 @@ class AcmChangeBitRateOldApi : public AcmSetBitRateOldApi {
         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 @@ class AcmChangeBitRateOldApi : public AcmSetBitRateOldApi {
       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 @@ class AcmSwitchingOutputFrequencyOldApi : public ::testing::Test,
   }
 
   // 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".

webrtc/modules/audio_coding/neteq/neteq_unittest.cc

@@ -349,7 +349,7 @@ void NetEqDecodingTest::Process() {
                                              (output_sample_rate_ / 1000))));
     }
     // Get next packet.
-    packet_.reset(rtp_source_->NextPacket());
+    packet_ = rtp_source_->NextPacket();
   }
 
   // Get audio from NetEq.
@@ -387,7 +387,7 @@ void NetEqDecodingTest::DecodeAndCompare(
       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;

webrtc/modules/audio_coding/neteq/tools/constant_pcm_packet_source.cc

@@ -35,7 +35,7 @@ ConstantPcmPacketSource::ConstantPcmPacketSource(size_t payload_len_samples,
   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* ConstantPcmPacketSource::NextPacket() {
     packet_memory[kHeaderLenBytes + i] = encoded_sample_[i % 2];
   WriteHeader(packet_memory);
   // |packet| assumes ownership of |packet_memory|.
-  Packet* packet =
+  std::unique_ptr<Packet> packet(
-      new Packet(packet_memory, packet_len_bytes_, next_arrival_time_ms_);
+      new Packet(packet_memory, packet_len_bytes_, next_arrival_time_ms_));
   next_arrival_time_ms_ += payload_len_samples_ / samples_per_ms_;
   return packet;
 }

webrtc/modules/audio_coding/neteq/tools/constant_pcm_packet_source.h

@@ -31,9 +31,7 @@ class ConstantPcmPacketSource : public PacketSource {
                           int sample_rate_hz,
                           int payload_type);
 
-  // Returns a pointer to the next packet. Will never return NULL. That is,
+  std::unique_ptr<Packet> NextPacket() override;
-  // the source is infinite.
-  Packet* NextPacket() override;
 
  private:
   void WriteHeader(uint8_t* packet_memory);

webrtc/modules/audio_coding/neteq/tools/neteq_rtpplay.cc

@@ -493,7 +493,7 @@ int main(int argc, char* argv[]) {
     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 @@ int main(int argc, char* argv[]) {
       }
 
       // 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 @@ int main(int argc, char* argv[]) {
         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.

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 @@ class PacketSource {
   PacketSource() : use_ssrc_filter_(false), ssrc_(0) {}
   virtual ~PacketSource() {}
 
-  // Returns a pointer to the next packet. Returns NULL if the source is
+  // Returns next packet. Returns nullptr if the source is depleted, or if an
-  // depleted, or if an error occurred.
+  // error occurred.
-  virtual Packet* NextPacket() = 0;
+  virtual std::unique_ptr<Packet> NextPacket() = 0;
 
   virtual void FilterOutPayloadType(uint8_t payload_type) {
     filter_.set(payload_type, true);

webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.cc

@@ -39,7 +39,7 @@ bool RtcEventLogSource::RegisterRtpHeaderExtension(RTPExtensionType type,
   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 @@ Packet* RtcEventLogSource::NextPacket() {
         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,
+        std::unique_ptr<Packet> packet(new Packet(
-                                    static_cast<double>(timestamp_us) / 1000,
+            packet_header, header_length, packet_length,
-                                    *parser_.get());
+            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 @@ Packet* RtcEventLogSource::NextPacket() {
                     << " 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_++;

webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.h

@@ -38,9 +38,7 @@ class RtcEventLogSource : public PacketSource {
   // 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
+  std::unique_ptr<Packet> NextPacket() override;
-  // reached.
-  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

webrtc/modules/audio_coding/neteq/tools/rtp_analyze.cc

@@ -107,7 +107,7 @@ int main(int argc, char* argv[]) {
   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;

webrtc/modules/audio_coding/neteq/tools/rtp_file_source.cc

@@ -55,7 +55,7 @@ bool RtpFileSource::RegisterRtpHeaderExtension(RTPExtensionType type,
   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 @@ Packet* RtpFileSource::NextPacket() {
       // This payload type should be filtered out. Continue to the next packet.
       continue;
     }
-    return packet.release();
+    return packet;
   }
 }
 

webrtc/modules/audio_coding/neteq/tools/rtp_file_source.h

@@ -44,9 +44,7 @@ class RtpFileSource : public PacketSource {
   // 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
+  std::unique_ptr<Packet> NextPacket() override;
-  // reached, or if a the data was corrupt.
-  Packet* NextPacket() override;
 
  private:
   static const int kFirstLineLength = 40;