Deliver packet to Call as rtc::CopyOnWriteBuffer

instead of pair of pointer + size. it removes hidden memcpy in RtpPacketReceived::Parse: RtpPacketReceived keeps a reference to a CopyOnWriteBuffer. By passing it the same CopyOnWriteBuffer that was created by BaseChannel, one allocation and memcpy is avoided. Bug: None Change-Id: I5f89f478b380fc9aece3762d3a04f228d48598f5 Reviewed-on: https://webrtc-review.googlesource.com/23761 Commit-Queue: Danil Chapovalov <[email protected]> Reviewed-by: Stefan Holmer <[email protected]> Reviewed-by: Niels Moller <[email protected]> Cr-Commit-Position: refs/heads/master@{#21143}
czs100 · Dec 7, 2017 · 292a73e · 292a73e
1 parent 4c34f43
commit 292a73e
Show file tree

Hide file tree

Showing 11 changed files with 57 additions and 68 deletions.
diff --git a/call/call.cc b/call/call.cc
@@ -201,8 +201,7 @@ class Call : public webrtc::Call,
 
   // Implements PacketReceiver.
   DeliveryStatus DeliverPacket(MediaType media_type,
-                               const uint8_t* packet,
-                               size_t length,
+                               rtc::CopyOnWriteBuffer packet,
                                const PacketTime& packet_time) override;
 
   // Implements RecoveredPacketReceiver.
@@ -242,8 +241,7 @@ class Call : public webrtc::Call,
   DeliveryStatus DeliverRtcp(MediaType media_type, const uint8_t* packet,
                              size_t length);
   DeliveryStatus DeliverRtp(MediaType media_type,
-                            const uint8_t* packet,
-                            size_t length,
+                            rtc::CopyOnWriteBuffer packet,
                             const PacketTime& packet_time);
   void ConfigureSync(const std::string& sync_group)
       RTC_EXCLUSIVE_LOCKS_REQUIRED(receive_crit_);
@@ -1323,13 +1321,13 @@ PacketReceiver::DeliveryStatus Call::DeliverRtcp(MediaType media_type,
 }
 
 PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type,
