Revert "Remove VCMEncoderDataBase and put remaining code into VideoStreamEncoder"

This reverts commit 715c4765b1ac20017e6e3b8b925d02536c6610c3.

Reason for revert: Breaks WebRTC roll to Chromium.
https://chromium-review.googlesource.com/c/chromium/src/+/1484629

# Fatal error in: ../../third_party/webrtc/modules/rtp_rtcp/source/rtp_sender.cc, line 796
# last system error: 0
# Check failed: diff_ms >= static_cast<int64_t>(0) (-307 vs. 0)
#

Original change's description:
> Remove VCMEncoderDataBase and put remaining code into VideoStreamEncoder
>
> Since this "data base" only holds a single encoder instance it just
> serves to confuse object ownership. Removing it and giving ownership
> of generic encoder instance to VideoStreamEncoder.
>
> This CL also removes VideoSender interface from video_coding_impl.h,
> which is mostly a leftover from
> https://webrtc-review.googlesource.com/c/src/+/123540
>
> Bug: webrtc:10164
> Change-Id: I9b7fec940dbcbccf3aa1278c2555da3bd5169ae1
> Reviewed-on: https://webrtc-review.googlesource.com/c/123920
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
> Reviewed-by: Niels Moller <nisse@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#26835}

TBR=brandtr@webrtc.org,nisse@webrtc.org,sprang@webrtc.org

Change-Id: I5432878c4c2e497cd848c4ce1b190e0307df03ca
Bug: webrtc:10164
Reviewed-on: https://webrtc-review.googlesource.com/c/124402
Commit-Queue: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26841}

modules/video_coding/BUILD.gn

