From 984cf9b837f906f5872fe601c9712b0444d5ce6d Mon Sep 17 00:00:00 2001
From: Sergey Silkin <ssilkin@webrtc.org>
Date: Mon, 22 Nov 2021 08:00:32 +0100
Subject: [PATCH] Explicitly set encoder and decoder format in codec tests.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This allows to differentiate and test codecs of the same type but
different implementations/settings.

Bug: none
Change-Id: I74f799b36411e63387513133ffc19a7f0c45d550
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/238165
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35396}
---
 api/test/videocodec_test_fixture.h            |  8 +++
 .../test/videocodec_test_fixture_impl.cc      | 58 ++++++++++++-------
 2 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/api/test/videocodec_test_fixture.h b/api/test/videocodec_test_fixture.h
index c9b9ed4087..dbf20993e2 100644
--- a/api/test/videocodec_test_fixture.h
+++ b/api/test/videocodec_test_fixture.h
@@ -128,6 +128,14 @@ class VideoCodecTestFixture {
     // Name of the codec being tested.
     std::string codec_name;
 
+    // Encoder and decoder format and parameters. If provided, format is used to
+    // instantiate the codec. If not provided, the test creates and uses the
+    // default `SdpVideoFormat` based on `codec_name`.
+    // Encoder and decoder name (`SdpVideoFormat::name`) should be the same as
+    // `codec_name`.
+    absl::optional<SdpVideoFormat> encoder_format;
+    absl::optional<SdpVideoFormat> decoder_format;
+
     // H.264 specific settings.
     struct H264CodecSettings {
       H264Profile profile = H264Profile::kProfileConstrainedBaseline;
diff --git a/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc b/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc
index 03da293d60..871300a6e0 100644
--- a/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc
+++ b/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc
@@ -150,6 +150,26 @@ std::string FilenameWithParams(
          std::to_string(config.codec_settings.startBitrate);
 }
 
+SdpVideoFormat CreateSdpVideoFormat(
+    const VideoCodecTestFixtureImpl::Config& config) {
+  if (config.codec_settings.codecType == kVideoCodecH264) {
+    const char* packetization_mode =
+        config.h264_codec_settings.packetization_mode ==
+                H264PacketizationMode::NonInterleaved
+            ? "1"
+            : "0";
+    SdpVideoFormat::Parameters codec_params = {
+        {cricket::kH264FmtpProfileLevelId,
+         *H264ProfileLevelIdToString(H264ProfileLevelId(
+             config.h264_codec_settings.profile, H264Level::kLevel3_1))},
+        {cricket::kH264FmtpPacketizationMode, packetization_mode}};
+
+    return SdpVideoFormat(config.codec_name, codec_params);
+  }
+
+  return SdpVideoFormat(config.codec_name);
+}
+
 }  // namespace
 
 VideoCodecTestFixtureImpl::Config::Config() = default;
@@ -611,23 +631,21 @@ void VideoCodecTestFixtureImpl::VerifyVideoStatistic(
 }
 
 bool VideoCodecTestFixtureImpl::CreateEncoderAndDecoder() {
-  SdpVideoFormat::Parameters params;
-  if (config_.codec_settings.codecType == kVideoCodecH264) {
-    const char* packetization_mode =
-        config_.h264_codec_settings.packetization_mode ==
-                H264PacketizationMode::NonInterleaved
-            ? "1"
-            : "0";
-    params = {{cricket::kH264FmtpProfileLevelId,
-               *H264ProfileLevelIdToString(H264ProfileLevelId(
-                   config_.h264_codec_settings.profile, H264Level::kLevel3_1))},
-              {cricket::kH264FmtpPacketizationMode, packetization_mode}};
-  } else {
-    params = {};
-  }
-  SdpVideoFormat format(config_.codec_name, params);
+  SdpVideoFormat encoder_format(CreateSdpVideoFormat(config_));
+  SdpVideoFormat decoder_format = encoder_format;
 
-  encoder_ = encoder_factory_->CreateVideoEncoder(format);
+  // Override encoder and decoder formats with explicitly provided ones.
+  if (config_.encoder_format) {
+    RTC_DCHECK_EQ(config_.encoder_format->name, config_.codec_name);
+    encoder_format = *config_.encoder_format;
+  }
+
+  if (config_.decoder_format) {
+    RTC_DCHECK_EQ(config_.decoder_format->name, config_.codec_name);
+    decoder_format = *config_.decoder_format;
+  }
+
+  encoder_ = encoder_factory_->CreateVideoEncoder(encoder_format);
   EXPECT_TRUE(encoder_) << "Encoder not successfully created.";
   if (encoder_ == nullptr) {
     return false;
@@ -636,15 +654,13 @@ bool VideoCodecTestFixtureImpl::CreateEncoderAndDecoder() {
   const size_t num_simulcast_or_spatial_layers = std::max(
       config_.NumberOfSimulcastStreams(), config_.NumberOfSpatialLayers());
   for (size_t i = 0; i < num_simulcast_or_spatial_layers; ++i) {
-    decoders_.push_back(std::unique_ptr<VideoDecoder>(
-        decoder_factory_->CreateVideoDecoder(format)));
-  }
-
-  for (const auto& decoder : decoders_) {
+    std::unique_ptr<VideoDecoder> decoder =
+        decoder_factory_->CreateVideoDecoder(decoder_format);
     EXPECT_TRUE(decoder) << "Decoder not successfully created.";
     if (decoder == nullptr) {
       return false;
     }
+    decoders_.push_back(std::move(decoder));
   }
 
   return true;