From 9e2b3155ee9e89888dc7102c946ba5473e55071e Mon Sep 17 00:00:00 2001 From: Tommi Date: Mon, 21 Jun 2021 22:15:39 +0200 Subject: [PATCH] Minor code cleanup of WebRtcVideoReceiveStream. * Remove unnecessary decoder factory pointer. * Set video decoder factory in the ctor of the config class. * Prepare SetRecvParameters for not needing RecreateWebRtcVideoStream. Bug: none Change-Id: I48fbf2920c9fe50f3995ceab5667eb2f70618f25 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/223067 Reviewed-by: Niels Moller Commit-Queue: Tommi Cr-Commit-Position: refs/heads/master@{#34351} --- call/video_receive_stream.cc | 6 +++-- call/video_receive_stream.h | 3 ++- media/engine/webrtc_video_engine.cc | 32 ++++++++++++++----------- media/engine/webrtc_video_engine.h | 3 --- video/video_receive_stream2_unittest.cc | 20 +++++++--------- video/video_receive_stream_unittest.cc | 12 ++++------ 6 files changed, 37 insertions(+), 39 deletions(-) diff --git a/call/video_receive_stream.cc b/call/video_receive_stream.cc index e0f3de366b..0b95d66767 100644 --- a/call/video_receive_stream.cc +++ b/call/video_receive_stream.cc @@ -74,8 +74,10 @@ std::string VideoReceiveStream::Stats::ToString(int64_t time_ms) const { VideoReceiveStream::Config::Config(const Config&) = default; VideoReceiveStream::Config::Config(Config&&) = default; -VideoReceiveStream::Config::Config(Transport* rtcp_send_transport) - : rtcp_send_transport(rtcp_send_transport) {} +VideoReceiveStream::Config::Config(Transport* rtcp_send_transport, + VideoDecoderFactory* decoder_factory) + : decoder_factory(decoder_factory), + rtcp_send_transport(rtcp_send_transport) {} VideoReceiveStream::Config& VideoReceiveStream::Config::operator=(Config&&) = default; diff --git a/call/video_receive_stream.h b/call/video_receive_stream.h index f0112e6f7b..61edc886cb 100644 --- a/call/video_receive_stream.h +++ b/call/video_receive_stream.h @@ -150,7 +150,8 @@ class VideoReceiveStream : public MediaReceiveStream { public: Config() = delete; Config(Config&&); - explicit Config(Transport* rtcp_send_transport); + Config(Transport* rtcp_send_transport, + VideoDecoderFactory* decoder_factory = nullptr); Config& operator=(Config&&); Config& operator=(const Config&) = delete; ~Config(); diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 3bf081440e..897aa77ac5 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -1468,7 +1468,7 @@ bool WebRtcVideoChannel::AddRecvStream(const StreamParams& sp, for (uint32_t used_ssrc : sp.ssrcs) receive_ssrcs_.insert(used_ssrc); - webrtc::VideoReceiveStream::Config config(this); + webrtc::VideoReceiveStream::Config config(this, decoder_factory_); webrtc::FlexfecReceiveStream::Config flexfec_config(this); ConfigureReceiverRtp(&config, &flexfec_config, sp); @@ -1483,8 +1483,8 @@ bool WebRtcVideoChannel::AddRecvStream(const StreamParams& sp, 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); + this, call_, sp, std::move(config), default_stream, recv_codecs_, + flexfec_config); return true; } @@ -2819,7 +2819,6 @@ WebRtcVideoChannel::WebRtcVideoReceiveStream::WebRtcVideoReceiveStream( webrtc::Call* call, const StreamParams& sp, webrtc::VideoReceiveStream::Config config, - webrtc::VideoDecoderFactory* decoder_factory, bool default_stream, const std::vector& recv_codecs, const webrtc::FlexfecReceiveStream::Config& flexfec_config) @@ -2831,10 +2830,10 @@ WebRtcVideoChannel::WebRtcVideoReceiveStream::WebRtcVideoReceiveStream( config_(std::move(config)), flexfec_config_(flexfec_config), flexfec_stream_(nullptr), - decoder_factory_(decoder_factory), sink_(NULL), first_frame_timestamp_(-1), estimated_remote_start_ntp_time_ms_(0) { + RTC_DCHECK(config_.decoder_factory); config_.renderer = this; ConfigureCodecs(recv_codecs); flexfec_config_.payload_type = flexfec_config.payload_type; @@ -2879,16 +2878,12 @@ WebRtcVideoChannel::WebRtcVideoReceiveStream::GetRtpParameters() const { void WebRtcVideoChannel::WebRtcVideoReceiveStream::ConfigureCodecs( const std::vector& recv_codecs) { RTC_DCHECK(!recv_codecs.empty()); + config_.decoders.clear(); config_.rtp.rtx_associated_payload_types.clear(); config_.rtp.raw_payload_types.clear(); - config_.decoder_factory = decoder_factory_; for (const auto& recv_codec : recv_codecs) { - webrtc::SdpVideoFormat video_format(recv_codec.codec.name, - recv_codec.codec.params); - webrtc::VideoReceiveStream::Decoder decoder; - decoder.video_format = video_format; decoder.payload_type = recv_codec.codec.id; decoder.video_format = webrtc::SdpVideoFormat(recv_codec.codec.name, recv_codec.codec.params); @@ -2981,10 +2976,18 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetRecvParameters( ConfigureCodecs(*params.codec_settings); video_needs_recreation = true; } + if (params.rtp_header_extensions) { - config_.rtp.extensions = *params.rtp_header_extensions; - flexfec_config_.rtp.extensions = *params.rtp_header_extensions; - video_needs_recreation = true; + if (config_.rtp.extensions != *params.rtp_header_extensions) { + config_.rtp.extensions = *params.rtp_header_extensions; + video_needs_recreation = true; + } + + if (flexfec_config_.rtp.extensions != *params.rtp_header_extensions) { + flexfec_config_.rtp.extensions = *params.rtp_header_extensions; + if (flexfec_stream_ || flexfec_config_.IsCompleteAndEnabled()) + video_needs_recreation = true; + } } if (params.flexfec_payload_type) { flexfec_config_.payload_type = *params.flexfec_payload_type; @@ -2992,7 +2995,8 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetRecvParameters( // configured and instead of recreating the video stream, reconfigure the // flexfec object from within the rtp callback (soon to be on the network // thread). - video_needs_recreation = true; + if (flexfec_stream_ || flexfec_config_.IsCompleteAndEnabled()) + video_needs_recreation = true; } if (video_needs_recreation) { RecreateWebRtcVideoStream(); diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index cd6fbe53fa..e79ebbf24a 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -436,7 +436,6 @@ class WebRtcVideoChannel : public VideoMediaChannel, webrtc::Call* call, const StreamParams& sp, webrtc::VideoReceiveStream::Config config, - webrtc::VideoDecoderFactory* decoder_factory, bool default_stream, const std::vector& recv_codecs, const webrtc::FlexfecReceiveStream::Config& flexfec_config); @@ -501,8 +500,6 @@ class WebRtcVideoChannel : public VideoMediaChannel, webrtc::FlexfecReceiveStream::Config flexfec_config_; webrtc::FlexfecReceiveStream* flexfec_stream_; - webrtc::VideoDecoderFactory* const decoder_factory_; - webrtc::Mutex sink_lock_; rtc::VideoSinkInterface* sink_ RTC_GUARDED_BY(sink_lock_); diff --git a/video/video_receive_stream2_unittest.cc b/video/video_receive_stream2_unittest.cc index f3372b1c3e..a37a5defb2 100644 --- a/video/video_receive_stream2_unittest.cc +++ b/video/video_receive_stream2_unittest.cc @@ -111,9 +111,9 @@ class VideoReceiveStream2Test : public ::testing::Test { VideoReceiveStream2Test() : process_thread_(ProcessThread::Create("TestThread")), task_queue_factory_(CreateDefaultTaskQueueFactory()), - config_(&mock_transport_), - call_stats_(Clock::GetRealTimeClock(), loop_.task_queue()), - h264_decoder_factory_(&mock_h264_video_decoder_) {} + h264_decoder_factory_(&mock_h264_video_decoder_), + config_(&mock_transport_, &h264_decoder_factory_), + call_stats_(Clock::GetRealTimeClock(), loop_.task_queue()) {} ~VideoReceiveStream2Test() override { if (video_receive_stream_) video_receive_stream_->UnregisterFromTransport(); @@ -124,7 +124,6 @@ class VideoReceiveStream2Test : public ::testing::Test { config_.rtp.remote_ssrc = 1111; config_.rtp.local_ssrc = 2222; config_.renderer = &fake_renderer_; - config_.decoder_factory = &h264_decoder_factory_; VideoReceiveStream::Decoder h264_decoder; h264_decoder.payload_type = 99; h264_decoder.video_format = SdpVideoFormat("H264"); @@ -149,10 +148,10 @@ class VideoReceiveStream2Test : public ::testing::Test { test::RunLoop loop_; std::unique_ptr process_thread_; const std::unique_ptr task_queue_factory_; + test::VideoDecoderProxyFactory h264_decoder_factory_; VideoReceiveStream::Config config_; internal::CallStats call_stats_; MockVideoDecoder mock_h264_video_decoder_; - test::VideoDecoderProxyFactory h264_decoder_factory_; cricket::FakeVideoRenderer fake_renderer_; cricket::FakeCall fake_call_; MockTransport mock_transport_; @@ -293,7 +292,7 @@ class VideoReceiveStream2TestWithFakeDecoder : public ::testing::Test { []() { return std::make_unique(); }), process_thread_(ProcessThread::Create("TestThread")), task_queue_factory_(CreateDefaultTaskQueueFactory()), - config_(&mock_transport_), + config_(&mock_transport_, &fake_decoder_factory_), call_stats_(Clock::GetRealTimeClock(), loop_.task_queue()) {} ~VideoReceiveStream2TestWithFakeDecoder() override { if (video_receive_stream_) @@ -304,7 +303,6 @@ class VideoReceiveStream2TestWithFakeDecoder : public ::testing::Test { config_.rtp.remote_ssrc = 1111; config_.rtp.local_ssrc = 2222; config_.renderer = &fake_renderer_; - config_.decoder_factory = &fake_decoder_factory_; VideoReceiveStream::Decoder fake_decoder; fake_decoder.payload_type = 99; fake_decoder.video_format = SdpVideoFormat("VP8"); @@ -561,12 +559,11 @@ class VideoReceiveStream2TestWithSimulatedClock Transport* transport, VideoDecoderFactory* decoder_factory, rtc::VideoSinkInterface* renderer) { - VideoReceiveStream::Config config(transport); + VideoReceiveStream::Config config(transport, decoder_factory); config.rtp.remote_ssrc = 1111; config.rtp.local_ssrc = 2222; config.rtp.nack.rtp_history_ms = GetParam(); // rtx-time. config.renderer = renderer; - config.decoder_factory = decoder_factory; VideoReceiveStream::Decoder fake_decoder; fake_decoder.payload_type = 99; fake_decoder.video_format = SdpVideoFormat("VP8"); @@ -734,7 +731,7 @@ class VideoReceiveStream2TestWithLazyDecoderCreation : public ::testing::Test { VideoReceiveStream2TestWithLazyDecoderCreation() : process_thread_(ProcessThread::Create("TestThread")), task_queue_factory_(CreateDefaultTaskQueueFactory()), - config_(&mock_transport_), + config_(&mock_transport_, &mock_h264_decoder_factory_), call_stats_(Clock::GetRealTimeClock(), loop_.task_queue()) {} ~VideoReceiveStream2TestWithLazyDecoderCreation() override { @@ -748,7 +745,6 @@ class VideoReceiveStream2TestWithLazyDecoderCreation : public ::testing::Test { config_.rtp.remote_ssrc = 1111; config_.rtp.local_ssrc = 2222; config_.renderer = &fake_renderer_; - config_.decoder_factory = &mock_h264_decoder_factory_; VideoReceiveStream::Decoder h264_decoder; h264_decoder.payload_type = 99; h264_decoder.video_format = SdpVideoFormat("H264"); @@ -773,10 +769,10 @@ class VideoReceiveStream2TestWithLazyDecoderCreation : public ::testing::Test { test::RunLoop loop_; std::unique_ptr process_thread_; const std::unique_ptr task_queue_factory_; + MockVideoDecoderFactory mock_h264_decoder_factory_; VideoReceiveStream::Config config_; internal::CallStats call_stats_; MockVideoDecoder mock_h264_video_decoder_; - MockVideoDecoderFactory mock_h264_decoder_factory_; cricket::FakeVideoRenderer fake_renderer_; cricket::FakeCall fake_call_; MockTransport mock_transport_; diff --git a/video/video_receive_stream_unittest.cc b/video/video_receive_stream_unittest.cc index c320bfa569..cb14f7dc06 100644 --- a/video/video_receive_stream_unittest.cc +++ b/video/video_receive_stream_unittest.cc @@ -96,16 +96,15 @@ class VideoReceiveStreamTest : public ::testing::Test { VideoReceiveStreamTest() : process_thread_(ProcessThread::Create("TestThread")), task_queue_factory_(CreateDefaultTaskQueueFactory()), - config_(&mock_transport_), - call_stats_(Clock::GetRealTimeClock(), process_thread_.get()), - h264_decoder_factory_(&mock_h264_video_decoder_) {} + h264_decoder_factory_(&mock_h264_video_decoder_), + config_(&mock_transport_, &h264_decoder_factory_), + call_stats_(Clock::GetRealTimeClock(), process_thread_.get()) {} void SetUp() { constexpr int kDefaultNumCpuCores = 2; config_.rtp.remote_ssrc = 1111; config_.rtp.local_ssrc = 2222; config_.renderer = &fake_renderer_; - config_.decoder_factory = &h264_decoder_factory_; VideoReceiveStream::Decoder h264_decoder; h264_decoder.payload_type = 99; h264_decoder.video_format = SdpVideoFormat("H264"); @@ -126,10 +125,10 @@ class VideoReceiveStreamTest : public ::testing::Test { protected: std::unique_ptr process_thread_; const std::unique_ptr task_queue_factory_; + test::VideoDecoderProxyFactory h264_decoder_factory_; VideoReceiveStream::Config config_; CallStats call_stats_; MockVideoDecoder mock_h264_video_decoder_; - test::VideoDecoderProxyFactory h264_decoder_factory_; cricket::FakeVideoRenderer fake_renderer_; MockTransport mock_transport_; PacketRouter packet_router_; @@ -235,14 +234,13 @@ class VideoReceiveStreamTestWithFakeDecoder : public ::testing::Test { []() { return std::make_unique(); }), process_thread_(ProcessThread::Create("TestThread")), task_queue_factory_(CreateDefaultTaskQueueFactory()), - config_(&mock_transport_), + config_(&mock_transport_, &fake_decoder_factory_), call_stats_(Clock::GetRealTimeClock(), process_thread_.get()) {} void SetUp() { config_.rtp.remote_ssrc = 1111; config_.rtp.local_ssrc = 2222; config_.renderer = &fake_renderer_; - config_.decoder_factory = &fake_decoder_factory_; VideoReceiveStream::Decoder fake_decoder; fake_decoder.payload_type = 99; fake_decoder.video_format = SdpVideoFormat("VP8");