@@ -102,6 +102,8 @@ rtc_static_library("video_coding") {
     "decoder_database.h",
     "decoding_state.cc",
     "decoding_state.h",
+    "encoder_database.cc",
+    "encoder_database.h",
     "fec_controller_default.cc",
     "fec_controller_default.h",
     "fec_rate_table.h",

modules/video_coding/encoder_database.cc

@@ -0,0 +1,186 @@
+/*
+ *  Copyright (c) 2018 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.
+ */
+
+#include "modules/video_coding/encoder_database.h"
+
+#include <string.h>
+
+#include "common_types.h"  // NOLINT(build/include)
+#include "rtc_base/checks.h"
+#include "rtc_base/logging.h"
+
+namespace webrtc {
+
+namespace {
+const size_t kDefaultPayloadSize = 1440;
+}
+
+VCMEncoderDataBase::VCMEncoderDataBase(
+    VCMEncodedFrameCallback* encoded_frame_callback)
+    : number_of_cores_(0),
+      max_payload_size_(kDefaultPayloadSize),
+      pending_encoder_reset_(true),
+      send_codec_(),
+      external_encoder_(nullptr),
+      internal_source_(false),
+      encoded_frame_callback_(encoded_frame_callback) {}
+
+VCMEncoderDataBase::~VCMEncoderDataBase() {
+  DeleteEncoder();
+}
+
+// Assuming only one registered encoder - since only one used, no need for more.
+bool VCMEncoderDataBase::SetSendCodec(const VideoCodec* send_codec,
+                                      int number_of_cores,
+                                      size_t max_payload_size) {
+  RTC_DCHECK(send_codec);
+  if (max_payload_size == 0) {
+    max_payload_size = kDefaultPayloadSize;
+  }
+  RTC_DCHECK_GE(number_of_cores, 1);
+  // Make sure the start bit rate is sane...
+  RTC_DCHECK_LE(send_codec->startBitrate, 1000000);
+  bool reset_required = pending_encoder_reset_;
+  if (number_of_cores_ != number_of_cores) {
+    number_of_cores_ = number_of_cores;
+    reset_required = true;
+  }
+  if (max_payload_size_ != max_payload_size) {
+    max_payload_size_ = max_payload_size;
+    reset_required = true;
+  }
+
+  VideoCodec new_send_codec;
+  memcpy(&new_send_codec, send_codec, sizeof(new_send_codec));
+
+  if (new_send_codec.maxBitrate == 0) {
+    // max is one bit per pixel
+    new_send_codec.maxBitrate = (static_cast<int>(send_codec->height) *
+                                 static_cast<int>(send_codec->width) *
+                                 static_cast<int>(send_codec->maxFramerate)) /
+                                1000;
+    if (send_codec->startBitrate > new_send_codec.maxBitrate) {
+      // But if the user tries to set a higher start bit rate we will
+      // increase the max accordingly.
+      new_send_codec.maxBitrate = send_codec->startBitrate;
+    }
+  }
+
+  if (new_send_codec.startBitrate > new_send_codec.maxBitrate)
+    new_send_codec.startBitrate = new_send_codec.maxBitrate;
+
+  if (!reset_required) {
+    reset_required = RequiresEncoderReset(new_send_codec);
+  }
+
+  memcpy(&send_codec_, &new_send_codec, sizeof(send_codec_));
+
+  if (!reset_required) {
+    return true;
+  }
+
+  // If encoder exists, will destroy it and create new one.
+  DeleteEncoder();
+  ptr_encoder_.reset(new VCMGenericEncoder(
+      external_encoder_, encoded_frame_callback_, internal_source_));
+  encoded_frame_callback_->SetInternalSource(internal_source_);
+  if (ptr_encoder_->InitEncode(&send_codec_, number_of_cores_,
+                               max_payload_size_) < 0) {
+    RTC_LOG(LS_ERROR) << "Failed to initialize video encoder.";
+    DeleteEncoder();
+    return false;
+  }
+
+  pending_encoder_reset_ = false;
+
+  return true;
+}
+
+void VCMEncoderDataBase::DeregisterExternalEncoder() {
+  DeleteEncoder();
+  memset(&send_codec_, 0, sizeof(VideoCodec));
+  external_encoder_ = nullptr;
+  internal_source_ = false;
+}
+
+void VCMEncoderDataBase::RegisterExternalEncoder(VideoEncoder* external_encoder,
+                                                 bool internal_source) {
+  // Since only one encoder can be used at a given time, only one external
+  // encoder can be registered/used.
+  RTC_CHECK(external_encoder_ == nullptr);
+  external_encoder_ = external_encoder;
+  internal_source_ = internal_source;
+  pending_encoder_reset_ = true;
+}
+
+bool VCMEncoderDataBase::RequiresEncoderReset(
+    const VideoCodec& new_send_codec) {
+  if (!ptr_encoder_)
+    return true;
+
+  // Does not check startBitrate, maxFramerate or plType
+  if (new_send_codec.codecType != send_codec_.codecType ||
+      new_send_codec.width != send_codec_.width ||
+      new_send_codec.height != send_codec_.height ||
+      new_send_codec.maxBitrate != send_codec_.maxBitrate ||
+      new_send_codec.minBitrate != send_codec_.minBitrate ||
+      new_send_codec.qpMax != send_codec_.qpMax ||
+      new_send_codec.numberOfSimulcastStreams !=
+          send_codec_.numberOfSimulcastStreams ||
+      new_send_codec.mode != send_codec_.mode) {
+    return true;
+  }
+
+  switch (new_send_codec.codecType) {
+    case kVideoCodecVP8:
+      if (new_send_codec.VP8() != *send_codec_.VP8()) {
+        return true;
+      }
+      break;
+
+    case kVideoCodecVP9:
+      if (new_send_codec.VP9() != *send_codec_.VP9()) {
+        return true;
+      }
+      break;
+
+    case kVideoCodecH264:
+      if (new_send_codec.H264() != *send_codec_.H264()) {
+        return true;
+      }
+      break;
+
+    default:
+      break;
+  }
+
+  for (unsigned char i = 0; i < new_send_codec.numberOfSimulcastStreams; ++i) {
+    if (new_send_codec.simulcastStream[i] != send_codec_.simulcastStream[i])
+      return true;
+  }
+  return false;
+}
+
+VCMGenericEncoder* VCMEncoderDataBase::GetEncoder() {
+  return ptr_encoder_.get();
+}
+
+void VCMEncoderDataBase::DeleteEncoder() {
+  if (!ptr_encoder_)
+    return;
+  ptr_encoder_->Release();
+  ptr_encoder_.reset();
+}
+
+bool VCMEncoderDataBase::MatchesCurrentResolution(int width, int height) const {
+  return send_codec_.width == width && send_codec_.height == height;
+}
+
+}  // namespace webrtc

modules/video_coding/encoder_database.h

@@ -0,0 +1,68 @@
+/*
+ *  Copyright (c) 2018 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 MODULES_VIDEO_CODING_ENCODER_DATABASE_H_
+#define MODULES_VIDEO_CODING_ENCODER_DATABASE_H_
+
+#include <stddef.h>
+#include <memory>
+
+#include "api/video_codecs/video_codec.h"
+#include "api/video_codecs/video_encoder.h"
+#include "modules/video_coding/generic_encoder.h"
+
+namespace webrtc {
+
+class VCMEncoderDataBase {
+ public:
+  explicit VCMEncoderDataBase(VCMEncodedFrameCallback* encoded_frame_callback);
+  ~VCMEncoderDataBase();
+
+  // Sets the sender side codec and initiates the desired codec given the
+  // VideoCodec struct.
+  // Returns true if the codec was successfully registered, false otherwise.
+  bool SetSendCodec(const VideoCodec* send_codec,
+                    int number_of_cores,
+                    size_t max_payload_size);
+
+  // Registers and initializes an external encoder object.
+  // |internal_source| should be set to true if the codec has an internal
+  // video source and doesn't need the user to provide it with frames via
+  // the Encode() method.
+  void RegisterExternalEncoder(VideoEncoder* external_encoder,
+                               bool internal_source);
+
+  // Deregisters any external encoder.
+  void DeregisterExternalEncoder();
+
+  VCMGenericEncoder* GetEncoder();
+
+  bool MatchesCurrentResolution(int width, int height) const;
+
+ private:
+  // Determines whether a new codec has to be created or not.
+  // Checks every setting apart from maxFramerate and startBitrate.
+  bool RequiresEncoderReset(const VideoCodec& send_codec);
+
+  void DeleteEncoder();
+
+  int number_of_cores_;
+  size_t max_payload_size_;
+  bool pending_encoder_reset_;
+  VideoCodec send_codec_;
+  VideoEncoder* external_encoder_;
+  bool internal_source_;
+  VCMEncodedFrameCallback* const encoded_frame_callback_;
+  std::unique_ptr<VCMGenericEncoder> ptr_encoder_;
+};
+
+}  // namespace webrtc
+
+#endif  // MODULES_VIDEO_CODING_ENCODER_DATABASE_H_

modules/video_coding/video_coding_impl.h

@@ -19,6 +19,7 @@
 
 #include "absl/types/optional.h"
 #include "modules/video_coding/decoder_database.h"
+#include "modules/video_coding/encoder_database.h"
 #include "modules/video_coding/frame_buffer.h"
 #include "modules/video_coding/generic_decoder.h"
 #include "modules/video_coding/generic_encoder.h"
@@ -56,6 +57,47 @@ class VCMProcessTimer {
   int64_t _latestMs;
 };
 
+class VideoSender {
+ public:
+  typedef VideoCodingModule::SenderNackMode SenderNackMode;
+
+  VideoSender(Clock* clock, EncodedImageCallback* post_encode_callback);
+  ~VideoSender();
+
+  // Register the send codec to be used.
+  // This method must be called on the construction thread.
+  int32_t RegisterSendCodec(const VideoCodec* sendCodec,
+                            uint32_t numberOfCores,
+                            uint32_t maxPayloadSize);
+
+  void RegisterExternalEncoder(VideoEncoder* externalEncoder,
+                               bool internalSource);
+
+  // Update the the encoder with new bitrate allocation and framerate.
+  int32_t SetChannelParameters(const VideoBitrateAllocation& bitrate_allocation,
+                               uint32_t framerate_fps);
+
+  int32_t AddVideoFrame(const VideoFrame& videoFrame,
+                        const CodecSpecificInfo* codecSpecificInfo,
+                        absl::optional<VideoEncoder::EncoderInfo> encoder_info);
+
+  int32_t IntraFrameRequest(size_t stream_index);
+
+ private:
+  rtc::CriticalSection encoder_crit_;
+  VCMGenericEncoder* _encoder;
+  VCMEncodedFrameCallback _encodedFrameCallback RTC_GUARDED_BY(encoder_crit_);
+  VCMEncoderDataBase _codecDataBase RTC_GUARDED_BY(encoder_crit_);
+
+  // Must be accessed on the construction thread of VideoSender.
+  VideoCodec current_codec_;
+  rtc::SequencedTaskChecker sequenced_checker_;
+
+  rtc::CriticalSection params_crit_;
+  bool encoder_has_internal_source_ RTC_GUARDED_BY(params_crit_);
+  std::vector<FrameType> next_frame_types_ RTC_GUARDED_BY(params_crit_);
+};
+
 class VideoReceiver : public Module {
  public:
   VideoReceiver(Clock* clock,

video/video_stream_encoder.cc

@@ -61,8 +61,6 @@ const float kFramedropThreshold = 0.3;
 // optimization module defaults.
 const int64_t kFrameRateAvergingWindowSizeMs = (1000 / 30) * 90;
 
-const size_t kDefaultPayloadSize = 1440;
-
 // Initial limits for BALANCED degradation preference.
 int MinFps(int pixels) {
   if (pixels <= 320 * 240) {
@@ -120,51 +118,6 @@ CpuOveruseOptions GetCpuOveruseOptions(
   return options;
 }
 
-bool RequiresEncoderReset(const VideoCodec& previous_send_codec,
-                          const VideoCodec& new_send_codec) {
-  // Does not check startBitrate or maxFramerate.
-  if (new_send_codec.codecType != previous_send_codec.codecType ||
-      new_send_codec.width != previous_send_codec.width ||
-      new_send_codec.height != previous_send_codec.height ||
-      new_send_codec.maxBitrate != previous_send_codec.maxBitrate ||
-      new_send_codec.minBitrate != previous_send_codec.minBitrate ||
-      new_send_codec.qpMax != previous_send_codec.qpMax ||
-      new_send_codec.numberOfSimulcastStreams !=
-          previous_send_codec.numberOfSimulcastStreams ||
-      new_send_codec.mode != previous_send_codec.mode) {
-    return true;
-  }
-
-  switch (new_send_codec.codecType) {
-    case kVideoCodecVP8:
-      if (new_send_codec.VP8() != previous_send_codec.VP8()) {
-        return true;
-      }
-      break;
-
-    case kVideoCodecVP9:
-      if (new_send_codec.VP9() != previous_send_codec.VP9()) {
-        return true;
-      }
-      break;
-
-    case kVideoCodecH264:
-      if (new_send_codec.H264() != previous_send_codec.H264()) {
-        return true;
-      }
-      break;
-
-    default:
-      break;
-  }
-
-  for (unsigned char i = 0; i < new_send_codec.numberOfSimulcastStreams; ++i) {
-    if (new_send_codec.simulcastStream[i] !=
-        previous_send_codec.simulcastStream[i])
-      return true;
-  }
-  return false;
-}
 }  //  namespace
 
 // VideoSourceProxy is responsible ensuring thread safety between calls to
@@ -441,6 +394,7 @@ VideoStreamEncoder::VideoStreamEncoder(
       pending_frame_drops_(0),
       generic_encoder_(nullptr),
       generic_encoder_callback_(this),
+      codec_database_(&generic_encoder_callback_),
       next_frame_types_(1, kVideoFrameDelta),
       encoder_queue_("EncoderQueue") {
   RTC_DCHECK(encoder_stats_observer);
@@ -460,11 +414,9 @@ void VideoStreamEncoder::Stop() {
   encoder_queue_.PostTask([this] {
     RTC_DCHECK_RUN_ON(&encoder_queue_);
     overuse_detector_->StopCheckForOveruse();
-    rate_allocator_ = nullptr;
+    rate_allocator_.reset();
     bitrate_observer_ = nullptr;
-    if (encoder_ != nullptr && generic_encoder_ != nullptr) {
+    codec_database_.DeregisterExternalEncoder();
-      encoder_->Release();
-    }
     generic_encoder_ = nullptr;
     quality_scaler_ = nullptr;
     shutdown_event_.Set();
@@ -641,37 +593,13 @@ void VideoStreamEncoder::ReconfigureEncoder() {
   }
   source_proxy_->SetMaxFramerate(max_framerate);
 
-  if (codec.maxBitrate == 0) {
-    // max is one bit per pixel
-    codec.maxBitrate =
-        (static_cast<int>(codec.height) * static_cast<int>(codec.width) *
-         static_cast<int>(codec.maxFramerate)) /
-        1000;
-    if (codec.startBitrate > codec.maxBitrate) {
-      // But if the user tries to set a higher start bit rate we will
-      // increase the max accordingly.
-      codec.maxBitrate = codec.startBitrate;
-    }
-  }
-
-  if (codec.startBitrate > codec.maxBitrate) {
-    codec.startBitrate = codec.maxBitrate;
-  }
-
-  // Reset (release existing encoder) if one exists and anything except
-  // start bitrate or max framerate has changed. Don't call Release() if
-  // |pending_encoder_creation_| as that means this is a new encoder
-  // that has not yet been initialized.
-  const bool reset_required = RequiresEncoderReset(codec, send_codec_);
-  send_codec_ = codec;
-
   // Keep the same encoder, as long as the video_format is unchanged.
   // Encoder creation block is split in two since EncoderInfo needed to start
   // CPU adaptation with the correct settings should be polled after
   // encoder_->InitEncode().
   if (pending_encoder_creation_) {
     if (encoder_) {
-      encoder_->Release();
+      codec_database_.DeregisterExternalEncoder();
       generic_encoder_ = nullptr;
     }
 
@@ -683,25 +611,15 @@ void VideoStreamEncoder::ReconfigureEncoder() {
 
     codec_info_ = settings_.encoder_factory->QueryVideoEncoder(
         encoder_config_.video_format);
-  } else if (reset_required) {
+
-    RTC_DCHECK(encoder_);
+    codec_database_.RegisterExternalEncoder(encoder_.get(),
-    encoder_->Release();
+                                            HasInternalSource());
   }
 
-  bool success = true;
+  // SetSendCodec implies an unconditional call to encoder_->InitEncode().
-  if (pending_encoder_creation_ || reset_required || !generic_encoder_) {
+  bool success = codec_database_.SetSendCodec(&codec, number_of_cores_,
-    RTC_DCHECK(encoder_);
+                                              max_data_payload_length_);
-    generic_encoder_ = absl::make_unique<VCMGenericEncoder>(
+  generic_encoder_ = codec_database_.GetEncoder();
-        encoder_.get(), &generic_encoder_callback_, HasInternalSource());
-    if (generic_encoder_->InitEncode(&send_codec_, number_of_cores_,
-                                     max_data_payload_length_ > 0
-                                         ? max_data_payload_length_
-                                         : kDefaultPayloadSize) < 0) {
-      encoder_->Release();
-      generic_encoder_ = nullptr;
-      success = false;
-    }
-  }
 
   if (success) {
     RTC_DCHECK(generic_encoder_);
@@ -715,7 +633,7 @@ void VideoStreamEncoder::ReconfigureEncoder() {
                         << " max payload size " << max_data_payload_length_;
   } else {
     RTC_LOG(LS_ERROR) << "Failed to configure encoder.";
-    rate_allocator_ = nullptr;
+    rate_allocator_.reset();
   }
 
   if (pending_encoder_creation_) {
@@ -1199,8 +1117,8 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame,
 
   encoder_info_ = info;
   RTC_DCHECK(generic_encoder_);
-  RTC_DCHECK_EQ(send_codec_.width, out_frame.width());
+  RTC_DCHECK(codec_database_.MatchesCurrentResolution(out_frame.width(),
-  RTC_DCHECK_EQ(send_codec_.height, out_frame.height());
+                                                      out_frame.height()));
   const VideoFrameBuffer::Type buffer_type =
       out_frame.video_frame_buffer()->type();
   const bool is_buffer_type_supported =

video/video_stream_encoder.h

@@ -304,12 +304,11 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
       RTC_GUARDED_BY(&encoder_queue_);
 
   // TODO(webrtc:10164): Refactor/remove these VCM classes.
-  std::unique_ptr<VCMGenericEncoder> generic_encoder_
+  // |generic_encoder_| instance is owned by |codec_database_|.
-      RTC_GUARDED_BY(&encoder_queue_);
+  VCMGenericEncoder* generic_encoder_ RTC_GUARDED_BY(&encoder_queue_);
   VCMEncodedFrameCallback generic_encoder_callback_
       RTC_GUARDED_BY(&encoder_queue_);
-  VideoCodec send_codec_ RTC_GUARDED_BY(&encoder_queue_);
+  VCMEncoderDataBase codec_database_ RTC_GUARDED_BY(&encoder_queue_);
-
   // TODO(sprang): Change actually support keyframe per simulcast stream, or
   // turn this into a simple bool |pending_keyframe_request_|.
   std::vector<FrameType> next_frame_types_ RTC_GUARDED_BY(&encoder_queue_);

video/video_stream_encoder_unittest.cc

@@ -547,7 +547,7 @@ class VideoStreamEncoderTest : public ::testing::Test {
     VideoEncoder::EncoderInfo GetEncoderInfo() const override {
       rtc::CritScope lock(&local_crit_sect_);
       EncoderInfo info;
-      if (initialized_ == EncoderState::kInitialized) {
+      if (initialized_) {
         if (quality_scaling_) {
           info.scaling_settings =
               VideoEncoder::ScalingSettings(1, 2, kMinPixelsPerFrame);
@@ -662,7 +662,6 @@ class VideoStreamEncoderTest : public ::testing::Test {
       int res =
           FakeEncoder::InitEncode(config, number_of_cores, max_payload_size);
       rtc::CritScope lock(&local_crit_sect_);
-      EXPECT_EQ(initialized_, EncoderState::kUninitialized);
       if (config->codecType == kVideoCodecVP8) {
         // Simulate setting up temporal layers, in order to validate the life
         // cycle of these objects.
@@ -673,22 +672,13 @@ class VideoStreamEncoderTest : public ::testing::Test {
                                       config->VP8().numberOfTemporalLayers));
         }
       }
-      if (force_init_encode_failed_) {
+      if (force_init_encode_failed_)
-        initialized_ = EncoderState::kInitializationFailed;
         return -1;
-      }
 
-      initialized_ = EncoderState::kInitialized;
+      initialized_ = true;
       return res;
     }
 
-    int32_t Release() override {
-      rtc::CritScope lock(&local_crit_sect_);
-      EXPECT_NE(initialized_, EncoderState::kUninitialized);
-      initialized_ = EncoderState::kUninitialized;
-      return FakeEncoder::Release();
-    }
-
     int32_t SetRateAllocation(const VideoBitrateAllocation& rate_allocation,
                               uint32_t framerate) {
       rtc::CritScope lock(&local_crit_sect_);
@@ -710,12 +700,7 @@ class VideoStreamEncoderTest : public ::testing::Test {
     }
 
     rtc::CriticalSection local_crit_sect_;
-    enum class EncoderState {
+    bool initialized_ RTC_GUARDED_BY(local_crit_sect_) = false;
-      kUninitialized,
-      kInitializationFailed,
-      kInitialized
-    } initialized_ RTC_GUARDED_BY(local_crit_sect_) =
-        EncoderState::kUninitialized;
     bool block_next_encode_ RTC_GUARDED_BY(local_crit_sect_) = false;
     rtc::Event continue_encode_event_;
     uint32_t timestamp_ RTC_GUARDED_BY(local_crit_sect_) = 0;