commit 18d1d0f793651cb124ccb880a220bd63b13b6af4
author Danil Chapovalov <danilchap@webrtc.org> Tue Jan 16 14:54:31 2024
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Jan 17 11:16:49 2024
tree 6726643ce987dc54aa613f587b149b2ded838fac
parent 9e4a97bb02663604b02e219b9d501a8dd91b5614

Fix perfect forwarding in RtpPacket::GetExtension

Thus allow to pass output parameter by reference.

Bug: None
Change-Id: I64821caf72875efee62d6cfc90691070dceba775
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/334644
Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41542}

modules/rtp_rtcp/source/rtp_packet.h

diff --git a/modules/rtp_rtcp/source/rtp_packet.h b/modules/rtp_rtcp/source/rtp_packet.h
index e91ec63..c002e51 100644
--- a/modules/rtp_rtcp/source/rtp_packet.h
+++ b/modules/rtp_rtcp/source/rtp_packet.h
@@ -11,6 +11,7 @@
 #define MODULES_RTP_RTCP_SOURCE_RTP_PACKET_H_
 
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "absl/types/optional.h"
@@ -127,7 +128,7 @@
   bool IsRegistered() const;
 
   template <typename Extension, typename FirstValue, typename... Values>
-  bool GetExtension(FirstValue, Values...) const;
+  bool GetExtension(FirstValue&&, Values&&...) const;
 
   template <typename Extension>
   absl::optional<typename Extension::value_type> GetExtension() const;
@@ -231,11 +232,12 @@
 }
 
 template <typename Extension, typename FirstValue, typename... Values>
-bool RtpPacket::GetExtension(FirstValue first, Values... values) const {
+bool RtpPacket::GetExtension(FirstValue&& first, Values&&... values) const {
   auto raw = FindExtension(Extension::kId);
   if (raw.empty())
     return false;
-  return Extension::Parse(raw, first, values...);
+  return Extension::Parse(raw, std::forward<FirstValue>(first),
+                          std::forward<Values>(values)...);
 }
 
 template <typename Extension>

modules/rtp_rtcp/source/rtp_packet_unittest.cc

diff --git a/modules/rtp_rtcp/source/rtp_packet_unittest.cc b/modules/rtp_rtcp/source/rtp_packet_unittest.cc
index a1d1c9d..ac46449 100644
--- a/modules/rtp_rtcp/source/rtp_packet_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_packet_unittest.cc
@@ -903,6 +903,41 @@
   EXPECT_TRUE(rtp_packet.GetExtension<UncopyableExtension>(&value2));
 }
 
+struct ParseByReferenceExtension {
+  static constexpr RTPExtensionType kId = kRtpExtensionDependencyDescriptor;
+  static constexpr absl::string_view Uri() { return "uri"; }
+
+  static size_t ValueSize(uint8_t value1, uint8_t value2) { return 2; }
+  static bool Write(rtc::ArrayView<uint8_t> data,
+                    uint8_t value1,
+                    uint8_t value2) {
+    data[0] = value1;
+    data[1] = value2;
+    return true;
+  }
+  static bool Parse(rtc::ArrayView<const uint8_t> data,
+                    uint8_t& value1,
+                    uint8_t& value2) {
+    value1 = data[0];
+    value2 = data[1];
+    return true;
+  }
+};
+
+TEST(RtpPacketTest, GetExtensionByReference) {
+  RtpHeaderExtensionMap extensions;
+  extensions.Register<ParseByReferenceExtension>(1);
+  RtpPacket rtp_packet(&extensions);
+  rtp_packet.SetExtension<ParseByReferenceExtension>(13, 42);
+
+  uint8_t value1 = 1;
+  uint8_t value2 = 1;
+  EXPECT_TRUE(
+      rtp_packet.GetExtension<ParseByReferenceExtension>(value1, value2));
+  EXPECT_EQ(int{value1}, 13);
+  EXPECT_EQ(int{value2}, 42);
+}
+
 TEST(RtpPacketTest, CreateAndParseTimingFrameExtension) {
   // Create a packet with video frame timing extension populated.
   RtpPacketToSend::ExtensionManager send_extensions;