Update the sync_group id without recreating audio receive streams.

Bug: webrtc:11993
Change-Id: I7aaff6d6f89332e60967fba741252b630fd941cf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/222043
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34308}

12 files changed, 76 insertions, 43 deletions

diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc
index bae0473501..6ec266b68d 100644
--- a/audio/audio_receive_stream.cc
+++ b/audio/audio_receive_stream.cc
@@ -445,6 +445,11 @@ void AudioReceiveStream::DeliverRtcp(const uint8_t* packet, size_t length) {
   channel_receive_->ReceivedRTCPPacket(packet, length);
 }
 
+void AudioReceiveStream::SetSyncGroup(const std::string& sync_group) {
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+  config_.sync_group = sync_group;
+}
+
 void AudioReceiveStream::SetLocalSsrc(uint32_t local_ssrc) {
   RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
   // TODO(tommi): Consider storing local_ssrc in one place.
diff --git a/audio/audio_receive_stream.h b/audio/audio_receive_stream.h
index 85ab8a1bb5..dc64e94510 100644
--- a/audio/audio_receive_stream.h
+++ b/audio/audio_receive_stream.h
@@ -13,6 +13,7 @@
 
 #include <map>
 #include <memory>
+#include <string>
 #include <vector>
 
 #include "api/audio/audio_mixer.h"
@@ -122,6 +123,8 @@ class AudioReceiveStream final : public webrtc::AudioReceiveStream,
   void AssociateSendStream(AudioSendStream* send_stream);
   void DeliverRtcp(const uint8_t* packet, size_t length);
 
+  void SetSyncGroup(const std::string& sync_group);
+
   void SetLocalSsrc(uint32_t local_ssrc);
 
   uint32_t local_ssrc() const;
diff --git a/call/call.cc b/call/call.cc
index b9ce0eb8a7..f4a7d7cc9e 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -272,6 +272,9 @@ class Call final : public webrtc::Call,
   void OnLocalSsrcUpdated(webrtc::AudioReceiveStream& stream,
                           uint32_t local_ssrc) override;
 
+  void OnUpdateSyncGroup(webrtc::AudioReceiveStream& stream,
+                         const std::string& sync_group) override;
+
   void OnSentPacket(const rtc::SentPacket& sent_packet) override;
 
   // Implements TargetTransferRateObserver,
@@ -1371,6 +1374,15 @@ void Call::OnLocalSsrcUpdated(webrtc::AudioReceiveStream& stream,
                                                                    : nullptr);
 }
 
