diff --git a/test/video_encoder_proxy_factory.h b/test/video_encoder_proxy_factory.h index d09e6ff694..ac19e5238a 100644 --- a/test/video_encoder_proxy_factory.h +++ b/test/video_encoder_proxy_factory.h @@ -30,7 +30,10 @@ const VideoEncoder::Capabilities kCapabilities(false); // a proxy for the same encoder, typically an instance of FakeEncoder. class VideoEncoderProxyFactory final : public VideoEncoderFactory { public: - explicit VideoEncoderProxyFactory(VideoEncoder* encoder) : encoder_(encoder) { + explicit VideoEncoderProxyFactory(VideoEncoder* encoder) + : encoder_(encoder), + num_simultaneous_encoder_instances_(0), + max_num_simultaneous_encoder_instances_(0) { codec_info_.is_hardware_accelerated = false; codec_info_.has_internal_source = false; } @@ -47,7 +50,11 @@ class VideoEncoderProxyFactory final : public VideoEncoderFactory { std::unique_ptr CreateVideoEncoder( const SdpVideoFormat& format) override { - return absl::make_unique(encoder_); + ++num_simultaneous_encoder_instances_; + max_num_simultaneous_encoder_instances_ = + std::max(max_num_simultaneous_encoder_instances_, + num_simultaneous_encoder_instances_); + return absl::make_unique(encoder_, this); } void SetIsHardwareAccelerated(bool is_hardware_accelerated) { @@ -57,12 +64,24 @@ class VideoEncoderProxyFactory final : public VideoEncoderFactory { codec_info_.has_internal_source = has_internal_source; } + int GetMaxNumberOfSimultaneousEncoderInstances() { + return max_num_simultaneous_encoder_instances_; + } + private: + void OnDestroyVideoEncoder() { + RTC_CHECK_GT(num_simultaneous_encoder_instances_, 0); + --num_simultaneous_encoder_instances_; + } + // Wrapper class, since CreateVideoEncoder needs to surrender // ownership to the object it returns. class EncoderProxy final : public VideoEncoder { public: - explicit EncoderProxy(VideoEncoder* encoder) : encoder_(encoder) {} + explicit EncoderProxy(VideoEncoder* encoder, + VideoEncoderProxyFactory* encoder_factory) + : encoder_(encoder), encoder_factory_(encoder_factory) {} + ~EncoderProxy() { encoder_factory_->OnDestroyVideoEncoder(); } private: void SetFecControllerOverride( @@ -96,10 +115,14 @@ class VideoEncoderProxyFactory final : public VideoEncoderFactory { } VideoEncoder* const encoder_; + VideoEncoderProxyFactory* const encoder_factory_; }; VideoEncoder* const encoder_; CodecInfo codec_info_; + + int num_simultaneous_encoder_instances_; + int max_num_simultaneous_encoder_instances_; }; } // namespace test diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 9b2f0f291b..3dba91e107 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -754,6 +754,11 @@ void VideoStreamEncoder::ReconfigureEncoder() { if (pending_encoder_creation_ || reset_required) { ReleaseEncoder(); if (pending_encoder_creation_) { + // Destroy existing encoder instance before creating a new one. Otherwise + // attempt to create another instance will fail if encoder factory + // supports only single encoder instance. + encoder_.reset(); + encoder_ = settings_.encoder_factory->CreateVideoEncoder( encoder_config_.video_format); // TODO(nisse): What to do if creating the encoder fails? Crash, diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index c973e87571..b80acc239b 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -1264,6 +1264,29 @@ TEST_F(VideoStreamEncoderTest, FrameResolutionChangeReconfigureEncoder) { video_stream_encoder_->Stop(); } +TEST_F(VideoStreamEncoderTest, + EncoderInstanceDestroyedBeforeAnotherInstanceCreated) { + video_stream_encoder_->OnBitrateUpdated( + DataRate::bps(kTargetBitrateBps), DataRate::bps(kTargetBitrateBps), 0, 0); + + // Capture a frame and wait for it to synchronize with the encoder thread. + video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); + WaitForEncodedFrame(1); + + VideoEncoderConfig video_encoder_config; + test::FillEncoderConfiguration(kVideoCodecVP8, 1, &video_encoder_config); + // Changing the max payload data length recreates encoder. + video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config), + kMaxPayloadLength / 2); + + // Capture a frame and wait for it to synchronize with the encoder thread. + video_source_.IncomingCapturedFrame(CreateFrame(2, nullptr)); + WaitForEncodedFrame(2); + EXPECT_EQ(1, encoder_factory_.GetMaxNumberOfSimultaneousEncoderInstances()); + + video_stream_encoder_->Stop(); +} + TEST_F(VideoStreamEncoderTest, BitrateLimitsChangeReconfigureRateAllocator) { video_stream_encoder_->OnBitrateUpdated( DataRate::bps(kTargetBitrateBps), DataRate::bps(kTargetBitrateBps), 0, 0);