Follow codec preference order for sending codec fallback.

When encoder selector is not enabled, currently we always fallback to
VP8 no matter how the codec preference is setup. Update to follow codec
preference order for the fallback.

Bug: chromium:378566918
Change-Id: Ia3fbfc9d407683ef7b3d6246af7e9ec58535dc89
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370707
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Jianlin Qiu <jianlin.qiu@intel.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43566}

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)),

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;
   }
 

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> 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) {

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<SdpVideoFormat> 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.

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<VideoStreamAdapter> video_stream_adapter_
       RTC_GUARDED_BY(encoder_queue_);
   // Responsible for adapting input resolution or frame rate to ensure resources

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<VideoFrameBuffer::Type, kMaxPreferredPixelFormats>
             pixel_formats) {
@@ -1259,6 +1264,7 @@ class VideoStreamEncoderTest : public ::testing::Test {
                    const std::vector<VideoFrameType>* 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<ResolutionBitrateLimits> 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<VideoFrameBuffer::Type> last_input_pixel_format_
         RTC_GUARDED_BY(local_mutex_);
     absl::InlinedVector<VideoFrameBuffer::Type, kMaxPreferredPixelFormats>
@@ -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<MockVideoEncoder> video_encoder;
+  StrictMock<MockEncoderSwitchRequestCallback> switch_callback;
+  video_send_config_.encoder_settings.encoder_switch_request_callback =
+      &switch_callback;
+  auto encoder_factory = std::make_unique<test::VideoEncoderProxyFactory>(
+      &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<MockVideoEncoder> video_encoder;
+  StrictMock<MockEncoderSwitchRequestCallback> switch_callback;
+  video_send_config_.encoder_settings.encoder_switch_request_callback =
+      &switch_callback;
+  auto encoder_factory = std::make_unique<test::VideoEncoderProxyFactory>(
+      &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;