diff --git a/experiments/field_trials.py b/experiments/field_trials.py index 3eea5853ec..0070768ca3 100755 --- a/experiments/field_trials.py +++ b/experiments/field_trials.py @@ -152,6 +152,9 @@ ACTIVE_FIELD_TRIALS: FrozenSet[FieldTrial] = frozenset([ FieldTrial('WebRTC-SrtpRemoveReceiveStream', 42225949, date(2024, 10, 1)), + FieldTrial('WebRTC-SwitchEncoderFollowCodecPreferenceOrder', + 378566918, + date(2025, 5, 1)), FieldTrial('WebRTC-TaskQueue-ReplaceLibeventWithStdlib', 42224654, date(2024, 4, 1)), diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 8822022103..527550a345 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -1233,6 +1233,10 @@ void WebRtcVideoSendChannel::RequestEncoderFallback() { RTC_DCHECK_RUN_ON(&thread_checker_); if (negotiated_codecs_.size() <= 1) { RTC_LOG(LS_WARNING) << "Encoder failed but no fallback codec is available"; + // Remove all the send streams since we cannot encode any more. + while (!send_streams_.empty()) { + RemoveSendStream(send_streams_.begin()->first); + } return; } diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 91249da4bd..a541532b3c 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -2547,6 +2547,31 @@ TEST_F(WebRtcVideoChannelBaseTest, TwoStreamsSendAndReceive) { TwoStreamsSendAndReceive(codec); } +TEST_F(WebRtcVideoChannelBaseTest, + RequestEncoderFallbackNextCodecFollowNegotiatedOrder) { + cricket::VideoSenderParameters parameters; + parameters.codecs.push_back(GetEngineCodec("VP9")); + parameters.codecs.push_back(GetEngineCodec("AV1")); + parameters.codecs.push_back(GetEngineCodec("VP8")); + EXPECT_TRUE(send_channel_->SetSenderParameters(parameters)); + + std::optional codec = send_channel_->GetSendCodec(); + ASSERT_TRUE(codec); + EXPECT_EQ("VP9", codec->name); + + SendImpl()->RequestEncoderFallback(); + time_controller_.AdvanceTime(kFrameDuration); + codec = send_channel_->GetSendCodec(); + ASSERT_TRUE(codec); + EXPECT_EQ("AV1", codec->name); + + SendImpl()->RequestEncoderFallback(); + time_controller_.AdvanceTime(kFrameDuration); + codec = send_channel_->GetSendCodec(); + ASSERT_TRUE(codec); + EXPECT_EQ("VP8", codec->name); +} + #if defined(RTC_ENABLE_VP9) TEST_F(WebRtcVideoChannelBaseTest, RequestEncoderFallback) { diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 8620c621dc..e527c5a92b 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -80,6 +80,10 @@ constexpr char kFrameDropperFieldTrial[] = "WebRTC-FrameDropper"; constexpr char kSwitchEncoderOnInitializationFailuresFieldTrial[] = "WebRTC-SwitchEncoderOnInitializationFailures"; +// TODO(crbugs.com/378566918): Remove this kill switch after rollout. +constexpr char kSwitchEncoderFollowCodecPreferenceOrderFieldTrial[] = + "WebRTC-SwitchEncoderFollowCodecPreferenceOrder"; + const size_t kDefaultPayloadSize = 1440; const int64_t kParameterUpdateIntervalMs = 1000; @@ -1481,15 +1485,21 @@ void VideoStreamEncoder::RequestEncoderSwitch() { } // If encoder selector is available, switch to the encoder it prefers. - // Otherwise try switching to VP8 (default WebRTC codec). std::optional preferred_fallback_encoder; if (is_encoder_selector_available) { preferred_fallback_encoder = encoder_selector_->OnEncoderBroken(); } if (!preferred_fallback_encoder) { - preferred_fallback_encoder = - SdpVideoFormat(CodecTypeToPayloadString(kVideoCodecVP8)); + if (!env_.field_trials().IsDisabled( + kSwitchEncoderFollowCodecPreferenceOrderFieldTrial)) { + encoder_fallback_requested_ = true; + settings_.encoder_switch_request_callback->RequestEncoderFallback(); + return; + } else { + preferred_fallback_encoder = + SdpVideoFormat(CodecTypeToPayloadString(kVideoCodecVP8)); + } } settings_.encoder_switch_request_callback->RequestEncoderSwitch( @@ -1892,11 +1902,15 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame, RTC_LOG(LS_VERBOSE) << __func__ << " posted " << time_when_posted_us << " ntp time " << video_frame.ntp_time_ms(); - // If the encoder fail we can't continue to encode frames. When this happens - // the WebrtcVideoSender is notified and the whole VideoSendStream is - // recreated. - if (encoder_failed_ || !encoder_initialized_) + // If encoder fallback is requested, but WebRtcVideoEngine cannot switch codec + // due to negotatied codec list is already exhausted, we don't continue to + // encode frames, and WebRtcVideoEngine will destroy the VideoSendStreams + // along with current VideoStreamEncoder. If the encoder fallback is requested + // and WebRtcVideoEngine responds to the fallback request, the VideoSendStream + // is recreated and current VideoStreamEncoder will no longer be used. + if (encoder_fallback_requested_ || !encoder_initialized_) { return; + } // It's possible that EncodeVideoFrame can be called after we've completed // a Stop() operation. Check if the encoder_ is set before continuing. diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 475a9e65b0..363e4dd1c3 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -325,8 +325,6 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, bool was_encode_called_since_last_initialization_ RTC_GUARDED_BY(encoder_queue_) = false; - bool encoder_failed_ RTC_GUARDED_BY(encoder_queue_) = false; - // Used to make sure incoming time stamp is increasing for every frame. int64_t last_captured_timestamp_ RTC_GUARDED_BY(encoder_queue_) = 0; // Delta used for translating between NTP and internal timestamps. @@ -382,6 +380,8 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, // Provides video stream input states: current resolution and frame rate. VideoStreamInputStateProvider input_state_provider_; + bool encoder_fallback_requested_ RTC_GUARDED_BY(encoder_queue_) = false; + const std::unique_ptr video_stream_adapter_ RTC_GUARDED_BY(encoder_queue_); // Responsible for adapting input resolution or frame rate to ensure resources diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index dc977f2e67..a843a81d57 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -1237,6 +1237,11 @@ class VideoStreamEncoderTest : public ::testing::Test { return num_set_rates_; } + int GetNumEncodes() const { + MutexLock lock(&local_mutex_); + return num_encodes_; + } + void SetPreferredPixelFormats( absl::InlinedVector pixel_formats) { @@ -1259,6 +1264,7 @@ class VideoStreamEncoderTest : public ::testing::Test { const std::vector* frame_types) override { { MutexLock lock(&local_mutex_); + num_encodes_++; if (expect_null_frame_) { EXPECT_EQ(input_image.rtp_timestamp(), 0u); EXPECT_EQ(input_image.width(), 1); @@ -1390,6 +1396,7 @@ class VideoStreamEncoderTest : public ::testing::Test { std::vector resolution_bitrate_limits_ RTC_GUARDED_BY(local_mutex_); int num_set_rates_ RTC_GUARDED_BY(local_mutex_) = 0; + int num_encodes_ RTC_GUARDED_BY(local_mutex_) = 0; std::optional last_input_pixel_format_ RTC_GUARDED_BY(local_mutex_); absl::InlinedVector @@ -8196,9 +8203,7 @@ TEST_F(VideoStreamEncoderTest, .WillByDefault(Return(WEBRTC_VIDEO_CODEC_ENCODER_FAILURE)); rtc::Event encode_attempted; - EXPECT_CALL(switch_callback, - RequestEncoderSwitch(Field(&SdpVideoFormat::name, "VP8"), - /*allow_default_fallback=*/true)) + EXPECT_CALL(switch_callback, RequestEncoderFallback()) .WillOnce([&encode_attempted]() { encode_attempted.Set(); }); video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); @@ -8266,6 +8271,112 @@ TEST_F(VideoStreamEncoderTest, NullEncoderReturnSwitch) { video_stream_encoder_.reset(); } +TEST_F(VideoStreamEncoderTest, NoPreferenceDefaultFallbackToVP8Disabled) { + constexpr int kSufficientBitrateToNotDrop = 1000; + constexpr int kDontCare = 100; + constexpr int kNumFrames = 8; + + NiceMock video_encoder; + StrictMock switch_callback; + video_send_config_.encoder_settings.encoder_switch_request_callback = + &switch_callback; + auto encoder_factory = std::make_unique( + &video_encoder, /*encoder_selector=*/nullptr); + video_send_config_.encoder_settings.encoder_factory = encoder_factory.get(); + + // Reset encoder for new configuration to take effect. + ConfigureEncoder(video_encoder_config_.Copy()); + + // The VideoStreamEncoder needs some bitrate before it can start encoding, + // setting some bitrate so that subsequent calls to WaitForEncodedFrame does + // not fail. + video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources( + /*target_bitrate=*/DataRate::KilobitsPerSec(kSufficientBitrateToNotDrop), + /*stable_target_bitrate=*/ + DataRate::KilobitsPerSec(kSufficientBitrateToNotDrop), + /*link_allocation=*/DataRate::KilobitsPerSec(kSufficientBitrateToNotDrop), + /*fraction_lost=*/0, + /*round_trip_time_ms=*/0, + /*cwnd_reduce_ratio=*/0); + + EXPECT_CALL(video_encoder, Encode) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_ENCODER_FAILURE)); + + EXPECT_CALL(switch_callback, RequestEncoderFallback()); + + VideoFrame frame = CreateFrame(1, kDontCare, kDontCare); + for (int i = 0; i < kNumFrames; ++i) { + int64_t timestamp_ms = CurrentTimeMs(); + frame.set_ntp_time_ms(timestamp_ms); + frame.set_timestamp_us(timestamp_ms * 1000); + video_source_.IncomingCapturedFrame(frame); + time_controller_.AdvanceTime(TimeDelta::Millis(33)); + } + + EXPECT_TRUE_WAIT(fake_encoder_.GetNumEncodes() == 0, 5000); + // After requesting fallback failure, the encoder will be released. + EXPECT_CALL(video_encoder, Release()).Times(1); + + AdvanceTime(TimeDelta::Zero()); + video_stream_encoder_->Stop(); + // The encoders produced by the VideoEncoderProxyFactory have a pointer back + // to it's factory, so in order for the encoder instance in the + // `video_stream_encoder_` to be destroyed before the `encoder_factory` we + // reset the `video_stream_encoder_` here. + video_stream_encoder_.reset(); +} + +TEST_F(VideoStreamEncoderTest, NoPreferenceDefaultFallbackToVP8Enabled) { + constexpr int kSufficientBitrateToNotDrop = 1000; + constexpr int kDontCare = 100; + + webrtc::test::ScopedKeyValueConfig field_trials( + field_trials_, + "WebRTC-SwitchEncoderFollowCodecPreferenceOrder/Disabled/"); + + NiceMock video_encoder; + StrictMock switch_callback; + video_send_config_.encoder_settings.encoder_switch_request_callback = + &switch_callback; + auto encoder_factory = std::make_unique( + &video_encoder, /*encoder_selector=*/nullptr); + video_send_config_.encoder_settings.encoder_factory = encoder_factory.get(); + video_encoder_config_.codec_type = kVideoCodecVP9; + + // Reset encoder for new configuration to take effect. + ConfigureEncoder(video_encoder_config_.Copy()); + + // The VideoStreamEncoder needs some bitrate before it can start encoding, + // setting some bitrate so that subsequent calls to WaitForEncodedFrame does + // not fail. + video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources( + /*target_bitrate=*/DataRate::KilobitsPerSec(kSufficientBitrateToNotDrop), + /*stable_target_bitrate=*/ + DataRate::KilobitsPerSec(kSufficientBitrateToNotDrop), + /*link_allocation=*/DataRate::KilobitsPerSec(kSufficientBitrateToNotDrop), + /*fraction_lost=*/0, + /*round_trip_time_ms=*/0, + /*cwnd_reduce_ratio=*/0); + + EXPECT_CALL(video_encoder, Encode) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_ENCODER_FAILURE)); + + // Fallback request will be asking for switching to VP8. + EXPECT_CALL(switch_callback, + RequestEncoderSwitch(Field(&SdpVideoFormat::name, "VP8"), + /*allow_default_fallback=*/true)); + + VideoFrame frame = CreateFrame(1, kDontCare, kDontCare); + video_source_.IncomingCapturedFrame(frame); + + video_stream_encoder_->Stop(); + // The encoders produced by the VideoEncoderProxyFactory have a pointer back + // to it's factory, so in order for the encoder instance in the + // `video_stream_encoder_` to be destroyed before the `encoder_factory` we + // reset the `video_stream_encoder_` here. + video_stream_encoder_.reset(); +} + TEST_F(VideoStreamEncoderTest, AllocationPropagatedToEncoderWhenTargetRateChanged) { const int kFrameWidth = 320;