From 55107c8507596b98d569f9304b15b6b5847936a4 Mon Sep 17 00:00:00 2001 From: Tommi Date: Wed, 16 Jun 2021 16:31:18 +0200 Subject: 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 Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#34308} --- audio/audio_receive_stream.cc | 5 ++++ audio/audio_receive_stream.h | 3 +++ call/call.cc | 12 ++++++++++ call/call.h | 3 +++ call/degraded_call.cc | 5 ++++ call/degraded_call.h | 3 +++ media/engine/fake_webrtc_call.cc | 6 +++++ media/engine/fake_webrtc_call.h | 6 +++++ media/engine/webrtc_voice_engine.cc | 23 ++++-------------- media/engine/webrtc_voice_engine_unittest.cc | 17 ++++++++++---- video/receive_statistics_proxy2.cc | 35 ++++++++++++---------------- video/rtp_video_stream_receiver2.cc | 1 - 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 #include +#include #include #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(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 #include +#include #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(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& 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(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(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 { -- cgit v1.2.3