From ed3832b89f50638ec27a7764f712ca019c2db0db Mon Sep 17 00:00:00 2001 From: Tommi Date: Tue, 22 Mar 2022 11:54:09 +0100 Subject: [PATCH] Make sure OnChanged() notifications are handled, post construction. If an instance of AudioRtpReceiver was initialized with a valid media channel pointer (i.e. SetMediaChannel() was not being called), then OnChanged() notification would not be handled correctly. This fixes the issue by making sure the safety flag is marked as 'alive' when [re]starting the media channel. Bug: webrtc:13854 Fixes: webrtc:13854 Change-Id: Iaa5cfeb4036bfc9dc2efbfa9e1319d508ab151a9 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/256361 Reviewed-by: Harald Alvestrand Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#36290} --- pc/audio_rtp_receiver.cc | 5 +++++ pc/audio_rtp_receiver_unittest.cc | 31 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/pc/audio_rtp_receiver.cc b/pc/audio_rtp_receiver.cc index 3a306720c7..58f4dfafe2 100644 --- a/pc/audio_rtp_receiver.cc +++ b/pc/audio_rtp_receiver.cc @@ -191,6 +191,11 @@ void AudioRtpReceiver::RestartMediaChannel_w( if (!media_channel_) return; // Can't restart. + // Make sure the safety flag is marked as `alive` for cases where the media + // channel was provided via the ctor and not an explicit call to + // SetMediaChannel. + worker_thread_safety_->SetAlive(); + if (state != MediaSourceInterface::kInitializing) { if (ssrc_ == ssrc) return; diff --git a/pc/audio_rtp_receiver_unittest.cc b/pc/audio_rtp_receiver_unittest.cc index ac843fe9c2..de72d3f9fb 100644 --- a/pc/audio_rtp_receiver_unittest.cc +++ b/pc/audio_rtp_receiver_unittest.cc @@ -18,6 +18,7 @@ #include "rtc_base/thread.h" #include "test/gmock.h" #include "test/gtest.h" +#include "test/run_loop.h" using ::testing::_; using ::testing::InvokeWithoutArgs; @@ -93,4 +94,34 @@ TEST_F(AudioRtpReceiverTest, VolumesSetBeforeStartingAreRespected) { receiver_->SetupMediaChannel(kSsrc); } + +// Tests that OnChanged notifications are processed correctly on the worker +// thread when a media channel pointer is passed to the receiver via the +// constructor. +TEST(AudioRtpReceiver, OnChangedNotificationsAfterConstruction) { + webrtc::test::RunLoop loop; + auto* thread = rtc::Thread::Current(); // Points to loop's thread. + cricket::MockVoiceMediaChannel media_channel(thread); + auto receiver = rtc::make_ref_counted( + thread, std::string(), std::vector(), true, &media_channel); + + EXPECT_CALL(media_channel, SetDefaultRawAudioSink(_)).Times(1); + EXPECT_CALL(media_channel, SetDefaultOutputVolume(kDefaultVolume)).Times(1); + receiver->SetupUnsignaledMediaChannel(); + loop.Flush(); + + // Mark the track as disabled. + receiver->track()->set_enabled(false); + + // When the track was marked as disabled, an async notification was queued + // for the worker thread. This notification should trigger the volume + // of the media channel to be set to kVolumeMuted. + // Flush the worker thread, but set the expectation first for the call. + EXPECT_CALL(media_channel, SetDefaultOutputVolume(kVolumeMuted)).Times(1); + loop.Flush(); + + EXPECT_CALL(media_channel, SetDefaultOutputVolume(kVolumeMuted)).Times(1); + receiver->SetMediaChannel(nullptr); +} + } // namespace webrtc