-                                                const uint8_t* packet,
-                                                size_t length,
+                                                rtc::CopyOnWriteBuffer packet,
                                                 const PacketTime& packet_time) {
+  int length = packet.size();
   TRACE_EVENT0("webrtc", "Call::DeliverRtp");
 
   RtpPacketReceived parsed_packet;
-  if (!parsed_packet.Parse(packet, length))
+  if (!parsed_packet.Parse(std::move(packet)))
     return DELIVERY_PACKET_ERROR;
 
   if (packet_time.timestamp != -1) {
@@ -1365,8 +1363,8 @@ PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type,
 
   if (media_type == MediaType::AUDIO) {
     if (audio_receiver_controller_.OnRtpPacket(parsed_packet)) {
-      received_bytes_per_second_counter_.Add(static_cast<int>(length));
-      received_audio_bytes_per_second_counter_.Add(static_cast<int>(length));
+      received_bytes_per_second_counter_.Add(length);
+      received_audio_bytes_per_second_counter_.Add(length);
       event_log_->Log(
           rtc::MakeUnique<RtcEventRtpPacketIncoming>(parsed_packet));
       const int64_t arrival_time_ms = parsed_packet.arrival_time_ms();
@@ -1378,8 +1376,8 @@ PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type,
     }
   } else if (media_type == MediaType::VIDEO) {
     if (video_receiver_controller_.OnRtpPacket(parsed_packet)) {
-      received_bytes_per_second_counter_.Add(static_cast<int>(length));
-      received_video_bytes_per_second_counter_.Add(static_cast<int>(length));
+      received_bytes_per_second_counter_.Add(length);
+      received_video_bytes_per_second_counter_.Add(length);
       event_log_->Log(
           rtc::MakeUnique<RtcEventRtpPacketIncoming>(parsed_packet));
       const int64_t arrival_time_ms = parsed_packet.arrival_time_ms();
@@ -1395,14 +1393,13 @@ PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type,
 
 PacketReceiver::DeliveryStatus Call::DeliverPacket(
     MediaType media_type,
-    const uint8_t* packet,
-    size_t length,
+    rtc::CopyOnWriteBuffer packet,
     const PacketTime& packet_time) {
   RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_);
-  if (RtpHeaderParser::IsRtcp(packet, length))
-    return DeliverRtcp(media_type, packet, length);
+  if (RtpHeaderParser::IsRtcp(packet.cdata(), packet.size()))
+    return DeliverRtcp(media_type, packet.cdata(), packet.size());
 
-  return DeliverRtp(media_type, packet, length, packet_time);
+  return DeliverRtp(media_type, std::move(packet), packet_time);
 }
 
 void Call::OnRecoveredPacket(const uint8_t* packet, size_t length) {

diff --git a/call/call.h b/call/call.h
@@ -25,6 +25,7 @@
 #include "call/video_send_stream.h"
 #include "common_types.h"  // NOLINT(build/include)
 #include "rtc_base/bitrateallocationstrategy.h"
+#include "rtc_base/copyonwritebuffer.h"
 #include "rtc_base/networkroute.h"
 #include "rtc_base/platform_file.h"
 #include "rtc_base/socket.h"
@@ -63,8 +64,7 @@ class PacketReceiver {
   };
 
   virtual DeliveryStatus DeliverPacket(MediaType media_type,
-                                       const uint8_t* packet,
-                                       size_t length,
+                                       rtc::CopyOnWriteBuffer packet,
                                        const PacketTime& packet_time) = 0;
 
  protected:

diff --git a/media/engine/fakewebrtccall.cc b/media/engine/fakewebrtccall.cc
@@ -546,15 +546,14 @@ webrtc::PacketReceiver* FakeCall::Receiver() {
 
 FakeCall::DeliveryStatus FakeCall::DeliverPacket(
     webrtc::MediaType media_type,
-    const uint8_t* packet,
-    size_t length,
+    rtc::CopyOnWriteBuffer packet,
     const webrtc::PacketTime& packet_time) {
-  EXPECT_GE(length, 12u);
+  EXPECT_GE(packet.size(), 12u);
   RTC_DCHECK(media_type == webrtc::MediaType::AUDIO ||
              media_type == webrtc::MediaType::VIDEO);
 
   uint32_t ssrc;
-  if (!GetRtpSsrc(packet, length, &ssrc))
+  if (!GetRtpSsrc(packet.cdata(), packet.size(), &ssrc))
     return DELIVERY_PACKET_ERROR;
 
   if (media_type == webrtc::MediaType::VIDEO) {
@@ -566,7 +565,7 @@ FakeCall::DeliveryStatus FakeCall::DeliverPacket(
   if (media_type == webrtc::MediaType::AUDIO) {
     for (auto receiver : audio_receive_streams_) {
       if (receiver->GetConfig().rtp.remote_ssrc == ssrc) {
-        receiver->DeliverRtp(packet, length, packet_time);
+        receiver->DeliverRtp(packet.cdata(), packet.size(), packet_time);
         return DELIVERY_OK;
       }
     }

diff --git a/media/engine/fakewebrtccall.h b/media/engine/fakewebrtccall.h
@@ -288,8 +288,7 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver {
   webrtc::PacketReceiver* Receiver() override;
 
   DeliveryStatus DeliverPacket(webrtc::MediaType media_type,
-                               const uint8_t* packet,
-                               size_t length,
+                               rtc::CopyOnWriteBuffer packet,
                                const webrtc::PacketTime& packet_time) override;
 
   webrtc::Call::Stats GetStats() const override;

diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc
@@ -1396,10 +1396,8 @@ void WebRtcVideoChannel::OnPacketReceived(
   const webrtc::PacketTime webrtc_packet_time(packet_time.timestamp,
                                               packet_time.not_before);
   const webrtc::PacketReceiver::DeliveryStatus delivery_result =
-      call_->Receiver()->DeliverPacket(
-          webrtc::MediaType::VIDEO,
-          packet->cdata(), packet->size(),
-          webrtc_packet_time);
+      call_->Receiver()->DeliverPacket(webrtc::MediaType::VIDEO, *packet,
+                                       webrtc_packet_time);
   switch (delivery_result) {
     case webrtc::PacketReceiver::DELIVERY_OK:
       return;
@@ -1442,10 +1440,9 @@ void WebRtcVideoChannel::OnPacketReceived(
       break;
   }
 
-  if (call_->Receiver()->DeliverPacket(
-          webrtc::MediaType::VIDEO,
-          packet->cdata(), packet->size(),
-          webrtc_packet_time) != webrtc::PacketReceiver::DELIVERY_OK) {
+  if (call_->Receiver()->DeliverPacket(webrtc::MediaType::VIDEO, *packet,
+                                       webrtc_packet_time) !=
+      webrtc::PacketReceiver::DELIVERY_OK) {
     RTC_LOG(LS_WARNING) << "Failed to deliver RTP packet on re-delivery.";
     return;
   }
@@ -1460,10 +1457,8 @@ void WebRtcVideoChannel::OnRtcpReceived(
   // for both audio and video on the same path. Since BundleFilter doesn't
   // filter RTCP anymore incoming RTCP packets could've been going to audio (so
   // logging failures spam the log).
-  call_->Receiver()->DeliverPacket(
-      webrtc::MediaType::VIDEO,
-      packet->cdata(), packet->size(),
-      webrtc_packet_time);
+  call_->Receiver()->DeliverPacket(webrtc::MediaType::VIDEO, *packet,
+                                   webrtc_packet_time);
 }
 
 void WebRtcVideoChannel::OnReadyToSend(bool ready) {

diff --git a/media/engine/webrtcvoiceengine.cc b/media/engine/webrtcvoiceengine.cc
@@ -2067,8 +2067,7 @@ void WebRtcVoiceMediaChannel::OnPacketReceived(
   const webrtc::PacketTime webrtc_packet_time(packet_time.timestamp,
                                               packet_time.not_before);
   webrtc::PacketReceiver::DeliveryStatus delivery_result =
-      call_->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO,
-                                       packet->cdata(), packet->size(),
+      call_->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO, *packet,
                                        webrtc_packet_time);
   if (delivery_result != webrtc::PacketReceiver::DELIVERY_UNKNOWN_SSRC) {
     return;
@@ -2121,10 +2120,8 @@ void WebRtcVoiceMediaChannel::OnPacketReceived(
     SetRawAudioSink(ssrc, std::move(proxy_sink));
   }
 
-  delivery_result = call_->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO,
-                                                     packet->cdata(),
-                                                     packet->size(),
-                                                     webrtc_packet_time);
+  delivery_result = call_->Receiver()->DeliverPacket(
+      webrtc::MediaType::AUDIO, *packet, webrtc_packet_time);
   RTC_DCHECK_NE(webrtc::PacketReceiver::DELIVERY_UNKNOWN_SSRC, delivery_result);
 }
 
@@ -2135,8 +2132,8 @@ void WebRtcVoiceMediaChannel::OnRtcpReceived(
   // Forward packet to Call as well.
   const webrtc::PacketTime webrtc_packet_time(packet_time.timestamp,
                                               packet_time.not_before);
-  call_->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO,
-      packet->cdata(), packet->size(), webrtc_packet_time);
+  call_->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO, *packet,
+                                   webrtc_packet_time);
 }
 
 void WebRtcVoiceMediaChannel::OnNetworkRouteChanged(

diff --git a/test/fake_network_pipe.cc b/test/fake_network_pipe.cc
@@ -53,8 +53,9 @@ void DemuxerImpl::DeliverPacket(const NetworkPacket* packet,
         << "payload type " << static_cast<int>(payload_type) << " unknown.";
     media_type = it->second;
   }
-  packet_receiver_->DeliverPacket(media_type, packet_data, packet_length,
-                                  packet_time);
+  packet_receiver_->DeliverPacket(
+      media_type, rtc::CopyOnWriteBuffer(packet_data, packet_length),
+      packet_time);
 }
 
 FakeNetworkPipe::FakeNetworkPipe(Clock* clock,

diff --git a/test/fake_network_pipe_unittest.cc b/test/fake_network_pipe_unittest.cc
@@ -50,9 +50,10 @@ class ReorderTestDemuxer : public TestDemuxer {
 
 class MockReceiver : public PacketReceiver {
  public:
-  MOCK_METHOD4(
-      DeliverPacket,
-      DeliveryStatus(MediaType, const uint8_t*, size_t, const PacketTime&));
+  MOCK_METHOD3(DeliverPacket,
+               DeliveryStatus(MediaType,
+                              rtc::CopyOnWriteBuffer,
+                              const PacketTime&));
 };
 
 class FakeNetworkPipeTest : public ::testing::Test {
@@ -430,14 +431,14 @@ TEST(DemuxerImplTest, Demuxing) {
   data[1] = kVideoPayloadType;
   std::unique_ptr<NetworkPacket> packet(
       new NetworkPacket(&data[0], kPacketSize, kTimeNow, kArrivalTime));
-  EXPECT_CALL(mock_receiver, DeliverPacket(MediaType::VIDEO, _, _, _))
+  EXPECT_CALL(mock_receiver, DeliverPacket(MediaType::VIDEO, _, _))
       .WillOnce(Return(PacketReceiver::DELIVERY_OK));
   demuxer.DeliverPacket(packet.get(), PacketTime());
 
   data[1] = kAudioPayloadType;
   packet.reset(
       new NetworkPacket(&data[0], kPacketSize, kTimeNow, kArrivalTime));
-  EXPECT_CALL(mock_receiver, DeliverPacket(MediaType::AUDIO, _, _, _))
+  EXPECT_CALL(mock_receiver, DeliverPacket(MediaType::AUDIO, _, _))
       .WillOnce(Return(PacketReceiver::DELIVERY_OK));
   demuxer.DeliverPacket(packet.get(), PacketTime());
 }

diff --git a/video/end_to_end_tests.cc b/video/end_to_end_tests.cc
@@ -1343,15 +1343,14 @@ TEST_P(EndToEndTest, UnknownRtpPacketGivesUnknownSsrcReturnCode) {
 
    private:
     DeliveryStatus DeliverPacket(MediaType media_type,
-                                 const uint8_t* packet,
-                                 size_t length,
+                                 rtc::CopyOnWriteBuffer packet,
                                  const PacketTime& packet_time) override {
-      if (RtpHeaderParser::IsRtcp(packet, length)) {
-        return receiver_->DeliverPacket(media_type, packet, length,
+      if (RtpHeaderParser::IsRtcp(packet.cdata(), packet.size())) {
+        return receiver_->DeliverPacket(media_type, std::move(packet),
                                         packet_time);
       } else {
-        DeliveryStatus delivery_status =
-            receiver_->DeliverPacket(media_type, packet, length, packet_time);
+        DeliveryStatus delivery_status = receiver_->DeliverPacket(
+            media_type, std::move(packet), packet_time);
         EXPECT_EQ(DELIVERY_UNKNOWN_SSRC, delivery_status);
         delivered_packet_.Set();
         return delivery_status;

diff --git a/video/replay.cc b/video/replay.cc
@@ -298,7 +298,8 @@ void RtpReplay() {
       break;
     ++num_packets;
     switch (call->Receiver()->DeliverPacket(
-        webrtc::MediaType::VIDEO, packet.data, packet.length, PacketTime())) {
+        webrtc::MediaType::VIDEO,
+        rtc::CopyOnWriteBuffer(packet.data, packet.length), PacketTime())) {
       case PacketReceiver::DELIVERY_OK:
         break;
       case PacketReceiver::DELIVERY_UNKNOWN_SSRC: {

diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc
@@ -280,25 +280,25 @@ class VideoAnalyzer : public PacketReceiver,
   }
 
   DeliveryStatus DeliverPacket(MediaType media_type,
-                               const uint8_t* packet,
-                               size_t length,
+                               rtc::CopyOnWriteBuffer packet,
                                const PacketTime& packet_time) override {
     // Ignore timestamps of RTCP packets. They're not synchronized with
     // RTP packet timestamps and so they would confuse wrap_handler_.
-    if (RtpHeaderParser::IsRtcp(packet, length)) {
-      return receiver_->DeliverPacket(media_type, packet, length, packet_time);
+    if (RtpHeaderParser::IsRtcp(packet.cdata(), packet.size())) {
+      return receiver_->DeliverPacket(media_type, std::move(packet),
+                                      packet_time);
     }
 
     if (rtp_file_writer_) {
       test::RtpPacket p;
-      memcpy(p.data, packet, length);
-      p.length = length;
-      p.original_length = length;
+      memcpy(p.data, packet.cdata(), packet.size());
+      p.length = packet.size();
+      p.original_length = packet.size();
       p.time_ms = clock_->TimeInMilliseconds() - start_ms_;
       rtp_file_writer_->WritePacket(&p);
     }
 
-    RtpUtility::RtpHeaderParser parser(packet, length);
+    RtpUtility::RtpHeaderParser parser(packet.cdata(), packet.size());
     RTPHeader header;
     parser.Parse(&header);
     if (!IsFlexfec(header.payloadType) &&
@@ -315,7 +315,7 @@ class VideoAnalyzer : public PacketReceiver,
           Clock::GetRealTimeClock()->CurrentNtpInMilliseconds();
     }
 
-    return receiver_->DeliverPacket(media_type, packet, length, packet_time);
+    return receiver_->DeliverPacket(media_type, std::move(packet), packet_time);
   }
 
   void MeasuredEncodeTiming(int64_t ntp_time_ms, int encode_time_ms) {