Destroy existing encoder instance before creating a new one.

Before this change, an attempt to recreate video encoder would fail if
video encoder factory supports only single instance of an encoder.

Added tracking of max number of existed simultaneously encoder
instances to VideoEncoderProxyFactory.

Bug: webrtc:10776
Change-Id: I317cbdf1af94dfb4c72bf99c5cd4ce7b454188fa
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144044
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28457}

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<VideoEncoder> CreateVideoEncoder(
       const SdpVideoFormat& format) override {
-    return absl::make_unique<EncoderProxy>(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<EncoderProxy>(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

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,

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