[WebRTC] No default rtc::COW allocation in RtpPacketReceived 13/295713/6 accepted/tizen/7.0/unified/20230720.082042
authorAdam Bujalski <a.bujalski@samsung.com>
Thu, 25 May 2023 13:45:05 +0000 (15:45 +0200)
committerBot Blink <blinkbot@samsung.com>
Mon, 17 Jul 2023 12:54:51 +0000 (12:54 +0000)
When parsing RTP contents `RtpPacketReceived` is created with
rtc::CopyOnWriteBuffer with default size. Such allocation is redundant
because in this scenario soon after creation `RtpPacket::Parse()`
method is called which uses data from provided buffer.

This patch adds ctor which avoids creation of such redundant buffer.

Bug: https://cam.sprc.samsung.pl/browse/VDGAME-249
Change-Id: I2d3de6f0e5f0b01799ba5525a84457f4f7733cfa

third_party/webrtc/call/call.cc
third_party/webrtc/modules/rtp_rtcp/source/rtp_packet.cc
third_party/webrtc/modules/rtp_rtcp/source/rtp_packet_received.cc
third_party/webrtc/modules/rtp_rtcp/source/rtp_packet_received.h
third_party/webrtc/modules/rtp_rtcp/source/rtp_packet_unittest.cc
third_party/webrtc/pc/rtp_transport.cc

index 00f58d30a069b4b8a4cfcb652322ee7cf4e41009..7ac23d3b293f6b8030799127242c09157710e447 100644 (file)
@@ -1536,7 +1536,8 @@ PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type,
   TRACE_EVENT0("webrtc", "Call::DeliverRtp");
   RTC_DCHECK_NE(media_type, MediaType::ANY);
 
-  RtpPacketReceived parsed_packet;
+  RtpPacketReceived parsed_packet(nullptr, 0 /* capacity */,
+                                  webrtc::Timestamp::MinusInfinity());
   if (!parsed_packet.Parse(std::move(packet)))
     return DELIVERY_PACKET_ERROR;
 
index 8523637febc66d90a3ffd63a573afa8f2d8972c0..b7fb9195758a9e7addc8780c98b1e6c76994ba4d 100644 (file)
@@ -433,15 +433,19 @@ void RtpPacket::Clear() {
   sequence_number_ = 0;
   timestamp_ = 0;
   ssrc_ = 0;
-  payload_offset_ = kFixedHeaderSize;
   payload_size_ = 0;
   padding_size_ = 0;
   extensions_size_ = 0;
   extension_entries_.clear();
 
-  memset(WriteAt(0), 0, kFixedHeaderSize);
-  buffer_.SetSize(kFixedHeaderSize);
-  WriteAt(0, kRtpVersion << 6);
+  if (buffer_.size() > 0) {
+    payload_offset_ = kFixedHeaderSize;
+    memset(WriteAt(0), 0, kFixedHeaderSize);
+    buffer_.SetSize(kFixedHeaderSize);
+    WriteAt(0, kRtpVersion << 6);
+  } else {
+    payload_offset_ = 0;
+  }
 }
 
 bool RtpPacket::ParseBuffer(const uint8_t* buffer, size_t size) {
index 6b2cc76981a0f76136daf39a90234d1cb9e942bb..f4cf075b503f963aefaedea0e00a252c571baa95 100644 (file)
@@ -25,6 +25,10 @@ RtpPacketReceived::RtpPacketReceived(
     const ExtensionManager* extensions,
     webrtc::Timestamp arrival_time /*= webrtc::Timestamp::MinusInfinity()*/)
     : RtpPacket(extensions), arrival_time_(arrival_time) {}
+RtpPacketReceived::RtpPacketReceived(const ExtensionManager* extensions,
+                                     size_t capacity,
+                                     webrtc::Timestamp arrival_time)
+    : RtpPacket(extensions, capacity), arrival_time_(arrival_time) {}
 RtpPacketReceived::RtpPacketReceived(const RtpPacketReceived& packet) = default;
 RtpPacketReceived::RtpPacketReceived(RtpPacketReceived&& packet) = default;
 
index 431d3f52be587935bbd9fcbf28173238047aa3d0..d9a1f0920519b72c3a60ba7e0f8bc66ec5fe3a81 100644 (file)
@@ -32,6 +32,9 @@ class RtpPacketReceived : public RtpPacket {
   explicit RtpPacketReceived(
       const ExtensionManager* extensions,
       webrtc::Timestamp arrival_time = webrtc::Timestamp::MinusInfinity());
+  explicit RtpPacketReceived(const ExtensionManager* extensions,
+                             size_t capacity,
+                             webrtc::Timestamp arrival_time);
   RtpPacketReceived(const RtpPacketReceived& packet);
   RtpPacketReceived(RtpPacketReceived&& packet);
 
index 8c5df1a0ad4b975a97ec15414e4f4ad21452d49f..c9d66c748eebeeb3b65c2fdecd46d488c95761a9 100644 (file)
@@ -12,6 +12,7 @@
 #include "modules/rtp_rtcp/source/rtp_header_extensions.h"
 #include "modules/rtp_rtcp/source/rtp_packet_received.h"
 #include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
+#include "rtc_base/copy_on_write_buffer.h"
 #include "rtc_base/random.h"
 #include "test/gmock.h"
 #include "test/gtest.h"
@@ -1240,5 +1241,24 @@ TEST(RtpPacketTest, RemoveExtensionFailure) {
   EXPECT_THAT(kPacketWithTO, ElementsAreArray(packet.data(), packet.size()));
 }
 
+// Passing 0 as the capacity in ctor MUST not create COW buffer,
+// this is usefull when packet data is going to be provided via
+// RtpPacket::Parse() method.
+TEST(RtpPacketTest, EmptyCOWBuffer) {
+  RtpPacket pkt(nullptr, 0u);
+
+  rtc::CopyOnWriteBuffer buf = pkt.PayloadBuffer();
+  EXPECT_EQ(buf.capacity(), 0u);
+  EXPECT_EQ(buf.cdata(), nullptr);
+}
+
+TEST(RtpPacketTest, EmptyCOWBufferInPacketReceived) {
+  RtpPacketReceived pkt(nullptr, 0u, Timestamp::MinusInfinity());
+
+  rtc::CopyOnWriteBuffer buf = pkt.PayloadBuffer();
+  EXPECT_EQ(buf.capacity(), 0u);
+  EXPECT_EQ(buf.cdata(), nullptr);
+}
+
 }  // namespace
 }  // namespace webrtc
index d4edb9501c6c38b757e4714a29b5be26276c8af3..7eb8d2cd1c7308b542f6aa481d561488b4d986fc 100644 (file)
@@ -182,9 +182,9 @@ bool RtpTransport::UnregisterRtpDemuxerSink(RtpPacketSinkInterface* sink) {
 void RtpTransport::DemuxPacket(rtc::CopyOnWriteBuffer packet,
                                int64_t packet_time_us) {
   webrtc::RtpPacketReceived parsed_packet(
-      &header_extension_map_, packet_time_us == -1
-                                  ? Timestamp::MinusInfinity()
-                                  : Timestamp::Micros(packet_time_us));
+      &header_extension_map_, 0 /* capacity */,
+      packet_time_us == -1 ? Timestamp::MinusInfinity()
+                           : Timestamp::Micros(packet_time_us));
   if (!parsed_packet.Parse(std::move(packet))) {
     RTC_LOG(LS_ERROR)
         << "Failed to parse the incoming RTP packet before demuxing. Drop it.";