+void Call::OnUpdateSyncGroup(webrtc::AudioReceiveStream& stream,
+                             const std::string& sync_group) {
+  RTC_DCHECK_RUN_ON(worker_thread_);
+  webrtc::internal::AudioReceiveStream& receive_stream =
+      static_cast<webrtc::internal::AudioReceiveStream&>(stream);
+  receive_stream.SetSyncGroup(sync_group);
+  ConfigureSync(sync_group);
+}
+
 void Call::OnSentPacket(const rtc::SentPacket& sent_packet) {
   // In production and with most tests, this method will be called on the
   // network thread. However some test classes such as DirectTransport don't
diff --git a/call/call.h b/call/call.h
index ae53b30593..f6388c3c78 100644
--- a/call/call.h
+++ b/call/call.h
@@ -160,6 +160,9 @@ class Call {
   virtual void OnLocalSsrcUpdated(AudioReceiveStream& stream,
                                   uint32_t local_ssrc) = 0;
 
+  virtual void OnUpdateSyncGroup(AudioReceiveStream& stream,
+                                 const std::string& sync_group) = 0;
+
   virtual void OnSentPacket(const rtc::SentPacket& sent_packet) = 0;
 
   virtual void SetClientBitratePreferences(
diff --git a/call/degraded_call.cc b/call/degraded_call.cc
index a34a9c873d..5462085490 100644
--- a/call/degraded_call.cc
+++ b/call/degraded_call.cc
@@ -293,6 +293,11 @@ void DegradedCall::OnLocalSsrcUpdated(AudioReceiveStream& stream,
   call_->OnLocalSsrcUpdated(stream, local_ssrc);
 }
 
+void DegradedCall::OnUpdateSyncGroup(AudioReceiveStream& stream,
+                                     const std::string& sync_group) {
+  call_->OnUpdateSyncGroup(stream, sync_group);
+}
+
 void DegradedCall::OnSentPacket(const rtc::SentPacket& sent_packet) {
   if (send_config_) {
     // If we have a degraded send-transport, we have already notified call
diff --git a/call/degraded_call.h b/call/degraded_call.h
index 621bbb31ac..70dc126807 100644
--- a/call/degraded_call.h
+++ b/call/degraded_call.h
@@ -16,6 +16,7 @@
 
 #include <map>
 #include <memory>
+#include <string>
 
 #include "absl/types/optional.h"
 #include "api/call/transport.h"
@@ -95,6 +96,8 @@ class DegradedCall : public Call, private PacketReceiver {
       int transport_overhead_per_packet) override;
   void OnLocalSsrcUpdated(AudioReceiveStream& stream,
                           uint32_t local_ssrc) override;
+  void OnUpdateSyncGroup(AudioReceiveStream& stream,
+                         const std::string& sync_group) override;
   void OnSentPacket(const rtc::SentPacket& sent_packet) override;
 
  protected:
diff --git a/media/engine/fake_webrtc_call.cc b/media/engine/fake_webrtc_call.cc
index 0483659910..e8c7f6e0c9 100644
--- a/media/engine/fake_webrtc_call.cc
+++ b/media/engine/fake_webrtc_call.cc
@@ -674,6 +674,12 @@ void FakeCall::OnLocalSsrcUpdated(webrtc::AudioReceiveStream& stream,
   fake_stream.SetLocalSsrc(local_ssrc);
 }
 
+void FakeCall::OnUpdateSyncGroup(webrtc::AudioReceiveStream& stream,
+                                 const std::string& sync_group) {
+  auto& fake_stream = static_cast<FakeAudioReceiveStream&>(stream);
+  fake_stream.SetSyncGroup(sync_group);
+}
+
 void FakeCall::OnSentPacket(const rtc::SentPacket& sent_packet) {
   last_sent_packet_ = sent_packet;
   if (sent_packet.packet_id >= 0) {
diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h
index 9dec31dbd1..20e65d45f4 100644
--- a/media/engine/fake_webrtc_call.h
+++ b/media/engine/fake_webrtc_call.h
@@ -104,6 +104,10 @@ class FakeAudioReceiveStream final : public webrtc::AudioReceiveStream {
     config_.rtp.local_ssrc = local_ssrc;
   }
 
+  void SetSyncGroup(const std::string& sync_group) {
+    config_.sync_group = sync_group;
+  }
+
  private:
   const webrtc::ReceiveStream::RtpConfig& rtp_config() const override {
     return config_.rtp;
@@ -397,6 +401,8 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver {
       int transport_overhead_per_packet) override;
   void OnLocalSsrcUpdated(webrtc::AudioReceiveStream& stream,
                           uint32_t local_ssrc) override;
+  void OnUpdateSyncGroup(webrtc::AudioReceiveStream& stream,
+                         const std::string& sync_group) override;
   void OnSentPacket(const rtc::SentPacket& sent_packet) override;
 
   webrtc::TaskQueueBase* const network_thread_;
diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc
index 1e896cdb50..351ea0911c 100644
--- a/media/engine/webrtc_voice_engine.cc
+++ b/media/engine/webrtc_voice_engine.cc
@@ -1249,22 +1249,6 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream {
     stream_->SetDecoderMap(decoder_map);
   }
 
-  void MaybeRecreateAudioReceiveStream(
-      const std::vector<std::string>& stream_ids) {
-    RTC_DCHECK_RUN_ON(&worker_thread_checker_);
-    std::string sync_group;
-    if (!stream_ids.empty()) {
-      sync_group = stream_ids[0];
-    }
-    if (config_.sync_group != sync_group) {
-      RTC_LOG(LS_INFO) << "Recreating AudioReceiveStream for SSRC="
-                       << config_.rtp.remote_ssrc
-                       << " because of sync group change.";
-      config_.sync_group = sync_group;
-      RecreateAudioReceiveStream();
-    }
-  }
-
   webrtc::AudioReceiveStream::Stats GetStats(
       bool get_and_clear_legacy_stats) const {
     RTC_DCHECK_RUN_ON(&worker_thread_checker_);
@@ -1985,9 +1969,12 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) {
   const uint32_t ssrc = sp.first_ssrc();
 
   // If this stream was previously received unsignaled, we promote it, possibly
-  // recreating the AudioReceiveStream, if stream ids have changed.
+  // updating the sync group if stream ids have changed.
   if (MaybeDeregisterUnsignaledRecvStream(ssrc)) {
-    recv_streams_[ssrc]->MaybeRecreateAudioReceiveStream(sp.stream_ids());
+    auto stream_ids = sp.stream_ids();
+    std::string sync_group = stream_ids.empty() ? std::string() : stream_ids[0];
+    call_->OnUpdateSyncGroup(recv_streams_[ssrc]->stream(),
+                             std::move(sync_group));
     return true;
   }
 
diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc
index ecf8c07e6a..c570b1a03a 100644
--- a/media/engine/webrtc_voice_engine_unittest.cc
+++ b/media/engine/webrtc_voice_engine_unittest.cc
@@ -2845,7 +2845,7 @@ TEST_P(WebRtcVoiceEngineTestFake, AddRecvStreamAfterUnsignaled_NoRecreate) {
   EXPECT_EQ(audio_receive_stream_id, streams.front()->id());
 }
 
-TEST_P(WebRtcVoiceEngineTestFake, AddRecvStreamAfterUnsignaled_Recreate) {
+TEST_P(WebRtcVoiceEngineTestFake, AddRecvStreamAfterUnsignaled_Updates) {
   EXPECT_TRUE(SetupChannel());
 
   // Spawn unsignaled stream with SSRC=1.
@@ -2854,17 +2854,26 @@ TEST_P(WebRtcVoiceEngineTestFake, AddRecvStreamAfterUnsignaled_Recreate) {
   EXPECT_TRUE(
       GetRecvStream(1).VerifyLastPacket(kPcmuFrame, sizeof(kPcmuFrame)));
 
-  // Verify that the underlying stream object in Call *is* recreated when a
+  // Verify that the underlying stream object in Call gets updated when a
   // stream with SSRC=1 is added, and which has changed stream parameters.
   const auto& streams = call_.GetAudioReceiveStreams();
   EXPECT_EQ(1u, streams.size());
+  // The sync_group id should be empty.
+  EXPECT_TRUE(streams.front()->GetConfig().sync_group.empty());
+
+  const std::string new_stream_id("stream_id");
   int audio_receive_stream_id = streams.front()->id();
   cricket::StreamParams stream_params;
   stream_params.ssrcs.push_back(1);
-  stream_params.set_stream_ids({"stream_id"});
+  stream_params.set_stream_ids({new_stream_id});
+
   EXPECT_TRUE(channel_->AddRecvStream(stream_params));
   EXPECT_EQ(1u, streams.size());
-  EXPECT_NE(audio_receive_stream_id, streams.front()->id());
+  // The audio receive stream should not have been recreated.
+  EXPECT_EQ(audio_receive_stream_id, streams.front()->id());
+
+  // The sync_group id should now match with the new stream params.
+  EXPECT_EQ(new_stream_id, streams.front()->GetConfig().sync_group);
 }
 
 // Test that AddRecvStream creates new stream.
diff --git a/video/receive_statistics_proxy2.cc b/video/receive_statistics_proxy2.cc
index 3cce3c8ea4..af3cd221e7 100644
--- a/video/receive_statistics_proxy2.cc
+++ b/video/receive_statistics_proxy2.cc
@@ -946,26 +946,21 @@ void ReceiveStatisticsProxy::OnRenderedFrame(
 void ReceiveStatisticsProxy::OnSyncOffsetUpdated(int64_t video_playout_ntp_ms,
                                                  int64_t sync_offset_ms,
                                                  double estimated_freq_khz) {
-  RTC_DCHECK_RUN_ON(&incoming_render_queue_);
-  int64_t now_ms = clock_->TimeInMilliseconds();
-  worker_thread_->PostTask(
-      ToQueuedTask(task_safety_, [video_playout_ntp_ms, sync_offset_ms,
-                                  estimated_freq_khz, now_ms, this]() {
-        RTC_DCHECK_RUN_ON(&main_thread_);
-        sync_offset_counter_.Add(std::abs(sync_offset_ms));
-        stats_.sync_offset_ms = sync_offset_ms;
-        last_estimated_playout_ntp_timestamp_ms_ = video_playout_ntp_ms;
-        last_estimated_playout_time_ms_ = now_ms;
-
-        const double kMaxFreqKhz = 10000.0;
-        int offset_khz = kMaxFreqKhz;
-        // Should not be zero or negative. If so, report max.
-        if (estimated_freq_khz < kMaxFreqKhz && estimated_freq_khz > 0.0)
-          offset_khz =
-              static_cast<int>(std::fabs(estimated_freq_khz - 90.0) + 0.5);
-
-        freq_offset_counter_.Add(offset_khz);
-      }));
+  RTC_DCHECK_RUN_ON(&main_thread_);
+
+  const int64_t now_ms = clock_->TimeInMilliseconds();
+  sync_offset_counter_.Add(std::abs(sync_offset_ms));
+  stats_.sync_offset_ms = sync_offset_ms;
+  last_estimated_playout_ntp_timestamp_ms_ = video_playout_ntp_ms;
+  last_estimated_playout_time_ms_ = now_ms;
+
+  const double kMaxFreqKhz = 10000.0;
+  int offset_khz = kMaxFreqKhz;
+  // Should not be zero or negative. If so, report max.
+  if (estimated_freq_khz < kMaxFreqKhz && estimated_freq_khz > 0.0)
+    offset_khz = static_cast<int>(std::fabs(estimated_freq_khz - 90.0) + 0.5);
+
+  freq_offset_counter_.Add(offset_khz);
 }
 
 void ReceiveStatisticsProxy::OnCompleteFrame(bool is_keyframe,
diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc
index 75e71cdafa..1dcdf72e60 100644
--- a/video/rtp_video_stream_receiver2.cc
+++ b/video/rtp_video_stream_receiver2.cc
@@ -49,7 +49,6 @@
 #include "system_wrappers/include/field_trial.h"
 #include "system_wrappers/include/metrics.h"
 #include "system_wrappers/include/ntp_time.h"
-#include "video/receive_statistics_proxy2.h"
 
 namespace webrtc {