diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 4980153490..abb35843c0 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -428,14 +428,18 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::ConfigureVideoEncoderSettings( } DefaultUnsignalledSsrcHandler::DefaultUnsignalledSsrcHandler() - : default_recv_ssrc_(0), default_sink_(NULL) {} + : default_sink_(nullptr) {} UnsignalledSsrcHandler::Action DefaultUnsignalledSsrcHandler::OnUnsignalledSsrc( WebRtcVideoChannel2* channel, uint32_t ssrc) { - if (default_recv_ssrc_ != 0) { // Already one default stream, so replace it. - channel->RemoveRecvStream(default_recv_ssrc_); - default_recv_ssrc_ = 0; + rtc::Optional default_recv_ssrc = + channel->GetDefaultReceiveStreamSsrc(); + + if (default_recv_ssrc) { + LOG(LS_INFO) << "Destroying old default receive stream for SSRC=" << ssrc + << "."; + channel->RemoveRecvStream(*default_recv_ssrc); } StreamParams sp; @@ -446,7 +450,6 @@ UnsignalledSsrcHandler::Action DefaultUnsignalledSsrcHandler::OnUnsignalledSsrc( } channel->SetSink(ssrc, default_sink_); - default_recv_ssrc_ = ssrc; return kDeliverPacket; } @@ -456,11 +459,13 @@ DefaultUnsignalledSsrcHandler::GetDefaultSink() const { } void DefaultUnsignalledSsrcHandler::SetDefaultSink( - VideoMediaChannel* channel, + WebRtcVideoChannel2* channel, rtc::VideoSinkInterface* sink) { default_sink_ = sink; - if (default_recv_ssrc_ != 0) { - channel->SetSink(default_recv_ssrc_, default_sink_); + rtc::Optional default_recv_ssrc = + channel->GetDefaultReceiveStreamSsrc(); + if (default_recv_ssrc) { + channel->SetSink(*default_recv_ssrc, default_sink_); } } @@ -1321,6 +1326,8 @@ bool WebRtcVideoChannel2::SetSink( LOG(LS_INFO) << "SetSink: ssrc:" << ssrc << " " << (sink ? "(ptr)" : "nullptr"); if (ssrc == 0) { + // Do not hold |stream_crit_| here, since SetDefaultSink will call + // WebRtcVideoChannel2::GetDefaultReceiveStreamSsrc(). default_unsignalled_ssrc_handler_.SetDefaultSink(this, sink); return true; } @@ -1529,6 +1536,18 @@ void WebRtcVideoChannel2::SetInterface(NetworkInterface* iface) { kVideoRtpBufferSize); } +rtc::Optional WebRtcVideoChannel2::GetDefaultReceiveStreamSsrc() { + rtc::CritScope stream_lock(&stream_crit_); + rtc::Optional ssrc; + for (auto it = receive_streams_.begin(); it != receive_streams_.end(); ++it) { + if (it->second->IsDefaultStream()) { + ssrc.emplace(it->first); + break; + } + } + return ssrc; +} + bool WebRtcVideoChannel2::SendRtp(const uint8_t* data, size_t len, const webrtc::PacketOptions& options) { diff --git a/webrtc/media/engine/webrtcvideoengine2.h b/webrtc/media/engine/webrtcvideoengine2.h index 2c7d36ccb5..c40014a39e 100644 --- a/webrtc/media/engine/webrtcvideoengine2.h +++ b/webrtc/media/engine/webrtcvideoengine2.h @@ -22,13 +22,14 @@ #include "webrtc/base/asyncinvoker.h" #include "webrtc/base/criticalsection.h" #include "webrtc/base/networkroute.h" +#include "webrtc/base/optional.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/base/thread_checker.h" -#include "webrtc/media/base/videosinkinterface.h" -#include "webrtc/media/base/videosourceinterface.h" #include "webrtc/call/call.h" #include "webrtc/call/flexfec_receive_stream.h" #include "webrtc/media/base/mediaengine.h" +#include "webrtc/media/base/videosinkinterface.h" +#include "webrtc/media/base/videosourceinterface.h" #include "webrtc/media/engine/webrtcvideodecoderfactory.h" #include "webrtc/media/engine/webrtcvideoencoderfactory.h" #include "webrtc/video_receive_stream.h" @@ -81,15 +82,12 @@ class DefaultUnsignalledSsrcHandler : public UnsignalledSsrcHandler { uint32_t ssrc) override; rtc::VideoSinkInterface* GetDefaultSink() const; - void SetDefaultSink(VideoMediaChannel* channel, + void SetDefaultSink(WebRtcVideoChannel2* channel, rtc::VideoSinkInterface* sink); - uint32_t default_recv_ssrc() const { return default_recv_ssrc_; } - virtual ~DefaultUnsignalledSsrcHandler() = default; private: - uint32_t default_recv_ssrc_; rtc::VideoSinkInterface* default_sink_; }; @@ -177,6 +175,8 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { // Implemented for VideoMediaChannelTest. bool sending() const { return sending_; } + rtc::Optional GetDefaultReceiveStreamSsrc(); + // AdaptReason is used for expressing why a WebRtcVideoSendStream request // a lower input frame size than the currently configured camera input frame // size. There can be more than one reason OR:ed together. diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index 46664c4d94..43dbbc6d18 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -3969,6 +3969,55 @@ TEST_F(WebRtcVideoChannel2Test, ReceiveDifferentUnsignaledSsrc) { #endif } +// This test verifies that when a new default stream is created for a new +// unsignaled SSRC, the new stream does not overwrite any old stream that had +// been the default receive stream before being properly signaled. +TEST_F(WebRtcVideoChannel2Test, + NewUnsignaledStreamDoesNotDestroyPreviouslyUnsignaledStream) { + cricket::VideoRecvParameters parameters; + parameters.codecs.push_back(GetEngineCodec("VP8")); + ASSERT_TRUE(channel_->SetRecvParameters(parameters)); + + // No streams signaled and no packets received, so we should not have any + // stream objects created yet. + EXPECT_EQ(0u, fake_call_->GetVideoReceiveStreams().size()); + + // Receive packet on an unsignaled SSRC. + uint8_t data[kMinRtpPacketLen]; + cricket::RtpHeader rtp_header; + rtp_header.payload_type = GetEngineCodec("VP8").id; + rtp_header.seq_num = rtp_header.timestamp = 0; + rtp_header.ssrc = kSsrcs3[0]; + cricket::SetRtpHeader(data, sizeof(data), rtp_header); + rtc::CopyOnWriteBuffer packet(data, sizeof(data)); + rtc::PacketTime packet_time; + channel_->OnPacketReceived(&packet, packet_time); + // Default receive stream should be created. + ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size()); + FakeVideoReceiveStream* recv_stream0 = + fake_call_->GetVideoReceiveStreams()[0]; + EXPECT_EQ(kSsrcs3[0], recv_stream0->GetConfig().rtp.remote_ssrc); + + // Signal the SSRC. + EXPECT_TRUE( + channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(kSsrcs3[0]))); + ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size()); + recv_stream0 = fake_call_->GetVideoReceiveStreams()[0]; + EXPECT_EQ(kSsrcs3[0], recv_stream0->GetConfig().rtp.remote_ssrc); + + // Receive packet on a different unsignaled SSRC. + rtp_header.ssrc = kSsrcs3[1]; + cricket::SetRtpHeader(data, sizeof(data), rtp_header); + packet.SetData(data, sizeof(data)); + channel_->OnPacketReceived(&packet, packet_time); + // New default receive stream should be created, but old stream should remain. + ASSERT_EQ(2u, fake_call_->GetVideoReceiveStreams().size()); + EXPECT_EQ(recv_stream0, fake_call_->GetVideoReceiveStreams()[0]); + FakeVideoReceiveStream* recv_stream1 = + fake_call_->GetVideoReceiveStreams()[1]; + EXPECT_EQ(kSsrcs3[1], recv_stream1->GetConfig().rtp.remote_ssrc); +} + TEST_F(WebRtcVideoChannel2Test, CanSentMaxBitrateForExistingStream) { AddSendStream();