From bb591c49e83b8e6dfc492b28146d5f4ffcd08ceb Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Wed, 28 Sep 2022 10:10:25 +0000 Subject: [PATCH] Change the default setting for PreStreamDecoders/LazyDecoderCreation The experiment has been approved for a full launch. Changing the default value so that no decoder is created before the stream starts. All decoders are created lazily on demand when we receive payload data of the corresponding type. Bug: chromium:1319864 Change-Id: Ifb412bbe49a7577a45c340496d5b8572ebc1ba44 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277120 Auto-Submit: Johannes Kron Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/main@{#38232} --- media/engine/webrtc_video_engine_unittest.cc | 68 +++----------------- video/video_receive_stream2.cc | 6 +- video/video_receive_stream2_unittest.cc | 10 +-- 3 files changed, 14 insertions(+), 70 deletions(-) diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index e7e43fcf3f..d72403b599 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -1076,13 +1076,13 @@ TEST_F(WebRtcVideoEngineTest, RegisterDecodersIfSupported) { EXPECT_TRUE( channel->AddRecvStream(cricket::StreamParams::CreateLegacy(kSsrc))); - // Decoder creation happens on the decoder thread, make sure it runs. + // Decoders are not created until they are used. time_controller_.AdvanceTime(webrtc::TimeDelta::Zero()); - ASSERT_EQ(1u, decoder_factory_->decoders().size()); + EXPECT_EQ(0u, decoder_factory_->decoders().size()); // Setting codecs of the same type should not reallocate the decoder. EXPECT_TRUE(channel->SetRecvParameters(parameters)); - EXPECT_EQ(1, decoder_factory_->GetNumCreatedDecoders()); + EXPECT_EQ(0, decoder_factory_->GetNumCreatedDecoders()); // Remove stream previously added to free the external decoder instance. EXPECT_TRUE(channel->RemoveRecvStream(kSsrc)); @@ -1104,9 +1104,9 @@ TEST_F(WebRtcVideoEngineTest, RegisterH264DecoderIfSupported) { EXPECT_TRUE( channel->AddRecvStream(cricket::StreamParams::CreateLegacy(kSsrc))); - // Decoder creation happens on the decoder thread, make sure it runs. + // Decoders are not created until they are used. time_controller_.AdvanceTime(webrtc::TimeDelta::Zero()); - ASSERT_EQ(1u, decoder_factory_->decoders().size()); + ASSERT_EQ(0u, decoder_factory_->decoders().size()); } // Tests when GetSources is called with non-existing ssrc, it will return an @@ -1223,10 +1223,9 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) { return std::make_unique(nullptr); }); - // Mock decoder creation. `engine` take ownership of the decoder. - EXPECT_CALL(*decoder_factory, CreateVideoDecoder(format)).WillOnce([] { - return std::make_unique(nullptr); - }); + // Expect no decoder to be created at this point. The decoder will only be + // created if we receive payload data. + EXPECT_CALL(*decoder_factory, CreateVideoDecoder(format)).Times(0); // Create a call. webrtc::RtcEventLogNull event_log; @@ -1280,57 +1279,6 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) { EXPECT_TRUE(recv_channel->RemoveRecvStream(recv_ssrc)); } -// Test behavior when decoder factory fails to create a decoder (returns null). -TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, NullDecoder) { - rtc::AutoThread main_thread_; - // `engine` take ownership of the factories. - webrtc::MockVideoEncoderFactory* encoder_factory = - new webrtc::MockVideoEncoderFactory(); - webrtc::MockVideoDecoderFactory* decoder_factory = - new webrtc::MockVideoDecoderFactory(); - std::unique_ptr - rate_allocator_factory = - std::make_unique(); - webrtc::FieldTrialBasedConfig trials; - WebRtcVideoEngine engine( - (std::unique_ptr(encoder_factory)), - (std::unique_ptr(decoder_factory)), trials); - const webrtc::SdpVideoFormat vp8_format("VP8"); - const std::vector supported_formats = {vp8_format}; - EXPECT_CALL(*encoder_factory, GetSupportedFormats()) - .WillRepeatedly(Return(supported_formats)); - - // Decoder creation fails. - EXPECT_CALL(*decoder_factory, CreateVideoDecoder).WillOnce([] { - return nullptr; - }); - - // Create a call. - webrtc::RtcEventLogNull event_log; - auto task_queue_factory = webrtc::CreateDefaultTaskQueueFactory(); - webrtc::Call::Config call_config(&event_log); - webrtc::FieldTrialBasedConfig field_trials; - call_config.trials = &field_trials; - call_config.task_queue_factory = task_queue_factory.get(); - const auto call = absl::WrapUnique(webrtc::Call::Create(call_config)); - - // Create recv channel. - EXPECT_CALL(*decoder_factory, GetSupportedFormats()) - .WillRepeatedly(::testing::Return(supported_formats)); - const int recv_ssrc = 321; - std::unique_ptr recv_channel(engine.CreateMediaChannel( - call.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), - rate_allocator_factory.get())); - cricket::VideoRecvParameters recv_parameters; - recv_parameters.codecs.push_back(engine.recv_codecs().front()); - EXPECT_TRUE(recv_channel->SetRecvParameters(recv_parameters)); - EXPECT_TRUE(recv_channel->AddRecvStream( - cricket::StreamParams::CreateLegacy(recv_ssrc))); - - // Remove streams previously added to free the encoder and decoder instance. - EXPECT_TRUE(recv_channel->RemoveRecvStream(recv_ssrc)); -} - TEST_F(WebRtcVideoEngineTest, DISABLED_RecreatesEncoderOnContentTypeChange) { encoder_factory_->AddSupportedVideoCodecType("VP8"); std::unique_ptr fake_call(new FakeCall()); diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index 647f4e580f..33d714b2a4 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -66,9 +66,9 @@ namespace { constexpr TimeDelta kMinBaseMinimumDelay = TimeDelta::Zero(); constexpr TimeDelta kMaxBaseMinimumDelay = TimeDelta::Seconds(10); -// Create a decoder for the preferred codec before the stream starts and any -// other decoder lazily on demand. -constexpr int kDefaultMaximumPreStreamDecoders = 1; +// Create no decoders before the stream starts. All decoders are created on +// demand when we receive payload data of the corresponding type. +constexpr int kDefaultMaximumPreStreamDecoders = 0; // Concrete instance of RecordableEncodedFrame wrapping needed content // from EncodedFrame. diff --git a/video/video_receive_stream2_unittest.cc b/video/video_receive_stream2_unittest.cc index a05524f9e8..4a0f777af2 100644 --- a/video/video_receive_stream2_unittest.cc +++ b/video/video_receive_stream2_unittest.cc @@ -485,19 +485,15 @@ TEST_P(VideoReceiveStream2Test, LazyDecoderCreation) { rtppacket.SetSequenceNumber(1); rtppacket.SetTimestamp(0); - // Only 1 decoder is created by default. It will be H265 since that was the - // first in the decoder list. + // No decoders are created by default. EXPECT_CALL(mock_h264_decoder_factory_, CreateVideoDecoder(_)).Times(0); - EXPECT_CALL( - mock_h264_decoder_factory_, - CreateVideoDecoder(Field(&SdpVideoFormat::name, testing::Eq("H265")))); video_receive_stream_->Start(); - // Decoder creation happens on the decoder thread, make sure it runs. time_controller_.AdvanceTime(TimeDelta::Zero()); EXPECT_TRUE( testing::Mock::VerifyAndClearExpectations(&mock_h264_decoder_factory_)); - + // Verify that the decoder is created when we receive payload data and tries + // to decode a frame. EXPECT_CALL( mock_h264_decoder_factory_, CreateVideoDecoder(Field(&SdpVideoFormat::name, testing::Eq("H264"))));