diff --git a/webrtc/media/engine/fakewebrtccall.cc b/webrtc/media/engine/fakewebrtccall.cc index 0cdc4d4863..9a05ae67e4 100644 --- a/webrtc/media/engine/fakewebrtccall.cc +++ b/webrtc/media/engine/fakewebrtccall.cc @@ -21,7 +21,8 @@ namespace cricket { FakeAudioSendStream::FakeAudioSendStream( - const webrtc::AudioSendStream::Config& config) : config_(config) { + int id, const webrtc::AudioSendStream::Config& config) + : id_(id), config_(config) { RTC_DCHECK(config.voe_channel_id != -1); } @@ -59,8 +60,8 @@ webrtc::AudioSendStream::Stats FakeAudioSendStream::GetStats() const { } FakeAudioReceiveStream::FakeAudioReceiveStream( - const webrtc::AudioReceiveStream::Config& config) - : config_(config) { + int id, const webrtc::AudioReceiveStream::Config& config) + : id_(id), config_(config) { RTC_DCHECK(config.voe_channel_id != -1); } @@ -404,7 +405,8 @@ webrtc::NetworkState FakeCall::GetNetworkState(webrtc::MediaType media) const { webrtc::AudioSendStream* FakeCall::CreateAudioSendStream( const webrtc::AudioSendStream::Config& config) { - FakeAudioSendStream* fake_stream = new FakeAudioSendStream(config); + FakeAudioSendStream* fake_stream = new FakeAudioSendStream(next_stream_id_++, + config); audio_send_streams_.push_back(fake_stream); ++num_created_send_streams_; return fake_stream; @@ -424,7 +426,8 @@ void FakeCall::DestroyAudioSendStream(webrtc::AudioSendStream* send_stream) { webrtc::AudioReceiveStream* FakeCall::CreateAudioReceiveStream( const webrtc::AudioReceiveStream::Config& config) { - audio_receive_streams_.push_back(new FakeAudioReceiveStream(config)); + audio_receive_streams_.push_back(new FakeAudioReceiveStream(next_stream_id_++, + config)); ++num_created_receive_streams_; return audio_receive_streams_.back(); } diff --git a/webrtc/media/engine/fakewebrtccall.h b/webrtc/media/engine/fakewebrtccall.h index 8557720d27..8a1547be19 100644 --- a/webrtc/media/engine/fakewebrtccall.h +++ b/webrtc/media/engine/fakewebrtccall.h @@ -43,8 +43,10 @@ class FakeAudioSendStream final : public webrtc::AudioSendStream { int duration_ms = 0; }; - explicit FakeAudioSendStream(const webrtc::AudioSendStream::Config& config); + explicit FakeAudioSendStream( + int id, const webrtc::AudioSendStream::Config& config); + int id() const { return id_; } const webrtc::AudioSendStream::Config& GetConfig() const; void SetStats(const webrtc::AudioSendStream::Stats& stats); TelephoneEvent GetLatestTelephoneEvent() const; @@ -61,6 +63,7 @@ class FakeAudioSendStream final : public webrtc::AudioSendStream { void SetMuted(bool muted) override; webrtc::AudioSendStream::Stats GetStats() const override; + int id_ = -1; TelephoneEvent latest_telephone_event_; webrtc::AudioSendStream::Config config_; webrtc::AudioSendStream::Stats stats_; @@ -71,8 +74,9 @@ class FakeAudioSendStream final : public webrtc::AudioSendStream { class FakeAudioReceiveStream final : public webrtc::AudioReceiveStream { public: explicit FakeAudioReceiveStream( - const webrtc::AudioReceiveStream::Config& config); + int id, const webrtc::AudioReceiveStream::Config& config); + int id() const { return id_; } const webrtc::AudioReceiveStream::Config& GetConfig() const; void SetStats(const webrtc::AudioReceiveStream::Stats& stats); int received_packets() const { return received_packets_; } @@ -93,6 +97,7 @@ class FakeAudioReceiveStream final : public webrtc::AudioReceiveStream { void SetSink(std::unique_ptr sink) override; void SetGain(float gain) override; + int id_ = -1; webrtc::AudioReceiveStream::Config config_; webrtc::AudioReceiveStream::Stats stats_; int received_packets_ = 0; @@ -293,6 +298,7 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver { webrtc::NetworkState video_network_state_; rtc::SentPacket last_sent_packet_; int last_sent_nonnegative_packet_id_ = -1; + int next_stream_id_ = 665; webrtc::Call::Stats stats_; std::vector video_send_streams_; std::vector audio_send_streams_; diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index f96c8e741e..f374b76e3e 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -1609,6 +1609,14 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream { RecreateAudioReceiveStream(); } + void MaybeRecreateAudioReceiveStream(const std::string& sync_group) { + RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); + if (config_.sync_group != sync_group) { + config_.sync_group = sync_group; + RecreateAudioReceiveStream(); + } + } + webrtc::AudioReceiveStream::Stats GetStats() const { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); RTC_DCHECK(stream_); @@ -2276,10 +2284,13 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) { return false; } - // Remove the default receive stream if one had been created with this ssrc; - // we'll recreate it then. + // If the default receive stream was created with this ssrc, we unmark it as + // being the default stream, and possibly recreate the AudioReceiveStream, if + // sync_label has changed. if (IsDefaultRecvStream(ssrc)) { - RemoveRecvStream(ssrc); + recv_streams_[ssrc]->MaybeRecreateAudioReceiveStream(sp.sync_label); + default_recv_ssrc_ = -1; + return true; } if (GetReceiveChannelId(ssrc) != -1) { diff --git a/webrtc/media/engine/webrtcvoiceengine_unittest.cc b/webrtc/media/engine/webrtcvoiceengine_unittest.cc index c55389ae73..0bb3501339 100644 --- a/webrtc/media/engine/webrtcvoiceengine_unittest.cc +++ b/webrtc/media/engine/webrtcvoiceengine_unittest.cc @@ -2814,6 +2814,50 @@ TEST_F(WebRtcVoiceEngineTestFake, RecvUnsignalledAfterSignalled) { EXPECT_EQ(2, call_.GetAudioReceiveStreams().size()); } +// Two tests to verify that adding a receive stream with the same SSRC as a +// previously added unsignaled stream will only recreate underlying stream +// objects if the stream parameters have changed. +TEST_F(WebRtcVoiceEngineTestFake, AddRecvStreamAfterUnsignaled_NoRecreate) { + EXPECT_TRUE(SetupChannel()); + + // Spawn unsignaled stream with SSRC=1. + DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame)); + EXPECT_EQ(1, call_.GetAudioReceiveStreams().size()); + EXPECT_TRUE(GetRecvStream(1).VerifyLastPacket(kPcmuFrame, + sizeof(kPcmuFrame))); + + // Verify that the underlying stream object in Call is not recreated when a + // stream with SSRC=1 is added. + const auto& streams = call_.GetAudioReceiveStreams(); + EXPECT_EQ(1, streams.size()); + int audio_receive_stream_id = streams.front()->id(); + EXPECT_TRUE(AddRecvStream(1)); + EXPECT_EQ(1, streams.size()); + EXPECT_EQ(audio_receive_stream_id, streams.front()->id()); +} + +TEST_F(WebRtcVoiceEngineTestFake, AddRecvStreamAfterUnsignaled_Recreate) { + EXPECT_TRUE(SetupChannel()); + + // Spawn unsignaled stream with SSRC=1. + DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame)); + EXPECT_EQ(1, call_.GetAudioReceiveStreams().size()); + EXPECT_TRUE(GetRecvStream(1).VerifyLastPacket(kPcmuFrame, + sizeof(kPcmuFrame))); + + // Verify that the underlying stream object in Call *is* recreated when a + // stream with SSRC=1 is added, and which has changed stream parameters. + const auto& streams = call_.GetAudioReceiveStreams(); + EXPECT_EQ(1, streams.size()); + int audio_receive_stream_id = streams.front()->id(); + cricket::StreamParams stream_params; + stream_params.ssrcs.push_back(1); + stream_params.sync_label = "sync_label"; + EXPECT_TRUE(channel_->AddRecvStream(stream_params)); + EXPECT_EQ(1, streams.size()); + EXPECT_NE(audio_receive_stream_id, streams.front()->id()); +} + // Test that we properly handle failures to add a receive stream. TEST_F(WebRtcVoiceEngineTestFake, AddRecvStreamFail) { EXPECT_TRUE(SetupChannel());