Reduce usage of RtpHeaderParser::CreateForTest in favor of RtpPacket

As a step to delete the legacy rtp packet parser.

Bug: None
Change-Id: I2aae86bc8847acd76cdd89007273a99f0298fdb9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221109
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34219}

media/BUILD.gn

@@ -622,6 +622,7 @@ if (rtc_include_tests) {
         "../modules/audio_processing:api",
         "../modules/audio_processing:mocks",
         "../modules/rtp_rtcp",
+        "../modules/rtp_rtcp:rtp_rtcp_format",
         "../modules/video_coding:simulcast_test_fixture_impl",
         "../modules/video_coding:video_codec_interface",
         "../modules/video_coding:webrtc_h264",

media/base/fake_network_interface.h

@@ -83,14 +83,12 @@ class FakeNetworkInterface : public MediaChannel::NetworkInterface,
     return static_cast<int>(sent_ssrcs_.size());
   }
 
-  // Note: callers are responsible for deleting the returned buffer.
+  rtc::CopyOnWriteBuffer GetRtpPacket(int index) RTC_LOCKS_EXCLUDED(mutex_) {
-  const rtc::CopyOnWriteBuffer* GetRtpPacket(int index)
-      RTC_LOCKS_EXCLUDED(mutex_) {
     webrtc::MutexLock lock(&mutex_);
     if (index >= static_cast<int>(rtp_packets_.size())) {
-      return NULL;
+      return {};
     }
-    return new rtc::CopyOnWriteBuffer(rtp_packets_[index]);
+    return rtp_packets_[index];
   }
 
   int NumRtcpPackets() RTC_LOCKS_EXCLUDED(mutex_) {

media/engine/webrtc_video_engine_unittest.cc

@@ -51,6 +51,7 @@
 #include "media/engine/fake_webrtc_video_engine.h"
 #include "media/engine/simulcast.h"
 #include "media/engine/webrtc_voice_engine.h"
+#include "modules/rtp_rtcp/source/rtp_packet.h"
 #include "rtc_base/arraysize.h"
 #include "rtc_base/experiments/min_video_bitrate_experiment.h"
 #include "rtc_base/fake_clock.h"
@@ -61,7 +62,6 @@
 #include "test/field_trial.h"
 #include "test/frame_forwarder.h"
 #include "test/gmock.h"
-#include "test/rtp_header_parser.h"
 
 using ::testing::_;
 using ::testing::Contains;
@@ -1659,20 +1659,13 @@ class WebRtcVideoChannelBaseTest : public ::testing::Test {
     return network_interface_.NumRtpPackets(ssrc);
   }
   int NumSentSsrcs() { return network_interface_.NumSentSsrcs(); }
-  const rtc::CopyOnWriteBuffer* GetRtpPacket(int index) {
+  rtc::CopyOnWriteBuffer GetRtpPacket(int index) {
     return network_interface_.GetRtpPacket(index);
   }
-  static int GetPayloadType(const rtc::CopyOnWriteBuffer* p) {
+  static int GetPayloadType(rtc::CopyOnWriteBuffer p) {
-    webrtc::RTPHeader header;
+    webrtc::RtpPacket header;
-    EXPECT_TRUE(ParseRtpPacket(p, &header));
+    EXPECT_TRUE(header.Parse(std::move(p)));
-    return header.payloadType;
+    return header.PayloadType();
-  }
-
-  static bool ParseRtpPacket(const rtc::CopyOnWriteBuffer* p,
-                             webrtc::RTPHeader* header) {
-    std::unique_ptr<webrtc::RtpHeaderParser> parser(
-        webrtc::RtpHeaderParser::CreateForTest());
-    return parser->Parse(p->cdata(), p->size(), header);
   }
 
   // Tests that we can send and receive frames.
@@ -1683,8 +1676,7 @@ class WebRtcVideoChannelBaseTest : public ::testing::Test {
     EXPECT_EQ(0, renderer_.num_rendered_frames());
     SendFrame();
     EXPECT_FRAME_WAIT(1, kVideoWidth, kVideoHeight, kTimeout);
-    std::unique_ptr<const rtc::CopyOnWriteBuffer> p(GetRtpPacket(0));
+    EXPECT_EQ(codec.id, GetPayloadType(GetRtpPacket(0)));
-    EXPECT_EQ(codec.id, GetPayloadType(p.get()));
   }
 
   void SendReceiveManyAndGetStats(const cricket::VideoCodec& codec,
@@ -1700,8 +1692,7 @@ class WebRtcVideoChannelBaseTest : public ::testing::Test {
         EXPECT_FRAME_WAIT(frame + i * fps, kVideoWidth, kVideoHeight, kTimeout);
       }
     }
-    std::unique_ptr<const rtc::CopyOnWriteBuffer> p(GetRtpPacket(0));
+    EXPECT_EQ(codec.id, GetPayloadType(GetRtpPacket(0)));
-    EXPECT_EQ(codec.id, GetPayloadType(p.get()));
   }
 
   cricket::VideoSenderInfo GetSenderStats(size_t i) {
@@ -2034,15 +2025,14 @@ TEST_F(WebRtcVideoChannelBaseTest, SetSendSsrc) {
   EXPECT_TRUE(SetSend(true));
   SendFrame();
   EXPECT_TRUE_WAIT(NumRtpPackets() > 0, kTimeout);
-  webrtc::RTPHeader header;
+  webrtc::RtpPacket header;
-  std::unique_ptr<const rtc::CopyOnWriteBuffer> p(GetRtpPacket(0));
+  EXPECT_TRUE(header.Parse(GetRtpPacket(0)));
-  EXPECT_TRUE(ParseRtpPacket(p.get(), &header));
+  EXPECT_EQ(kSsrc, header.Ssrc());
-  EXPECT_EQ(kSsrc, header.ssrc);
 
   // Packets are being paced out, so these can mismatch between the first and
   // second call to NumRtpPackets until pending packets are paced out.
-  EXPECT_EQ_WAIT(NumRtpPackets(), NumRtpPackets(header.ssrc), kTimeout);
+  EXPECT_EQ_WAIT(NumRtpPackets(), NumRtpPackets(header.Ssrc()), kTimeout);
-  EXPECT_EQ_WAIT(NumRtpBytes(), NumRtpBytes(header.ssrc), kTimeout);
+  EXPECT_EQ_WAIT(NumRtpBytes(), NumRtpBytes(header.Ssrc()), kTimeout);
   EXPECT_EQ(1, NumSentSsrcs());
   EXPECT_EQ(0, NumRtpPackets(kSsrc - 1));
   EXPECT_EQ(0, NumRtpBytes(kSsrc - 1));
@@ -2059,14 +2049,13 @@ TEST_F(WebRtcVideoChannelBaseTest, SetSendSsrcAfterSetCodecs) {
   EXPECT_TRUE(SetSend(true));
   EXPECT_TRUE(WaitAndSendFrame(0));
   EXPECT_TRUE_WAIT(NumRtpPackets() > 0, kTimeout);
-  webrtc::RTPHeader header;
+  webrtc::RtpPacket header;
-  std::unique_ptr<const rtc::CopyOnWriteBuffer> p(GetRtpPacket(0));
+  EXPECT_TRUE(header.Parse(GetRtpPacket(0)));
-  EXPECT_TRUE(ParseRtpPacket(p.get(), &header));
+  EXPECT_EQ(999u, header.Ssrc());
-  EXPECT_EQ(999u, header.ssrc);
   // Packets are being paced out, so these can mismatch between the first and
   // second call to NumRtpPackets until pending packets are paced out.
-  EXPECT_EQ_WAIT(NumRtpPackets(), NumRtpPackets(header.ssrc), kTimeout);
+  EXPECT_EQ_WAIT(NumRtpPackets(), NumRtpPackets(header.Ssrc()), kTimeout);
-  EXPECT_EQ_WAIT(NumRtpBytes(), NumRtpBytes(header.ssrc), kTimeout);
+  EXPECT_EQ_WAIT(NumRtpBytes(), NumRtpBytes(header.Ssrc()), kTimeout);
   EXPECT_EQ(1, NumSentSsrcs());
   EXPECT_EQ(0, NumRtpPackets(kSsrc));
   EXPECT_EQ(0, NumRtpBytes(kSsrc));
@@ -2098,12 +2087,10 @@ TEST_F(WebRtcVideoChannelBaseTest, AddRemoveSendStreams) {
   SendFrame();
   EXPECT_FRAME_WAIT(1, kVideoWidth, kVideoHeight, kTimeout);
   EXPECT_GT(NumRtpPackets(), 0);
-  webrtc::RTPHeader header;
+  webrtc::RtpPacket header;
   size_t last_packet = NumRtpPackets() - 1;
-  std::unique_ptr<const rtc::CopyOnWriteBuffer> p(
+  EXPECT_TRUE(header.Parse(GetRtpPacket(static_cast<int>(last_packet))));
-      GetRtpPacket(static_cast<int>(last_packet)));
+  EXPECT_EQ(kSsrc, header.Ssrc());
-  EXPECT_TRUE(ParseRtpPacket(p.get(), &header));
-  EXPECT_EQ(kSsrc, header.ssrc);
 
   // Remove the send stream that was added during Setup.
   EXPECT_TRUE(channel_->RemoveSendStream(kSsrc));
@@ -2118,9 +2105,8 @@ TEST_F(WebRtcVideoChannelBaseTest, AddRemoveSendStreams) {
   EXPECT_TRUE_WAIT(NumRtpPackets() > rtp_packets, kTimeout);
 
   last_packet = NumRtpPackets() - 1;
-  p.reset(GetRtpPacket(static_cast<int>(last_packet)));
+  EXPECT_TRUE(header.Parse(GetRtpPacket(static_cast<int>(last_packet))));
-  EXPECT_TRUE(ParseRtpPacket(p.get(), &header));
+  EXPECT_EQ(789u, header.Ssrc());
-  EXPECT_EQ(789u, header.ssrc);
 }
 
 // Tests the behavior of incoming streams in a conference scenario.
@@ -2148,8 +2134,7 @@ TEST_F(WebRtcVideoChannelBaseTest, SimulateConference) {
   EXPECT_FRAME_ON_RENDERER_WAIT(renderer2, 1, kVideoWidth, kVideoHeight,
                                 kTimeout);
 
-  std::unique_ptr<const rtc::CopyOnWriteBuffer> p(GetRtpPacket(0));
+  EXPECT_EQ(DefaultCodec().id, GetPayloadType(GetRtpPacket(0)));
-  EXPECT_EQ(DefaultCodec().id, GetPayloadType(p.get()));
   EXPECT_EQ(kVideoWidth, renderer1.width());
   EXPECT_EQ(kVideoHeight, renderer1.height());
   EXPECT_EQ(kVideoWidth, renderer2.width());

modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc

@@ -72,11 +72,10 @@ class SendTransport : public Transport {
   bool SendRtp(const uint8_t* data,
                size_t len,
                const PacketOptions& options) override {
-    RTPHeader header;
+    RtpPacket packet;
-    std::unique_ptr<RtpHeaderParser> parser(RtpHeaderParser::CreateForTest());
+    EXPECT_TRUE(packet.Parse(data, len));
-    EXPECT_TRUE(parser->Parse(static_cast<const uint8_t*>(data), len, &header));
     ++rtp_packets_sent_;
-    last_rtp_header_ = header;
+    last_rtp_sequence_number_ = packet.SequenceNumber();
     return true;
   }
   bool SendRtcp(const uint8_t* data, size_t len) override {
@@ -98,7 +97,7 @@ class SendTransport : public Transport {
   int64_t delay_ms_;
   int rtp_packets_sent_;
   size_t rtcp_packets_sent_;
-  RTPHeader last_rtp_header_;
+  uint16_t last_rtp_sequence_number_;
   std::vector<uint16_t> last_nack_list_;
 };
 
@@ -138,7 +137,7 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver {
   }
   int RtpSent() { return transport_.rtp_packets_sent_; }
   uint16_t LastRtpSequenceNumber() {
-    return transport_.last_rtp_header_.sequenceNumber;
+    return transport_.last_rtp_sequence_number_;
   }
   std::vector<uint16_t> LastNackListSent() {
     return transport_.last_nack_list_;

test/scenario/audio_stream.cc

@@ -185,7 +185,6 @@ ReceiveAudioStream::ReceiveAudioStream(
     recv_config.rtp.extensions = {{RtpExtension::kTransportSequenceNumberUri,
                                    kTransportSequenceNumberExtensionId}};
   }
-  receiver_->AddExtensions(recv_config.rtp.extensions);
   recv_config.decoder_factory = decoder_factory;
   recv_config.decoder_map = {
       {CallTest::kAudioSendPayloadType, {"opus", 48000, 2}}};

test/scenario/call_client.cc

@@ -17,6 +17,7 @@
 #include "api/rtc_event_log/rtc_event_log_factory.h"
 #include "api/transport/network_types.h"
 #include "modules/audio_mixer/audio_mixer_impl.h"
+#include "test/rtp_header_parser.h"
 
 namespace webrtc {
 namespace test {
@@ -213,7 +214,6 @@ CallClient::CallClient(
       clock_(time_controller->GetClock()),
       log_writer_factory_(std::move(log_writer_factory)),
       network_controller_factory_(log_writer_factory_.get(), config.transport),
-      header_parser_(RtpHeaderParser::CreateForTest()),
       task_queue_(time_controller->GetTaskQueueFactory()->CreateTaskQueue(
           "CallClient",
           TaskQueueFactory::Priority::NORMAL)) {
@@ -338,11 +338,6 @@ uint32_t CallClient::GetNextRtxSsrc() {
   return kSendRtxSsrcs[next_rtx_ssrc_index_++];
 }
 
-void CallClient::AddExtensions(std::vector<RtpExtension> extensions) {
-  for (const auto& extension : extensions)
-    header_parser_->RegisterRtpHeaderExtension(extension);
-}
-
 void CallClient::SendTask(std::function<void()> task) {
   task_queue_.SendTask(std::move(task), RTC_FROM_HERE);
 }

test/scenario/call_client.h

@@ -26,7 +26,6 @@
 #include "rtc_base/task_queue_for_test.h"
 #include "test/logging/log_writer.h"
 #include "test/network/network_emulation.h"
-#include "test/rtp_header_parser.h"
 #include "test/scenario/column_printer.h"
 #include "test/scenario/network_node.h"
 #include "test/scenario/scenario_config.h"
@@ -137,7 +136,6 @@ class CallClient : public EmulatedNetworkReceiverInterface {
   uint32_t GetNextAudioSsrc();
   uint32_t GetNextAudioLocalSsrc();
   uint32_t GetNextRtxSsrc();
-  void AddExtensions(std::vector<RtpExtension> extensions);
   int16_t Bind(EmulatedEndpoint* endpoint);
   void UnBind();
 
@@ -149,7 +147,6 @@ class CallClient : public EmulatedNetworkReceiverInterface {
   CallClientFakeAudio fake_audio_setup_;
   std::unique_ptr<Call> call_;
   std::unique_ptr<NetworkNodeTransport> transport_;
-  std::unique_ptr<RtpHeaderParser> const header_parser_;
   std::vector<std::pair<EmulatedEndpoint*, uint16_t>> endpoints_;
 
   int next_video_ssrc_index_ = 0;

video/end_to_end_tests/stats_tests.cc

@@ -17,7 +17,7 @@
 #include "api/test/video/function_video_encoder_factory.h"
 #include "call/fake_network_pipe.h"
 #include "call/simulated_network.h"
-#include "modules/rtp_rtcp/source/rtp_utility.h"
+#include "modules/rtp_rtcp/source/rtp_packet.h"
 #include "modules/video_coding/include/video_coding_defines.h"
 #include "rtc_base/strings/string_builder.h"
 #include "rtc_base/synchronization/mutex.h"
@@ -71,12 +71,11 @@ TEST_F(StatsEndToEndTest, GetStats) {
     Action OnSendRtp(const uint8_t* packet, size_t length) override {
       // Drop every 25th packet => 4% loss.
       static const int kPacketLossFrac = 25;
-      RTPHeader header;
+      RtpPacket header;
-      RtpUtility::RtpHeaderParser parser(packet, length);
+      if (header.Parse(packet, length) &&
-      if (parser.Parse(&header) &&
+          expected_send_ssrcs_.find(header.Ssrc()) !=
-          expected_send_ssrcs_.find(header.ssrc) !=
               expected_send_ssrcs_.end() &&
-          header.sequenceNumber % kPacketLossFrac == 0) {
+          header.SequenceNumber() % kPacketLossFrac == 0) {
         return DROP_PACKET;
       }
       check_stats_event_.Set();
@@ -613,11 +612,9 @@ TEST_F(StatsEndToEndTest, VerifyNackStats) {
     Action OnSendRtp(const uint8_t* packet, size_t length) override {
       MutexLock lock(&mutex_);
       if (++sent_rtp_packets_ == kPacketNumberToDrop) {
-        std::unique_ptr<RtpHeaderParser> parser(
+        RtpPacket header;
-            RtpHeaderParser::CreateForTest());
+        EXPECT_TRUE(header.Parse(packet, length));
-        RTPHeader header;
+        dropped_rtp_packet_ = header.SequenceNumber();
-        EXPECT_TRUE(parser->Parse(packet, length, &header));
-        dropped_rtp_packet_ = header.sequenceNumber;
         return DROP_PACKET;
       }
       task_queue_->PostTask(std::unique_ptr<QueuedTask>(this));