From 98ed5409a8e50cd0f546683ce609cf4c3dd1eb4b Mon Sep 17 00:00:00 2001
From: philipel <philipel@webrtc.org>
Date: Tue, 21 May 2024 15:59:43 +0200
Subject: [PATCH] Change WebRTC-Video-SimulcastIndependentFrameIds to a
 kill-switch.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Bug: b/329063481
Change-Id: I737702424bc3c359edf2f44c4f299e507db69aa0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/351141
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42370}
---
 call/rtp_video_sender.cc          |  2 +-
 call/rtp_video_sender_unittest.cc | 62 ++++++++++++++++++++++++++++++-
 2 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 47e422ebfc..7e0258ae77 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -401,7 +401,7 @@ RtpVideoSender::RtpVideoSender(
       codec_type_(GetVideoCodecType(rtp_config)),
       transport_(transport),
       independent_frame_ids_(
-          field_trials_.IsEnabled(
+          !field_trials_.IsDisabled(
               "WebRTC-Video-SimulcastIndependentFrameIds") &&
           field_trials_.IsDisabled("WebRTC-GenericDescriptorAuth")),
       transport_overhead_bytes_per_packet_(0),
diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc
index 549f17f85d..6cad9c5f50 100644
--- a/call/rtp_video_sender_unittest.cc
+++ b/call/rtp_video_sender_unittest.cc
@@ -739,7 +739,6 @@ TEST(RtpVideoSenderTest, SupportsDependencyDescriptor) {
 
 TEST(RtpVideoSenderTest, SimulcastIndependentFrameIds) {
   test::ExplicitKeyValueConfig field_trials(
-      "WebRTC-Video-SimulcastIndependentFrameIds/Enabled/"
       "WebRTC-GenericDescriptorAuth/Disabled/");
   const std::map<uint32_t, RtpPayloadState> kPayloadStates = {
       {kSsrc1, {.frame_id = 100}}, {kSsrc2, {.frame_id = 200}}};
@@ -799,7 +798,6 @@ TEST(RtpVideoSenderTest, SimulcastIndependentFrameIds) {
 TEST(RtpVideoSenderTest,
      SimulcastNoIndependentFrameIdsIfGenericDescriptorAuthIsEnabled) {
   test::ExplicitKeyValueConfig field_trials(
-      "WebRTC-Video-SimulcastIndependentFrameIds/Enabled/"
       "WebRTC-GenericDescriptorAuth/Enabled/");
   const std::map<uint32_t, RtpPayloadState> kPayloadStates = {
       {kSsrc1, {.shared_frame_id = 1000, .frame_id = 100}},
@@ -857,6 +855,66 @@ TEST(RtpVideoSenderTest,
   EXPECT_EQ(dd_s1.frame_number(), 1002);
 }
 
+TEST(RtpVideoSenderTest,
+     SimulcastNoIndependentFrameIdsIfIndependentFrameIdsDisabled) {
+  test::ExplicitKeyValueConfig field_trials(
+      "WebRTC-Video-SimulcastIndependentFrameIds/Disabled/");
+  const std::map<uint32_t, RtpPayloadState> kPayloadStates = {
+      {kSsrc1, {.shared_frame_id = 1000, .frame_id = 100}},
+      {kSsrc2, {.shared_frame_id = 1000, .frame_id = 200}}};
+  RtpVideoSenderTestFixture test({kSsrc1, kSsrc2}, {}, kPayloadType,
+                                 kPayloadStates, &field_trials);
+  test.SetSending(true);
+
+  RtpHeaderExtensionMap extensions;
+  extensions.Register<RtpDependencyDescriptorExtension>(
+      kDependencyDescriptorExtensionId);
+  std::vector<RtpPacket> sent_packets;
+  ON_CALL(test.transport(), SendRtp)
+      .WillByDefault([&](rtc::ArrayView<const uint8_t> packet,
+                         const PacketOptions& options) {
+        sent_packets.emplace_back(&extensions);
+        EXPECT_TRUE(sent_packets.back().Parse(packet));
+        return true;
+      });
+
+  const uint8_t kPayload[1] = {'a'};
+  EncodedImage encoded_image;
+  encoded_image.SetEncodedData(
+      EncodedImageBuffer::Create(kPayload, sizeof(kPayload)));
+
+  CodecSpecificInfo codec_specific;
+  codec_specific.codecType = VideoCodecType::kVideoCodecGeneric;
+  codec_specific.template_structure.emplace();
+  codec_specific.template_structure->num_decode_targets = 1;
+  codec_specific.template_structure->templates = {
+      FrameDependencyTemplate().T(0).Dtis("S"),
+      FrameDependencyTemplate().T(0).Dtis("S").FrameDiffs({1}),
+  };
+  codec_specific.generic_frame_info =
+      GenericFrameInfo::Builder().T(0).Dtis("S").Build();
+  encoded_image._frameType = VideoFrameType::kVideoFrameKey;
+  codec_specific.generic_frame_info->encoder_buffers = {{0, false, true}};
+
+  encoded_image.SetSimulcastIndex(0);
+  EXPECT_EQ(test.router()->OnEncodedImage(encoded_image, &codec_specific).error,
+            EncodedImageCallback::Result::OK);
+  encoded_image.SetSimulcastIndex(1);
+  EXPECT_EQ(test.router()->OnEncodedImage(encoded_image, &codec_specific).error,
+            EncodedImageCallback::Result::OK);
+
+  test.AdvanceTime(TimeDelta::Millis(33));
+  ASSERT_THAT(sent_packets, SizeIs(2));
+  DependencyDescriptorMandatory dd_s0;
+  DependencyDescriptorMandatory dd_s1;
+  ASSERT_TRUE(
+      sent_packets[0].GetExtension<RtpDependencyDescriptorExtension>(&dd_s0));
+  ASSERT_TRUE(
+      sent_packets[1].GetExtension<RtpDependencyDescriptorExtension>(&dd_s1));
+  EXPECT_EQ(dd_s0.frame_number(), 1001);
+  EXPECT_EQ(dd_s1.frame_number(), 1002);
+}
+
 TEST(RtpVideoSenderTest,
      SupportsDependencyDescriptorForVp8NotProvidedByEncoder) {
   constexpr uint8_t kPayload[1] = {'a'};