From adc4da30f420b1f50815ef3890bb4c95fb867d35 Mon Sep 17 00:00:00 2001 From: Marina Ciocea Date: Sat, 11 Apr 2020 12:42:49 +0200 Subject: [PATCH] [InsertableStreams] Fix video receiver simulcast. Save the frame transformer set on unsignaled receivers, and set the transformer when the ssrc becomes known. Pass the receiver's ssrc on registering the transformed frame callback, to associate separate frame transformer sinks for each receiver. Bug: chromium:1065838 Bug: chromium:1065838 Change-Id: I2a214bdb6cb9a8012928a03f046f311c344370f8 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/173201 Commit-Queue: Marina Ciocea Reviewed-by: Tommi Cr-Commit-Position: refs/heads/master@{#31051} --- media/engine/webrtc_video_engine.cc | 11 +++++++ media/engine/webrtc_video_engine.h | 4 +++ pc/video_rtp_receiver.cc | 12 ++++--- video/rtp_video_stream_receiver.cc | 20 +++++------- ...eam_receiver_frame_transformer_delegate.cc | 19 +++++------ ...ream_receiver_frame_transformer_delegate.h | 7 ++-- ...ver_frame_transformer_delegate_unittest.cc | 32 ++++++++++++------- video/rtp_video_stream_receiver_unittest.cc | 12 ++++--- 8 files changed, 72 insertions(+), 45 deletions(-) diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index fc9a843071..e879749f79 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -1404,6 +1404,9 @@ bool WebRtcVideoChannel::AddRecvStream(const StreamParams& sp, config.sync_group = sp.stream_ids()[0]; } + if (unsignaled_frame_transformer_ && !config.frame_transformer) + config.frame_transformer = unsignaled_frame_transformer_; + receive_streams_[ssrc] = new WebRtcVideoReceiveStream( this, call_, sp, std::move(config), decoder_factory_, default_stream, recv_codecs_, flexfec_config); @@ -3278,7 +3281,15 @@ void WebRtcVideoChannel::SetEncoderToPacketizerFrameTransformer( void WebRtcVideoChannel::SetDepacketizerToDecoderFrameTransformer( uint32_t ssrc, rtc::scoped_refptr frame_transformer) { + RTC_DCHECK(frame_transformer); RTC_DCHECK_RUN_ON(&thread_checker_); + if (ssrc == 0) { + // If the receiver is unsignaled, save the frame transformer and set it when + // the stream is associated with an ssrc. + unsignaled_frame_transformer_ = std::move(frame_transformer); + return; + } + auto matching_stream = receive_streams_.find(ssrc); if (matching_stream != receive_streams_.end()) { matching_stream->second->SetDepacketizerToDecoderFrameTransformer( diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index 323eaa96ff..6ed556e359 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -620,6 +620,10 @@ class WebRtcVideoChannel : public VideoMediaChannel, // connection. const webrtc::CryptoOptions crypto_options_ RTC_GUARDED_BY(thread_checker_); + // Optional frame transformer set on unsignaled streams. + rtc::scoped_refptr + unsignaled_frame_transformer_ RTC_GUARDED_BY(thread_checker_); + // Buffer for unhandled packets. std::unique_ptr unknown_ssrc_packet_buffer_ RTC_GUARDED_BY(thread_checker_); diff --git a/pc/video_rtp_receiver.cc b/pc/video_rtp_receiver.cc index a63a0f68f2..f093bf4b33 100644 --- a/pc/video_rtp_receiver.cc +++ b/pc/video_rtp_receiver.cc @@ -109,9 +109,9 @@ void VideoRtpReceiver::SetDepacketizerToDecoderFrameTransformer( worker_thread_->Invoke(RTC_FROM_HERE, [&] { RTC_DCHECK_RUN_ON(worker_thread_); frame_transformer_ = std::move(frame_transformer); - if (media_channel_ && ssrc_.has_value() && !stopped_) { + if (media_channel_ && !stopped_) { media_channel_->SetDepacketizerToDecoderFrameTransformer( - *ssrc_, frame_transformer_); + ssrc_.value_or(0), frame_transformer_); } }); } @@ -157,9 +157,9 @@ void VideoRtpReceiver::RestartMediaChannel(absl::optional ssrc) { SetEncodedSinkEnabled(true); } - if (frame_transformer_ && media_channel_ && ssrc_.has_value()) { + if (frame_transformer_ && media_channel_) { media_channel_->SetDepacketizerToDecoderFrameTransformer( - *ssrc_, frame_transformer_); + ssrc_.value_or(0), frame_transformer_); } }); @@ -268,6 +268,10 @@ void VideoRtpReceiver::SetMediaChannel(cricket::MediaChannel* media_channel) { if (encoded_sink_enabled) { SetEncodedSinkEnabled(true); } + if (frame_transformer_) { + media_channel_->SetDepacketizerToDecoderFrameTransformer( + ssrc_.value_or(0), frame_transformer_); + } } }); } diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index aecbf4fe54..d67d7fc051 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -285,7 +285,8 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( if (frame_transformer) { frame_transformer_delegate_ = new rtc::RefCountedObject< RtpVideoStreamReceiverFrameTransformerDelegate>( - this, std::move(frame_transformer), rtc::Thread::Current()); + this, std::move(frame_transformer), rtc::Thread::Current(), + config_.rtp.remote_ssrc); frame_transformer_delegate_->Init(); } } @@ -837,8 +838,7 @@ void RtpVideoStreamReceiver::OnAssembledFrame( if (buffered_frame_decryptor_ != nullptr) { buffered_frame_decryptor_->ManageEncryptedFrame(std::move(frame)); } else if (frame_transformer_delegate_) { - frame_transformer_delegate_->TransformFrame(std::move(frame), - config_.rtp.remote_ssrc); + frame_transformer_delegate_->TransformFrame(std::move(frame)); } else { reference_finder_->ManageFrame(std::move(frame)); } @@ -884,15 +884,11 @@ void RtpVideoStreamReceiver::SetFrameDecryptor( void RtpVideoStreamReceiver::SetDepacketizerToDecoderFrameTransformer( rtc::scoped_refptr frame_transformer) { RTC_DCHECK_RUN_ON(&network_tc_); - if (!frame_transformer_delegate_) { - frame_transformer_delegate_ = new rtc::RefCountedObject< - RtpVideoStreamReceiverFrameTransformerDelegate>( - this, std::move(frame_transformer), rtc::Thread::Current()); - frame_transformer_delegate_->Init(); - } else { - RTC_LOG(LS_ERROR) - << "Attempting to replace an existing frame transformer in a receiver"; - } + frame_transformer_delegate_ = + new rtc::RefCountedObject( + this, std::move(frame_transformer), rtc::Thread::Current(), + config_.rtp.remote_ssrc); + frame_transformer_delegate_->Init(); } void RtpVideoStreamReceiver::UpdateRtt(int64_t max_rtt_ms) { diff --git a/video/rtp_video_stream_receiver_frame_transformer_delegate.cc b/video/rtp_video_stream_receiver_frame_transformer_delegate.cc index db7f48616d..c2fb8feb42 100644 --- a/video/rtp_video_stream_receiver_frame_transformer_delegate.cc +++ b/video/rtp_video_stream_receiver_frame_transformer_delegate.cc @@ -66,27 +66,28 @@ RtpVideoStreamReceiverFrameTransformerDelegate:: RtpVideoStreamReceiverFrameTransformerDelegate( RtpVideoStreamReceiver* receiver, rtc::scoped_refptr frame_transformer, - rtc::Thread* network_thread) + rtc::Thread* network_thread, + uint32_t ssrc) : receiver_(receiver), frame_transformer_(std::move(frame_transformer)), - network_thread_(network_thread) {} + network_thread_(network_thread), + ssrc_(ssrc) {} void RtpVideoStreamReceiverFrameTransformerDelegate::Init() { RTC_DCHECK_RUN_ON(&network_sequence_checker_); - frame_transformer_->RegisterTransformedFrameCallback( - rtc::scoped_refptr(this)); + frame_transformer_->RegisterTransformedFrameSinkCallback( + rtc::scoped_refptr(this), ssrc_); } void RtpVideoStreamReceiverFrameTransformerDelegate::Reset() { RTC_DCHECK_RUN_ON(&network_sequence_checker_); - frame_transformer_->UnregisterTransformedFrameCallback(); + frame_transformer_->UnregisterTransformedFrameSinkCallback(ssrc_); frame_transformer_ = nullptr; receiver_ = nullptr; } void RtpVideoStreamReceiverFrameTransformerDelegate::TransformFrame( - std::unique_ptr frame, - uint32_t ssrc) { + std::unique_ptr frame) { RTC_DCHECK_RUN_ON(&network_sequence_checker_); // TODO(bugs.webrtc.org/11380) remove once this version of TransformFrame is // deprecated. @@ -95,11 +96,11 @@ void RtpVideoStreamReceiverFrameTransformerDelegate::TransformFrame( auto frame_copy = std::make_unique(*frame.get()); frame_transformer_->TransformFrame(std::move(frame_copy), - std::move(additional_data), ssrc); + std::move(additional_data), ssrc_); frame_transformer_->Transform( std::make_unique(std::move(frame), - ssrc)); + ssrc_)); } void RtpVideoStreamReceiverFrameTransformerDelegate::OnTransformedFrame( diff --git a/video/rtp_video_stream_receiver_frame_transformer_delegate.h b/video/rtp_video_stream_receiver_frame_transformer_delegate.h index 85fad1e002..eb3c2625f0 100644 --- a/video/rtp_video_stream_receiver_frame_transformer_delegate.h +++ b/video/rtp_video_stream_receiver_frame_transformer_delegate.h @@ -30,14 +30,14 @@ class RtpVideoStreamReceiverFrameTransformerDelegate RtpVideoStreamReceiverFrameTransformerDelegate( RtpVideoStreamReceiver* receiver, rtc::scoped_refptr frame_transformer, - rtc::Thread* network_thread); + rtc::Thread* network_thread, + uint32_t ssrc); void Init(); void Reset(); // Delegates the call to FrameTransformerInterface::TransformFrame. - void TransformFrame(std::unique_ptr frame, - uint32_t ssrc); + void TransformFrame(std::unique_ptr frame); // Implements TransformedFrameCallback. Can be called on any thread. Posts // the transformed frame to be managed on the |network_thread_|. @@ -60,6 +60,7 @@ class RtpVideoStreamReceiverFrameTransformerDelegate rtc::scoped_refptr frame_transformer_ RTC_GUARDED_BY(network_sequence_checker_); rtc::Thread* const network_thread_; + const uint32_t ssrc_; }; } // namespace webrtc diff --git a/video/rtp_video_stream_receiver_frame_transformer_delegate_unittest.cc b/video/rtp_video_stream_receiver_frame_transformer_delegate_unittest.cc index bc9fe13a72..5626d83d39 100644 --- a/video/rtp_video_stream_receiver_frame_transformer_delegate_unittest.cc +++ b/video/rtp_video_stream_receiver_frame_transformer_delegate_unittest.cc @@ -117,33 +117,39 @@ class MockFrameTransformer : public FrameTransformerInterface { uint32_t), (override)); MOCK_METHOD(void, - RegisterTransformedFrameCallback, - (rtc::scoped_refptr), + RegisterTransformedFrameSinkCallback, + (rtc::scoped_refptr, uint32_t), + (override)); + MOCK_METHOD(void, + UnregisterTransformedFrameSinkCallback, + (uint32_t), (override)); - MOCK_METHOD(void, UnregisterTransformedFrameCallback, (), (override)); }; TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest, - RegisterTransformedFrameCallbackOnInit) { + RegisterTransformedFrameCallbackSinkOnInit) { TestRtpVideoStreamReceiver receiver; rtc::scoped_refptr frame_transformer( new rtc::RefCountedObject()); rtc::scoped_refptr delegate( new rtc::RefCountedObject( - &receiver, frame_transformer, rtc::Thread::Current())); - EXPECT_CALL(*frame_transformer, RegisterTransformedFrameCallback); + &receiver, frame_transformer, rtc::Thread::Current(), + /*remote_ssrc*/ 1111)); + EXPECT_CALL(*frame_transformer, + RegisterTransformedFrameSinkCallback(testing::_, 1111)); delegate->Init(); } TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest, - UnregisterTransformedFrameCallbackOnReset) { + UnregisterTransformedFrameSinkCallbackOnReset) { TestRtpVideoStreamReceiver receiver; rtc::scoped_refptr frame_transformer( new rtc::RefCountedObject()); rtc::scoped_refptr delegate( new rtc::RefCountedObject( - &receiver, frame_transformer, rtc::Thread::Current())); - EXPECT_CALL(*frame_transformer, UnregisterTransformedFrameCallback); + &receiver, frame_transformer, rtc::Thread::Current(), + /*remote_ssrc*/ 1111)); + EXPECT_CALL(*frame_transformer, UnregisterTransformedFrameSinkCallback(1111)); delegate->Reset(); } @@ -153,12 +159,13 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest, TransformFrame) { new rtc::RefCountedObject()); rtc::scoped_refptr delegate( new rtc::RefCountedObject( - &receiver, frame_transformer, rtc::Thread::Current())); + &receiver, frame_transformer, rtc::Thread::Current(), + /*remote_ssrc*/ 1111)); auto frame = CreateRtpFrameObject(); EXPECT_CALL(*frame_transformer, TransformFrame(_, RtpDescriptorAuthentication(RTPVideoHeader()), /*remote_ssrc*/ 1111)); - delegate->TransformFrame(std::move(frame), /*remote_ssrc*/ 1111); + delegate->TransformFrame(std::move(frame)); } TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest, @@ -176,7 +183,8 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest, RTC_FROM_HERE, [&]() mutable { return new rtc::RefCountedObject< RtpVideoStreamReceiverFrameTransformerDelegate>( - &receiver, frame_transformer, network_thread.get()); + &receiver, frame_transformer, network_thread.get(), + /*remote_ssrc*/ 1111); }); auto frame = CreateRtpFrameObject(); diff --git a/video/rtp_video_stream_receiver_unittest.cc b/video/rtp_video_stream_receiver_unittest.cc index ad3bc607f4..088465c301 100644 --- a/video/rtp_video_stream_receiver_unittest.cc +++ b/video/rtp_video_stream_receiver_unittest.cc @@ -131,9 +131,9 @@ class MockFrameTransformer : public FrameTransformerInterface { void(std::unique_ptr frame, std::vector additional_data, uint32_t ssrc)); - MOCK_METHOD1(RegisterTransformedFrameCallback, - void(rtc::scoped_refptr)); - MOCK_METHOD0(UnregisterTransformedFrameCallback, void()); + MOCK_METHOD2(RegisterTransformedFrameSinkCallback, + void(rtc::scoped_refptr, uint32_t)); + MOCK_METHOD1(UnregisterTransformedFrameSinkCallback, void(uint32_t)); }; constexpr uint32_t kSsrc = 111; @@ -1221,7 +1221,8 @@ TEST_F(RtpVideoStreamReceiverTest, RepeatedSecondarySinkDisallowed) { TEST_F(RtpVideoStreamReceiverTest, TransformFrame) { rtc::scoped_refptr mock_frame_transformer = new rtc::RefCountedObject(); - EXPECT_CALL(*mock_frame_transformer, RegisterTransformedFrameCallback); + EXPECT_CALL(*mock_frame_transformer, + RegisterTransformedFrameSinkCallback(_, config_.rtp.remote_ssrc)); auto receiver = std::make_unique( Clock::GetRealTimeClock(), &mock_transport_, nullptr, nullptr, &config_, rtp_receive_statistics_.get(), nullptr, process_thread_.get(), @@ -1248,7 +1249,8 @@ TEST_F(RtpVideoStreamReceiverTest, TransformFrame) { config_.rtp.remote_ssrc)); receiver->OnReceivedPayloadData(data, rtp_packet, video_header); - EXPECT_CALL(*mock_frame_transformer, UnregisterTransformedFrameCallback()); + EXPECT_CALL(*mock_frame_transformer, + UnregisterTransformedFrameSinkCallback(config_.rtp.remote_ssrc)); receiver = nullptr; }