diff --git a/test/BUILD.gn b/test/BUILD.gn index 953d7746a0..e2c9050703 100644 --- a/test/BUILD.gn +++ b/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", ] diff --git a/test/video_encoder_nullable_proxy_factory.h b/test/video_encoder_nullable_proxy_factory.h new file mode 100644 index 0000000000..da81fff343 --- /dev/null +++ b/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 +#include + +#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 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_ diff --git a/test/video_encoder_proxy_factory.h b/test/video_encoder_proxy_factory.h index ffa8efcf1d..ba7d1e3fde 100644 --- a/test/video_encoder_proxy_factory.h +++ b/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_; diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index f21801330d..916819ed33 100644 --- a/video/video_stream_encoder.cc +++ b/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; } diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index a77c386265..dc164e7b61 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/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 encoder_selector; + StrictMock switch_callback; + video_send_config_.encoder_settings.encoder_switch_request_callback = + &switch_callback; + auto encoder_factory = + std::make_unique( + /*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;