Fix to not crash when VideoEncoderFactory fails to create encoder

Bug: webrtc:13082
Change-Id: I5f1cfa7db6259e71ce3fc18281a3d084c32911ea
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/257923
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Daniel.L (Byoungchan) Lee <daniel.l@hpcnt.com>
Cr-Commit-Position: refs/heads/main@{#36501}

test/BUILD.gn

@@ -865,6 +865,7 @@ rtc_library("test_common") {
     "run_loop.cc",
     "run_loop.h",
     "video_decoder_proxy_factory.h",
+    "video_encoder_nullable_proxy_factory.h",
     "video_encoder_proxy_factory.h",
   ]
 

test/video_encoder_nullable_proxy_factory.h

@@ -0,0 +1,45 @@
+/*
+ *  Copyright (c) 2022 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef TEST_VIDEO_ENCODER_NULLABLE_PROXY_FACTORY_H_
+#define TEST_VIDEO_ENCODER_NULLABLE_PROXY_FACTORY_H_
+
+#include <memory>
+#include <vector>
+
+#include "api/video_codecs/video_encoder.h"
+#include "api/video_codecs/video_encoder_factory.h"
+#include "test/video_encoder_proxy_factory.h"
+
+namespace webrtc {
+namespace test {
+
+class VideoEncoderNullableProxyFactory final : public VideoEncoderProxyFactory {
+ public:
+  explicit VideoEncoderNullableProxyFactory(
+      VideoEncoder* encoder,
+      EncoderSelectorInterface* encoder_selector)
+      : VideoEncoderProxyFactory(encoder, encoder_selector) {}
+
+  ~VideoEncoderNullableProxyFactory() override = default;
+
+  std::unique_ptr<VideoEncoder> CreateVideoEncoder(
+      const SdpVideoFormat& format) override {
+    if (!encoder_) {
+      return nullptr;
+    }
+    return VideoEncoderProxyFactory::CreateVideoEncoder(format);
+  }
+};
+
+}  // namespace test
+}  // namespace webrtc
+
+#endif  // TEST_VIDEO_ENCODER_NULLABLE_PROXY_FACTORY_H_

test/video_encoder_proxy_factory.h

@@ -27,7 +27,7 @@ const VideoEncoder::Capabilities kCapabilities(false);
 // An encoder factory with a single underlying VideoEncoder object,
 // intended for test purposes. Each call to CreateVideoEncoder returns
 // a proxy for the same encoder, typically an instance of FakeEncoder.
-class VideoEncoderProxyFactory final : public VideoEncoderFactory {
+class VideoEncoderProxyFactory : public VideoEncoderFactory {
  public:
   explicit VideoEncoderProxyFactory(VideoEncoder* encoder)
       : VideoEncoderProxyFactory(encoder, nullptr) {}
@@ -68,7 +68,7 @@ class VideoEncoderProxyFactory final : public VideoEncoderFactory {
     return max_num_simultaneous_encoder_instances_;
   }
 
- private:
+ protected:
   void OnDestroyVideoEncoder() {
     RTC_CHECK_GT(num_simultaneous_encoder_instances_, 0);
     --num_simultaneous_encoder_instances_;

video/video_stream_encoder.cc

@@ -903,9 +903,12 @@ void VideoStreamEncoder::ReconfigureEncoder() {
 
     encoder_ = settings_.encoder_factory->CreateVideoEncoder(
         encoder_config_.video_format);
-    // TODO(nisse): What to do if creating the encoder fails? Crash,
-    // or just discard incoming frames?
-    RTC_CHECK(encoder_);
+    if (!encoder_) {
+      RTC_LOG(LS_ERROR) << "CreateVideoEncoder failed, failing encoder format: "
+                        << encoder_config_.video_format.ToString();
+      RequestEncoderSwitch();
+      return;
+    }
 
     if (encoder_selector_) {
       encoder_selector_->OnCurrentEncoder(encoder_config_.video_format);
@@ -2171,7 +2174,7 @@ void VideoStreamEncoder::OnBitrateUpdated(DataRate target_bitrate,
 }
 
 bool VideoStreamEncoder::DropDueToSize(uint32_t pixel_count) const {
-  if (!stream_resource_manager_.DropInitialFrames() ||
+  if (!encoder_ || !stream_resource_manager_.DropInitialFrames() ||
       !encoder_target_bitrate_bps_.has_value()) {
     return false;
   }

video/video_stream_encoder_unittest.cc

@@ -65,6 +65,7 @@
 #include "test/mappable_native_buffer.h"
 #include "test/scoped_key_value_config.h"
 #include "test/time_controller/simulated_time_controller.h"
+#include "test/video_encoder_nullable_proxy_factory.h"
 #include "test/video_encoder_proxy_factory.h"
 #include "video/frame_cadence_adapter.h"
 #include "video/send_statistics_proxy.h"
@@ -7701,6 +7702,57 @@ TEST_F(VideoStreamEncoderTest,
   video_stream_encoder_.reset();
 }
 
+TEST_F(VideoStreamEncoderTest, NullEncoderReturnSwitch) {
+  // As a variant of EncoderSelectorBrokenEncoderSwitch, when a null
+  // VideoEncoder is passed in encoder_factory, it checks whether
+  // Codec Switch occurs without a crash.
+  constexpr int kSufficientBitrateToNotDrop = 1000;
+  constexpr int kDontCare = 100;
+
+  NiceMock<MockEncoderSelector> encoder_selector;
+  StrictMock<MockEncoderSwitchRequestCallback> switch_callback;
+  video_send_config_.encoder_settings.encoder_switch_request_callback =
+      &switch_callback;
+  auto encoder_factory =
+      std::make_unique<test::VideoEncoderNullableProxyFactory>(
+          /*encoder=*/nullptr, &encoder_selector);
+  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);
+  ON_CALL(encoder_selector, OnEncoderBroken)
+      .WillByDefault(Return(SdpVideoFormat("AV2")));
+  rtc::Event encode_attempted;
+  EXPECT_CALL(switch_callback,
+              RequestEncoderSwitch(Field(&SdpVideoFormat::name, "AV2"),
+                                   /*allow_default_fallback=*/_))
+      .WillOnce([&encode_attempted]() { encode_attempted.Set(); });
+
+  video_source_.IncomingCapturedFrame(CreateFrame(1, kDontCare, kDontCare));
+  encode_attempted.Wait(3000);
+
+  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,
        AllocationPropagatedToEncoderWhenTargetRateChanged) {
   const int kFrameWidth = 320;