diff --git a/experiments/field_trials.py b/experiments/field_trials.py index 60ecc46455..6d8e83c291 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_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 911a34bde0..9a1ef8599a 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -2575,6 +2575,38 @@ 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); + + SendImpl()->RequestEncoderFallback(); + time_controller_.AdvanceTime(kFrameDuration); + + webrtc::test::FrameForwarder frame_forwarder; + EXPECT_TRUE(send_channel_->SetVideoSend(kSsrc, nullptr, &frame_forwarder)); + EXPECT_TRUE(send_channel_->RemoveSendStream(kSsrc)); +} + #if defined(RTC_ENABLE_VP9) TEST_F(WebRtcVideoChannelBaseTest, RequestEncoderFallback) { diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 5f5c3a9e43..4764308ed3 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; @@ -1483,15 +1487,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( @@ -1894,11 +1904,14 @@ 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 we run out of codecs to be + // negotiated, we don't continue to encode frames. The send streams will still + // be kept. Otherwise if WebRtcVideoEngine responds to the fallback request, + // the send streams will be 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 c26934ae14..a0c9eabd24 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -94,6 +94,7 @@ #include "rtc_base/event.h" #include "rtc_base/experiments/encoder_info_settings.h" #include "rtc_base/experiments/rate_control_settings.h" +#include "rtc_base/gunit.h" #include "rtc_base/logging.h" #include "rtc_base/ref_counted_object.h" #include "rtc_base/strings/string_builder.h" @@ -1273,6 +1274,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) { @@ -1295,6 +1301,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); @@ -1426,6 +1433,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 @@ -8253,9 +8261,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)); @@ -8323,6 +8329,115 @@ 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)); + } + + ASSERT_THAT(WaitUntil([&] { return fake_encoder_.GetNumEncodes() == 0; }, + ::testing::IsTrue()), + IsRtcOk()); + + // 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;