commit a8da642371246488b76ca485eae35e8a71e897ce
author Erik Språng <sprang@webrtc.org> Tue May 14 16:55:34 2024
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed May 15 13:12:08 2024
tree 9899194a4e4905cea0002f4209442da77cdd3c70
parent 7db2a8569767bd9cf4fffa7dca93ebf7fc31ece8

Reconfigure encoders or source change

This is done in case the new source has a different clock source which
might be out of sync with the previous one, leading to timestamp
conflicts.

Bug: b/340198307
Change-Id: Id1a82823ca85e03d99dad31b7ce9f19f09447755
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/350560
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42312}

media/engine/webrtc_video_engine.cc

diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 7a10bf1..e985d29 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -1786,6 +1786,7 @@
   TRACE_EVENT0("webrtc", "WebRtcVideoSendStream::SetVideoSend");
   RTC_DCHECK_RUN_ON(&thread_checker_);
 
+  bool reconfiguration_needed = false;
   if (options) {
     VideoOptions old_options = parameters_.options;
     parameters_.options.SetAll(*options);
@@ -1801,13 +1802,21 @@
       old_options.is_screencast = options->is_screencast;
     }
     if (parameters_.options != old_options) {
-      ReconfigureEncoder(nullptr);
+      reconfiguration_needed = true;
     }
   }
 
   if (source_ && stream_) {
     stream_->SetSource(nullptr, webrtc::DegradationPreference::DISABLED);
+    if (source && source != source_) {
+      reconfiguration_needed = true;
+    }
   }
+
+  if (reconfiguration_needed) {
+    ReconfigureEncoder(nullptr);
+  }
+
   // Switch to the new source.
   source_ = source;
   if (source && stream_) {

media/engine/webrtc_video_engine_unittest.cc

diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index 6debc13..3ab079c 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -3691,6 +3691,39 @@
   // Expect 1 reconfigurations at this point from the initial configuration.
   EXPECT_EQ(1, send_stream->num_encoder_reconfigurations());
 
+  webrtc::test::FrameForwarder new_frame_forwarder;
+
+  // Set the options one more time but with a new source instance, expect
+  // one additional reconfiguration.
+  EXPECT_TRUE(
+      send_channel_->SetVideoSend(last_ssrc_, &options, &new_frame_forwarder));
+  new_frame_forwarder.IncomingCapturedFrame(frame_source_.GetFrame());
+  EXPECT_EQ(2, send_stream->num_encoder_reconfigurations());
+
+  EXPECT_TRUE(send_channel_->SetVideoSend(last_ssrc_, nullptr, nullptr));
+}
+
+// Test that if a new source is set, we reconfigure the encoder even if the
+// same options are used.
+TEST_F(WebRtcVideoChannelTest,
+       SetNewSourceWithIdenticalOptionsReconfiguresEncoder) {
+  VideoOptions options;
+  webrtc::test::FrameForwarder frame_forwarder;
+
+  AddSendStream();
+  cricket::VideoSenderParameters parameters;
+  parameters.codecs.push_back(GetEngineCodec("VP8"));
+  ASSERT_TRUE(send_channel_->SetSenderParameters(parameters));
+  FakeVideoSendStream* send_stream = fake_call_->GetVideoSendStreams().front();
+
+  EXPECT_TRUE(
+      send_channel_->SetVideoSend(last_ssrc_, &options, &frame_forwarder));
+  EXPECT_TRUE(
+      send_channel_->SetVideoSend(last_ssrc_, &options, &frame_forwarder));
+  frame_forwarder.IncomingCapturedFrame(frame_source_.GetFrame());
+  // Expect 1 reconfigurations at this point from the initial configuration.
+  EXPECT_EQ(1, send_stream->num_encoder_reconfigurations());
+
   // Set the options one more time and expect no additional reconfigurations.
   EXPECT_TRUE(
       send_channel_->SetVideoSend(last_ssrc_, &options, &frame_forwarder));