Use VideoDecoder::Configure interface when setting up decoder

Bug: webrtc:13045
Change-Id: I322ff91d96bab8bb7c40f4dea1c9c2b5c7631635
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228420
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34756}

api/test/mock_video_decoder.h

@@ -37,6 +37,12 @@ class MockDecodedImageCallback : public DecodedImageCallback {
 
 class MockVideoDecoder : public VideoDecoder {
  public:
+  MockVideoDecoder() {
+    // Make `Configure` succeed by default, so that individual tests that
+    // verify other methods wouldn't need to stub `Configure`.
+    ON_CALL(*this, Configure).WillByDefault(testing::Return(true));
+  }
+
   MOCK_METHOD(bool, Configure, (const Settings& settings), (override));
   MOCK_METHOD(int32_t,
               InitDecode,

modules/video_coding/decoder_database.cc

@@ -48,19 +48,17 @@ bool VCMDecoderDataBase::IsExternalDecoderRegistered(
          decoders_.find(payload_type) != decoders_.end();
 }
 
-bool VCMDecoderDataBase::RegisterReceiveCodec(uint8_t payload_type,
+bool VCMDecoderDataBase::RegisterReceiveCodec(
-                                              const VideoCodec& receive_codec,
+    uint8_t payload_type,
-                                              int number_of_cores) {
+    const VideoDecoder::Settings& settings) {
-  if (number_of_cores < 0) {
+  if (settings.number_of_cores() < 0) {
     return false;
   }
   // If payload value already exists, erase old and insert new.
   if (payload_type == current_payload_type_) {
     current_payload_type_ = absl::nullopt;
   }
-  auto& entry = decoder_settings_[payload_type];
+  decoder_settings_[payload_type] = settings;
-  entry.settings = receive_codec;
-  entry.number_of_cores = number_of_cores;
   return true;
 }
 
@@ -127,16 +125,14 @@ void VCMDecoderDataBase::CreateAndInitDecoder(const VCMEncodedFrame& frame) {
   // the first frame being of a different resolution than the database values.
   // This is best effort, since there's no guarantee that width/height have been
   // parsed yet (and may be zero).
-  if (frame.EncodedImage()._encodedWidth > 0 &&
+  RenderResolution frame_resolution(frame.EncodedImage()._encodedWidth,
-      frame.EncodedImage()._encodedHeight > 0) {
+                                    frame.EncodedImage()._encodedHeight);
-    decoder_item->second.settings.width = frame.EncodedImage()._encodedWidth;
+  if (frame_resolution.Valid()) {
-    decoder_item->second.settings.height = frame.EncodedImage()._encodedHeight;
+    decoder_item->second.set_max_render_resolution(frame_resolution);
   }
-  int err = current_decoder_->InitDecode(&decoder_item->second.settings,
+  if (!current_decoder_->Configure(decoder_item->second)) {
-                                         decoder_item->second.number_of_cores);
-  if (err < 0) {
     current_decoder_ = absl::nullopt;
-    RTC_LOG(LS_ERROR) << "Failed to initialize decoder. Error code: " << err;
+    RTC_LOG(LS_ERROR) << "Failed to initialize decoder.";
   }
 }
 

modules/video_coding/decoder_database.h

@@ -35,8 +35,7 @@ class VCMDecoderDataBase {
   bool IsExternalDecoderRegistered(uint8_t payload_type) const;
 
   bool RegisterReceiveCodec(uint8_t payload_type,
-                            const VideoCodec& receive_codec,
+                            const VideoDecoder::Settings& settings);
-                            int number_of_cores);
   bool DeregisterReceiveCodec(uint8_t payload_type);
 
   // Returns a decoder specified by frame.PayloadType. The decoded frame
@@ -49,17 +48,12 @@ class VCMDecoderDataBase {
       VCMDecodedFrameCallback* decoded_frame_callback);
 
  private:
-  struct DecoderSettings {
-    VideoCodec settings;
-    int number_of_cores;
-  };
-
   void CreateAndInitDecoder(const VCMEncodedFrame& frame);
 
   absl::optional<uint8_t> current_payload_type_;
   absl::optional<VCMGenericDecoder> current_decoder_;
   // Initialization paramaters for decoders keyed by payload type.
-  std::map<uint8_t, DecoderSettings> decoder_settings_;
+  std::map<uint8_t, VideoDecoder::Settings> decoder_settings_;
   // Decoders keyed by payload type.
   std::map<uint8_t, VideoDecoder*> decoders_;
 };

modules/video_coding/generic_decoder.cc

@@ -245,18 +245,17 @@ VCMGenericDecoder::~VCMGenericDecoder() {
   decoder_->Release();
 }
 
-int32_t VCMGenericDecoder::InitDecode(const VideoCodec* settings,
+bool VCMGenericDecoder::Configure(const VideoDecoder::Settings& settings) {
-                                      int32_t numberOfCores) {
+  TRACE_EVENT0("webrtc", "VCMGenericDecoder::Configure");
-  TRACE_EVENT0("webrtc", "VCMGenericDecoder::InitDecode");
 
-  int err = decoder_->InitDecode(settings, numberOfCores);
+  bool ok = decoder_->Configure(settings);
   decoder_info_ = decoder_->GetDecoderInfo();
   RTC_LOG(LS_INFO) << "Decoder implementation: " << decoder_info_.ToString();
   if (_callback) {
     _callback->OnDecoderImplementationName(
         decoder_info_.implementation_name.c_str());
   }
-  return err;
+  return ok;
 }
 
 int32_t VCMGenericDecoder::Decode(const VCMEncodedFrame& frame, Timestamp now) {

modules/video_coding/generic_decoder.h

@@ -15,6 +15,7 @@
 
 #include "api/sequence_checker.h"
 #include "api/units/time_delta.h"
+#include "api/video_codecs/video_decoder.h"
 #include "modules/video_coding/encoded_frame.h"
 #include "modules/video_coding/include/video_codec_interface.h"
 #include "modules/video_coding/timestamp_map.h"
@@ -80,9 +81,9 @@ class VCMGenericDecoder {
   ~VCMGenericDecoder();
 
   /**
-   * Initialize the decoder with the information from the VideoCodec
+   * Initialize the decoder with the information from the `settings`
    */
-  int32_t InitDecode(const VideoCodec* settings, int32_t numberOfCores);
+  bool Configure(const VideoDecoder::Settings& settings);
 
   /**
    * Decode to a raw I420 frame,

modules/video_coding/generic_decoder_unittest.cc

@@ -15,6 +15,7 @@
 
 #include "absl/types/optional.h"
 #include "api/task_queue/default_task_queue_factory.h"
+#include "api/video_codecs/video_decoder.h"
 #include "common_video/test/utilities.h"
 #include "modules/video_coding/timing.h"
 #include "rtc_base/event.h"
@@ -74,11 +75,11 @@ class GenericDecoderTest : public ::testing::Test {
   void SetUp() override {
     generic_decoder_.RegisterDecodeCompleteCallback(&vcm_callback_);
     vcm_callback_.SetUserReceiveCallback(&user_callback_);
-    VideoCodec settings;
+    VideoDecoder::Settings settings;
-    settings.codecType = kVideoCodecVP8;
+    settings.set_codec_type(kVideoCodecVP8);
-    settings.width = 10;
+    settings.set_max_render_resolution({10, 10});
-    settings.height = 10;
+    settings.set_number_of_cores(4);
-    generic_decoder_.InitDecode(&settings, /*numberOfCores=*/4);
+    generic_decoder_.Configure(settings);
   }
 
   SimulatedClock clock_;

modules/video_coding/include/video_coding.h

@@ -13,6 +13,7 @@
 
 #include "api/video/video_frame.h"
 #include "api/video_codecs/video_codec.h"
+#include "api/video_codecs/video_decoder.h"
 #include "modules/include/module.h"
 #include "modules/rtp_rtcp/source/rtp_video_header.h"
 #include "modules/video_coding/include/video_coding_defines.h"
@@ -43,12 +44,13 @@ class VideoCodingModule : public Module {
   //
   // Input:
   //      - payload_type      : RTP payload type
-  //      - receiveCodec      : Settings for the codec to be registered.
+  //      - settings          : Settings for the decoder to be registered.
-  //      - numberOfCores     : Number of CPU cores that the decoder is allowed
-  //      to use.
   //
-  // Return value      : VCM_OK, on success.
+  // Return value      : true,  on success.
-  //                     < 0,    on error.
+  //                     false, on error.
+  virtual bool RegisterReceiveCodec(uint8_t payload_type,
+                                    const VideoDecoder::Settings& settings) = 0;
+  ABSL_DEPRECATED("Use RegisterReceiveCodec above")
   virtual int32_t RegisterReceiveCodec(uint8_t payload_type,
                                        const VideoCodec* receiveCodec,
                                        int32_t numberOfCores) = 0;

modules/video_coding/video_coding_impl.cc

@@ -56,11 +56,25 @@ class VideoCodingModuleImpl : public VideoCodingModule {
 
   void Process() override { receiver_.Process(); }
 
+  bool RegisterReceiveCodec(
+      uint8_t payload_type,
+      const VideoDecoder::Settings& decoder_settings) override {
+    return receiver_.RegisterReceiveCodec(payload_type, decoder_settings);
+  }
+
   int32_t RegisterReceiveCodec(uint8_t payload_type,
-                               const VideoCodec* receiveCodec,
+                               const VideoCodec* receive_codec,
-                               int32_t numberOfCores) override {
+                               int32_t number_of_cores) override {
-    return receiver_.RegisterReceiveCodec(payload_type, receiveCodec,
+    VideoDecoder::Settings decoder_settings;
-                                          numberOfCores);
+    if (receive_codec != nullptr) {
+      decoder_settings.set_codec_type(receive_codec->codecType);
+      decoder_settings.set_max_render_resolution(
+          {receive_codec->width, receive_codec->height});
+      decoder_settings.set_buffer_pool_size(receive_codec->buffer_pool_size);
+    }
+    decoder_settings.set_number_of_cores(number_of_cores);
+    return receiver_.RegisterReceiveCodec(payload_type, decoder_settings) ? 0
+                                                                          : -1;
   }
 
   void RegisterExternalDecoder(VideoDecoder* externalDecoder,

modules/video_coding/video_coding_impl.h

@@ -59,9 +59,8 @@ class VideoReceiver : public Module {
   VideoReceiver(Clock* clock, VCMTiming* timing);
   ~VideoReceiver() override;
 
-  int32_t RegisterReceiveCodec(uint8_t payload_type,
+  bool RegisterReceiveCodec(uint8_t payload_type,
-                               const VideoCodec* receiveCodec,
+                            const VideoDecoder::Settings& settings);
-                               int32_t numberOfCores);
 
   void RegisterExternalDecoder(VideoDecoder* externalDecoder,
                                uint8_t payloadType);

modules/video_coding/video_receiver.cc

@@ -246,18 +246,11 @@ int32_t VideoReceiver::Decode(const VCMEncodedFrame& frame) {
 }
 
 // Register possible receive codecs, can be called multiple times
-int32_t VideoReceiver::RegisterReceiveCodec(uint8_t payload_type,
+bool VideoReceiver::RegisterReceiveCodec(
-                                            const VideoCodec* receiveCodec,
+    uint8_t payload_type,
-                                            int32_t numberOfCores) {
+    const VideoDecoder::Settings& settings) {
   RTC_DCHECK_RUN_ON(&construction_thread_checker_);
-  if (receiveCodec == nullptr) {
+  return _codecDataBase.RegisterReceiveCodec(payload_type, settings);
-    return VCM_PARAMETER_ERROR;
-  }
-  if (!_codecDataBase.RegisterReceiveCodec(payload_type, *receiveCodec,
-                                           numberOfCores)) {
-    return -1;
-  }
-  return 0;
 }
 
 // Incoming packet from network parsed and ready for decode, non blocking.

modules/video_coding/video_receiver2.cc

@@ -110,19 +110,12 @@ int32_t VideoReceiver2::Decode(const VCMEncodedFrame* frame) {
 }
 
 // Register possible receive codecs, can be called multiple times
-int32_t VideoReceiver2::RegisterReceiveCodec(uint8_t payload_type,
+bool VideoReceiver2::RegisterReceiveCodec(
-                                             const VideoCodec* receiveCodec,
+    uint8_t payload_type,
-                                             int32_t numberOfCores) {
+    const VideoDecoder::Settings& settings) {
   RTC_DCHECK_RUN_ON(&construction_sequence_checker_);
   RTC_DCHECK(!IsDecoderThreadRunning());
-  if (receiveCodec == nullptr) {
+  return codecDataBase_.RegisterReceiveCodec(payload_type, settings);
-    return VCM_PARAMETER_ERROR;
-  }
-  if (!codecDataBase_.RegisterReceiveCodec(payload_type, *receiveCodec,
-                                           numberOfCores)) {
-    return -1;
-  }
-  return 0;
 }
 
 bool VideoReceiver2::IsDecoderThreadRunning() {

modules/video_coding/video_receiver2.h

@@ -12,6 +12,7 @@
 #define MODULES_VIDEO_CODING_VIDEO_RECEIVER2_H_
 
 #include "api/sequence_checker.h"
+#include "api/video_codecs/video_decoder.h"
 #include "modules/video_coding/decoder_database.h"
 #include "modules/video_coding/encoded_frame.h"
 #include "modules/video_coding/generic_decoder.h"
@@ -30,9 +31,8 @@ class VideoReceiver2 {
   VideoReceiver2(Clock* clock, VCMTiming* timing);
   ~VideoReceiver2();
 
-  int32_t RegisterReceiveCodec(uint8_t payload_type,
+  bool RegisterReceiveCodec(uint8_t payload_type,
-                               const VideoCodec* receiveCodec,
+                            const VideoDecoder::Settings& decoder_settings);
-                               int32_t numberOfCores);
 
   void RegisterExternalDecoder(VideoDecoder* externalDecoder,
                                uint8_t payloadType);

modules/video_coding/video_receiver_unittest.cc

@@ -9,12 +9,12 @@
  */
 
 #include "api/test/mock_video_decoder.h"
+#include "api/video_codecs/video_decoder.h"
 #include "modules/video_coding/include/video_coding.h"
 #include "modules/video_coding/timing.h"
 #include "modules/video_coding/video_coding_impl.h"
 #include "system_wrappers/include/clock.h"
 #include "test/gtest.h"
-#include "test/video_codec_settings.h"
 
 using ::testing::_;
 using ::testing::AnyNumber;
@@ -56,9 +56,9 @@ class TestVideoReceiver : public ::testing::Test {
   virtual void SetUp() {
     // Register decoder.
     receiver_.RegisterExternalDecoder(&decoder_, kUnusedPayloadType);
-    webrtc::test::CodecSettings(kVideoCodecVP8, &settings_);
+    VideoDecoder::Settings settings;
-    EXPECT_EQ(
+    settings.set_codec_type(kVideoCodecVP8);
-        0, receiver_.RegisterReceiveCodec(kUnusedPayloadType, &settings_, 1));
+    EXPECT_TRUE(receiver_.RegisterReceiveCodec(kUnusedPayloadType, settings));
 
     // Set protection mode.
     const size_t kMaxNackListSize = 250;
@@ -119,7 +119,6 @@ class TestVideoReceiver : public ::testing::Test {
   }
 
   SimulatedClock clock_;
-  VideoCodec settings_;
   NiceMock<MockVideoDecoder> decoder_;
   NiceMock<MockPacketRequestCallback> packet_request_callback_;
   VCMTiming timing_;

video/rtp_video_stream_receiver_unittest.cc

@@ -539,7 +539,6 @@ TEST_P(RtpVideoStreamReceiverTestH264, InBandSpsPps) {
 
 TEST_P(RtpVideoStreamReceiverTestH264, OutOfBandFmtpSpsPps) {
   constexpr int kPayloadType = 99;
-  VideoCodec codec;
   std::map<std::string, std::string> codec_params;
   // Example parameter sets from https://tools.ietf.org/html/rfc3984#section-8.2
   // .

video/video_receive_stream.cc

@@ -111,34 +111,6 @@ class WebRtcRecordableEncodedFrame : public RecordableEncodedFrame {
   absl::optional<webrtc::ColorSpace> color_space_;
 };
 
-VideoCodec CreateDecoderVideoCodec(const VideoReceiveStream::Decoder& decoder) {
-  VideoCodec codec;
-  codec.codecType = PayloadStringToCodecType(decoder.video_format.name);
-
-  if (codec.codecType == kVideoCodecVP8) {
-    *(codec.VP8()) = VideoEncoder::GetDefaultVp8Settings();
-  } else if (codec.codecType == kVideoCodecVP9) {
-    *(codec.VP9()) = VideoEncoder::GetDefaultVp9Settings();
-  } else if (codec.codecType == kVideoCodecH264) {
-    *(codec.H264()) = VideoEncoder::GetDefaultH264Settings();
-  } else if (codec.codecType == kVideoCodecMultiplex) {
-    VideoReceiveStream::Decoder associated_decoder = decoder;
-    associated_decoder.video_format =
-        SdpVideoFormat(CodecTypeToPayloadString(kVideoCodecVP9));
-    VideoCodec associated_codec = CreateDecoderVideoCodec(associated_decoder);
-    associated_codec.codecType = kVideoCodecMultiplex;
-    return associated_codec;
-  }
-
-  codec.width = 320;
-  codec.height = 180;
-  const int kDefaultStartBitrate = 300;
-  codec.startBitrate = codec.minBitrate = codec.maxBitrate =
-      kDefaultStartBitrate;
-
-  return codec;
-}
-
 // Video decoder class to be used for unknown codecs. Doesn't support decoding
 // but logs messages to LS_ERROR.
 class NullVideoDecoder : public webrtc::VideoDecoder {
@@ -364,15 +336,19 @@ void VideoReceiveStream::Start() {
 
     video_receiver_.RegisterExternalDecoder(video_decoders_.back().get(),
                                             decoder.payload_type);
-    VideoCodec codec = CreateDecoderVideoCodec(decoder);
+    VideoDecoder::Settings settings;
+    settings.set_codec_type(
+        PayloadStringToCodecType(decoder.video_format.name));
+    settings.set_max_render_resolution({320, 180});
+    settings.set_number_of_cores(num_cpu_cores_);
 
     const bool raw_payload =
         config_.rtp.raw_payload_types.count(decoder.payload_type) > 0;
     rtp_video_stream_receiver_.AddReceiveCodec(
-        decoder.payload_type, codec.codecType, decoder.video_format.parameters,
+        decoder.payload_type, settings.codec_type(),
-        raw_payload);
+        decoder.video_format.parameters, raw_payload);
-    RTC_CHECK_EQ(VCM_OK, video_receiver_.RegisterReceiveCodec(
+    RTC_CHECK(
-                             decoder.payload_type, &codec, num_cpu_cores_));
+        video_receiver_.RegisterReceiveCodec(decoder.payload_type, settings));
   }
 
   RTC_DCHECK(renderer != nullptr);

video/video_receive_stream2.cc

@@ -113,43 +113,17 @@ class WebRtcRecordableEncodedFrame : public RecordableEncodedFrame {
   absl::optional<webrtc::ColorSpace> color_space_;
 };
 
-VideoCodec CreateDecoderVideoCodec(const VideoReceiveStream::Decoder& decoder) {
+RenderResolution InitialDecoderResolution() {
-  VideoCodec codec;
-  codec.codecType = PayloadStringToCodecType(decoder.video_format.name);
-
-  if (codec.codecType == kVideoCodecVP8) {
-    *(codec.VP8()) = VideoEncoder::GetDefaultVp8Settings();
-  } else if (codec.codecType == kVideoCodecVP9) {
-    *(codec.VP9()) = VideoEncoder::GetDefaultVp9Settings();
-  } else if (codec.codecType == kVideoCodecH264) {
-    *(codec.H264()) = VideoEncoder::GetDefaultH264Settings();
-  } else if (codec.codecType == kVideoCodecMultiplex) {
-    VideoReceiveStream::Decoder associated_decoder = decoder;
-    associated_decoder.video_format =
-        SdpVideoFormat(CodecTypeToPayloadString(kVideoCodecVP9));
-    VideoCodec associated_codec = CreateDecoderVideoCodec(associated_decoder);
-    associated_codec.codecType = kVideoCodecMultiplex;
-    return associated_codec;
-  }
-
   FieldTrialOptional<int> width("w");
   FieldTrialOptional<int> height("h");
   ParseFieldTrial(
       {&width, &height},
       field_trial::FindFullName("WebRTC-Video-InitialDecoderResolution"));
   if (width && height) {
-    codec.width = width.Value();
+    return RenderResolution(width.Value(), height.Value());
-    codec.height = height.Value();
-  } else {
-    codec.width = 320;
-    codec.height = 180;
   }
 
-  const int kDefaultStartBitrate = 300;
+  return RenderResolution(320, 180);
-  codec.startBitrate = codec.minBitrate = codec.maxBitrate =
-      kDefaultStartBitrate;
-
-  return codec;
 }
 
 // Video decoder class to be used for unknown codecs. Doesn't support decoding
@@ -380,7 +354,11 @@ void VideoReceiveStream2::Start() {
       ++decoders_count;
     }
 
-    VideoCodec codec = CreateDecoderVideoCodec(decoder);
+    VideoDecoder::Settings settings;
+    settings.set_codec_type(
+        PayloadStringToCodecType(decoder.video_format.name));
+    settings.set_max_render_resolution(InitialDecoderResolution());
+    settings.set_number_of_cores(num_cpu_cores_);
 
     const bool raw_payload =
         config_.rtp.raw_payload_types.count(decoder.payload_type) > 0;
@@ -388,11 +366,11 @@ void VideoReceiveStream2::Start() {
       // TODO(bugs.webrtc.org/11993): Make this call on the network thread.
       RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
       rtp_video_stream_receiver_.AddReceiveCodec(
-          decoder.payload_type, codec.codecType,
+          decoder.payload_type, settings.codec_type(),
           decoder.video_format.parameters, raw_payload);
     }
-    RTC_CHECK_EQ(VCM_OK, video_receiver_.RegisterReceiveCodec(
+    RTC_CHECK(
-                             decoder.payload_type, &codec, num_cpu_cores_));
+        video_receiver_.RegisterReceiveCodec(decoder.payload_type, settings));
   }
 
   RTC_DCHECK(renderer != nullptr);