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 <kron@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38232}
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index e7e43fc..d72403b 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -1076,13 +1076,13 @@
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 @@
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 @@
return std::make_unique<FakeWebRtcVideoEncoder>(nullptr);
});
- // Mock decoder creation. `engine` take ownership of the decoder.
- EXPECT_CALL(*decoder_factory, CreateVideoDecoder(format)).WillOnce([] {
- return std::make_unique<FakeWebRtcVideoDecoder>(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 @@
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<webrtc::MockVideoBitrateAllocatorFactory>
- rate_allocator_factory =
- std::make_unique<webrtc::MockVideoBitrateAllocatorFactory>();
- webrtc::FieldTrialBasedConfig trials;
- WebRtcVideoEngine engine(
- (std::unique_ptr<webrtc::VideoEncoderFactory>(encoder_factory)),
- (std::unique_ptr<webrtc::VideoDecoderFactory>(decoder_factory)), trials);
- const webrtc::SdpVideoFormat vp8_format("VP8");
- const std::vector<webrtc::SdpVideoFormat> 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<VideoMediaChannel> 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<FakeCall> fake_call(new FakeCall());
diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc
index 647f4e5..33d714b 100644
--- a/video/video_receive_stream2.cc
+++ b/video/video_receive_stream2.cc
@@ -66,9 +66,9 @@
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 a05524f..4a0f777 100644
--- a/video/video_receive_stream2_unittest.cc
+++ b/video/video_receive_stream2_unittest.cc
@@ -485,19 +485,15 @@
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"))));
