Provide Environment to construct VideoBitrateAllocator

To allow various VideoBitrateAllocators to use propagated rather than global field trials Bug: webrtc:42220378 Change-Id: I52816628169a54b18a4405d84fee69b101f92f72 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349920 Reviewed-by: Erik Språng <sprang@webrtc.org> Reviewed-by: Philip Eliasson <philipel@webrtc.org> Commit-Queue: Danil Chapovalov <danilchap@webrtc.org> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org> Cr-Commit-Position: refs/heads/main@{#42288}
signalapp · May 13, 2024 · 4bf4e17 · 4bf4e17
1 parent eaa4f3a
commit 4bf4e17
Show file tree

Hide file tree

Showing 20 changed files with 89 additions and 58 deletions.
diff --git a/api/test/mock_video_bitrate_allocator_factory.h b/api/test/mock_video_bitrate_allocator_factory.h
@@ -18,13 +18,12 @@
 
 namespace webrtc {
 
-class MockVideoBitrateAllocatorFactory
-    : public webrtc::VideoBitrateAllocatorFactory {
+class MockVideoBitrateAllocatorFactory : public VideoBitrateAllocatorFactory {
  public:
   ~MockVideoBitrateAllocatorFactory() override { Die(); }
   MOCK_METHOD(std::unique_ptr<VideoBitrateAllocator>,
-              CreateVideoBitrateAllocator,
-              (const VideoCodec&),
+              Create,
+              (const Environment&, const VideoCodec&),
               (override));
   MOCK_METHOD(void, Die, ());
 };

diff --git a/api/video/BUILD.gn b/api/video/BUILD.gn
@@ -264,6 +264,8 @@ rtc_source_set("video_bitrate_allocator_factory") {
   sources = [ "video_bitrate_allocator_factory.h" ]
   deps = [
     ":video_bitrate_allocator",
+    "../../rtc_base:checks",
+    "../environment",
     "../video_codecs:video_codecs_api",
   ]
 }

diff --git a/api/video/builtin_video_bitrate_allocator_factory.cc b/api/video/builtin_video_bitrate_allocator_factory.cc
@@ -29,7 +29,10 @@ class BuiltinVideoBitrateAllocatorFactory
   BuiltinVideoBitrateAllocatorFactory() = default;
   ~BuiltinVideoBitrateAllocatorFactory() override = default;
 
-  std::unique_ptr<VideoBitrateAllocator> CreateVideoBitrateAllocator(
+  // TODO: bugs.webrtc.org/42220378 - propagate Environment into RateAllocators
+  // so that they wouldn't use global field trials string to query field trials.
+  std::unique_ptr<VideoBitrateAllocator> Create(
+      const Environment& /*env*/,
       const VideoCodec& codec) override {
     // TODO(https://crbug.com/webrtc/14884): Update SvcRateAllocator to
     // support simulcast and use it for VP9/AV1 simulcast as well.

diff --git a/api/video/video_bitrate_allocator_factory.h b/api/video/video_bitrate_allocator_factory.h
@@ -13,8 +13,10 @@
 
 #include <memory>
 
+#include "api/environment/environment.h"
 #include "api/video/video_bitrate_allocator.h"
 #include "api/video_codecs/video_codec.h"
+#include "rtc_base/checks.h"
 
 namespace webrtc {
 
@@ -23,9 +25,19 @@ namespace webrtc {
 class VideoBitrateAllocatorFactory {
  public:
   virtual ~VideoBitrateAllocatorFactory() = default;
+
   // Creates a VideoBitrateAllocator for a specific video codec.
+  virtual std::unique_ptr<VideoBitrateAllocator> Create(
+      const Environment& env,
+      const VideoCodec& codec) {
+    return CreateVideoBitrateAllocator(codec);
+  }
   virtual std::unique_ptr<VideoBitrateAllocator> CreateVideoBitrateAllocator(
-      const VideoCodec& codec) = 0;
+      const VideoCodec& codec) {
+    // Newer code shouldn't call this function,
+    // Older code should implement it in derived classes.
+    RTC_CHECK_NOTREACHED();
+  }
 };
 
 }  // namespace webrtc

diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
@@ -1363,9 +1363,9 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) {
   std::unique_ptr<webrtc::MockVideoBitrateAllocatorFactory>
       rate_allocator_factory =
           std::make_unique<webrtc::MockVideoBitrateAllocatorFactory>();
-  EXPECT_CALL(*rate_allocator_factory,
-              CreateVideoBitrateAllocator(Field(&webrtc::VideoCodec::codecType,
-                                                webrtc::kVideoCodecVP8)))
+  EXPECT_CALL(
+      *rate_allocator_factory,
+      Create(_, Field(&webrtc::VideoCodec::codecType, webrtc::kVideoCodecVP8)))
       .WillOnce(
           [] { return std::make_unique<webrtc::MockVideoBitrateAllocator>(); });
   webrtc::FieldTrialBasedConfig trials;

diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn
@@ -1204,6 +1204,7 @@ if (rtc_include_tests) {
       "../../api:scoped_refptr",
       "../../api:simulcast_test_fixture_api",
       "../../api:videocodec_test_fixture_api",
+      "../../api/environment",
       "../../api/environment:environment_factory",
       "../../api/task_queue",
       "../../api/task_queue:default_task_queue_factory",

diff --git a/modules/video_coding/codecs/test/video_codec_test.cc b/modules/video_coding/codecs/test/video_codec_test.cc
@@ -319,7 +319,7 @@ TEST_P(SpatialQualityTest, SpatialQuality) {
   VideoSourceSettings source_settings = ToSourceSettings(video_info);
 
   EncodingSettings encoding_settings = VideoCodecTester::CreateEncodingSettings(
-      codec_type, /*scalability_mode=*/"L1T1", width, height,
+      env, codec_type, /*scalability_mode=*/"L1T1", width, height,
       {DataRate::KilobitsPerSec(bitrate_kbps)},
       Frequency::Hertz(framerate_fps));
 
@@ -398,14 +398,14 @@ TEST_P(BitrateAdaptationTest, BitrateAdaptation) {
   VideoSourceSettings source_settings = ToSourceSettings(video_info);
 
   EncodingSettings encoding_settings = VideoCodecTester::CreateEncodingSettings(
-      codec_type, /*scalability_mode=*/"L1T1",
+      env, codec_type, /*scalability_mode=*/"L1T1",
       /*width=*/640, /*height=*/360,
       {DataRate::KilobitsPerSec(bitrate_kbps.first)},
       /*framerate=*/Frequency::Hertz(30));
 
   EncodingSettings encoding_settings2 =
       VideoCodecTester::CreateEncodingSettings(
-          codec_type, /*scalability_mode=*/"L1T1",
+          env, codec_type, /*scalability_mode=*/"L1T1",
           /*width=*/640, /*height=*/360,
           {DataRate::KilobitsPerSec(bitrate_kbps.second)},
           /*framerate=*/Frequency::Hertz(30));
@@ -484,14 +484,14 @@ TEST_P(FramerateAdaptationTest, FramerateAdaptation) {
   VideoSourceSettings source_settings = ToSourceSettings(video_info);
 
   EncodingSettings encoding_settings = VideoCodecTester::CreateEncodingSettings(
-      codec_type, /*scalability_mode=*/"L1T1",
+      env, codec_type, /*scalability_mode=*/"L1T1",
       /*width=*/640, /*height=*/360,
       /*bitrate=*/{DataRate::KilobitsPerSec(512)},
       Frequency::Hertz(framerate_fps.first));
 
   EncodingSettings encoding_settings2 =
       VideoCodecTester::CreateEncodingSettings(
-          codec_type, /*scalability_mode=*/"L1T1",
+          env, codec_type, /*scalability_mode=*/"L1T1",
           /*width=*/640, /*height=*/360,
           /*bitrate=*/{DataRate::KilobitsPerSec(512)},
           Frequency::Hertz(framerate_fps.second));
@@ -571,7 +571,7 @@ TEST(VideoCodecTest, DISABLED_EncodeDecode) {
           .value_or(absl::GetFlag(FLAGS_input_framerate_fps)));
 
   EncodingSettings encoding_settings = VideoCodecTester::CreateEncodingSettings(
-      CodecNameToCodecType(absl::GetFlag(FLAGS_encoder)),
+      env, CodecNameToCodecType(absl::GetFlag(FLAGS_encoder)),
       absl::GetFlag(FLAGS_scalability_mode),
       absl::GetFlag(FLAGS_width).value_or(absl::GetFlag(FLAGS_input_width)),
       absl::GetFlag(FLAGS_height).value_or(absl::GetFlag(FLAGS_input_height)),

diff --git a/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc b/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc
@@ -446,6 +446,7 @@ VideoCodecTestFixtureImpl::VideoCodecTestFixtureImpl(Config config)
                            webrtc::LibvpxVp9DecoderTemplateAdapter,
                            webrtc::OpenH264DecoderTemplateAdapter,
                            webrtc::Dav1dDecoderTemplateAdapter>>()),
+      env_(CreateEnvironment()),
       config_(config) {}
 
 VideoCodecTestFixtureImpl::VideoCodecTestFixtureImpl(
@@ -454,6 +455,7 @@ VideoCodecTestFixtureImpl::VideoCodecTestFixtureImpl(
     std::unique_ptr<VideoEncoderFactory> encoder_factory)
     : encoder_factory_(std::move(encoder_factory)),
       decoder_factory_(std::move(decoder_factory)),
+      env_(CreateEnvironment()),
       config_(config) {}
 
 VideoCodecTestFixtureImpl::~VideoCodecTestFixtureImpl() = default;
@@ -695,8 +697,6 @@ void VideoCodecTestFixtureImpl::VerifyVideoStatistic(
 }
 
 bool VideoCodecTestFixtureImpl::CreateEncoderAndDecoder() {
-  const Environment env = CreateEnvironment();
-
   SdpVideoFormat encoder_format(CreateSdpVideoFormat(config_));
   SdpVideoFormat decoder_format = encoder_format;
 
@@ -711,7 +711,7 @@ bool VideoCodecTestFixtureImpl::CreateEncoderAndDecoder() {
     decoder_format = *config_.decoder_format;
   }
 
-  encoder_ = encoder_factory_->Create(env, encoder_format);
+  encoder_ = encoder_factory_->Create(env_, encoder_format);
   EXPECT_TRUE(encoder_) << "Encoder not successfully created.";
   if (encoder_ == nullptr) {
     return false;
@@ -721,7 +721,7 @@ bool VideoCodecTestFixtureImpl::CreateEncoderAndDecoder() {
       config_.NumberOfSimulcastStreams(), config_.NumberOfSpatialLayers());
   for (size_t i = 0; i < num_simulcast_or_spatial_layers; ++i) {
     std::unique_ptr<VideoDecoder> decoder =
-        decoder_factory_->Create(env, decoder_format);
+        decoder_factory_->Create(env_, decoder_format);
     EXPECT_TRUE(decoder) << "Decoder not successfully created.";
     if (decoder == nullptr) {
       return false;
@@ -818,7 +818,7 @@ bool VideoCodecTestFixtureImpl::SetUpAndInitObjects(
 
   task_queue->SendTask([this]() {
     processor_ = std::make_unique<VideoProcessor>(
-        encoder_.get(), &decoders_, source_frame_reader_.get(), config_,
+        env_, encoder_.get(), &decoders_, source_frame_reader_.get(), config_,
         &stats_, &encoded_frame_writers_,
         decoded_frame_writers_.empty() ? nullptr : &decoded_frame_writers_);
   });

diff --git a/modules/video_coding/codecs/test/videocodec_test_fixture_impl.h b/modules/video_coding/codecs/test/videocodec_test_fixture_impl.h
@@ -15,6 +15,7 @@
 #include <string>
 #include <vector>
 
+#include "api/environment/environment.h"
 #include "api/test/videocodec_test_fixture.h"
 #include "api/video_codecs/video_decoder_factory.h"
 #include "api/video_codecs/video_encoder_factory.h"
@@ -92,6 +93,7 @@ class VideoCodecTestFixtureImpl : public VideoCodecTestFixture {
   VideoProcessor::VideoDecoderList decoders_;
 
   // Helper objects.
+  const Environment env_;
   Config config_;
   VideoCodecTestStatsImpl stats_;
   std::unique_ptr<FrameReader> source_frame_reader_;

diff --git a/modules/video_coding/codecs/test/videoprocessor.cc b/modules/video_coding/codecs/test/videoprocessor.cc
@@ -135,7 +135,8 @@ void CalculateFrameQuality(const I420BufferInterface& ref_buffer,
 
 }  // namespace
 
-VideoProcessor::VideoProcessor(webrtc::VideoEncoder* encoder,
+VideoProcessor::VideoProcessor(const Environment& env,
+                               webrtc::VideoEncoder* encoder,
                                VideoDecoderList* decoders,
                                FrameReader* input_frame_reader,
                                const VideoCodecTestFixture::Config& config,
@@ -150,9 +151,9 @@ VideoProcessor::VideoProcessor(webrtc::VideoEncoder* encoder,
       stats_(stats),
       encoder_(encoder),
       decoders_(decoders),
-      bitrate_allocator_(
-          CreateBuiltinVideoBitrateAllocatorFactory()
-              ->CreateVideoBitrateAllocator(config_.codec_settings)),
+      bitrate_allocator_(CreateBuiltinVideoBitrateAllocatorFactory()->Create(
+          env,
+          config_.codec_settings)),
       encode_callback_(this),
       input_frame_reader_(input_frame_reader),
       merged_encoded_frames_(num_simulcast_or_spatial_layers_),

diff --git a/modules/video_coding/codecs/test/videoprocessor.h b/modules/video_coding/codecs/test/videoprocessor.h
@@ -20,6 +20,7 @@
 #include <vector>
 
 #include "absl/types/optional.h"
+#include "api/environment/environment.h"
 #include "api/sequence_checker.h"
 #include "api/task_queue/task_queue_base.h"
 #include "api/test/videocodec_test_fixture.h"
@@ -61,7 +62,8 @@ class VideoProcessor {
   using FrameWriterList = std::vector<std::unique_ptr<FrameWriter>>;
   using FrameStatistics = VideoCodecTestStats::FrameStatistics;
 
-  VideoProcessor(webrtc::VideoEncoder* encoder,
+  VideoProcessor(const Environment& env,
+                 VideoEncoder* encoder,
                  VideoDecoderList* decoders,
                  FrameReader* input_frame_reader,
                  const VideoCodecTestFixture::Config& config,

diff --git a/modules/video_coding/codecs/test/videoprocessor_unittest.cc b/modules/video_coding/codecs/test/videoprocessor_unittest.cc
@@ -12,6 +12,7 @@
 
 #include <memory>
 
+#include "api/environment/environment_factory.h"
 #include "api/scoped_refptr.h"
 #include "api/test/mock_video_decoder.h"
 #include "api/test/mock_video_encoder.h"
@@ -53,8 +54,9 @@ class VideoProcessorTest : public ::testing::Test {
     ExpectInit();
     q_.SendTask([this] {
       video_processor_ = std::make_unique<VideoProcessor>(
-          &encoder_mock_, &decoders_, &frame_reader_mock_, config_, &stats_,
-          &encoded_frame_writers_, /*decoded_frame_writers=*/nullptr);
+          CreateEnvironment(), &encoder_mock_, &decoders_, &frame_reader_mock_,
+          config_, &stats_, &encoded_frame_writers_,
+          /*decoded_frame_writers=*/nullptr);
     });
   }
 

diff --git a/modules/video_coding/video_codec_initializer_unittest.cc b/modules/video_coding/video_codec_initializer_unittest.cc
@@ -16,6 +16,8 @@
 #include <memory>
 
 #include "absl/types/optional.h"
+#include "api/environment/environment.h"
+#include "api/environment/environment_factory.h"
 #include "api/scoped_refptr.h"
 #include "api/test/mock_fec_controller_override.h"
 #include "api/video/builtin_video_bitrate_allocator_factory.h"
@@ -92,10 +94,10 @@ class VideoCodecInitializerTest : public ::testing::Test {
 
   void InitializeCodec() {
     frame_buffer_controller_.reset();
-    codec_out_ =
-        VideoCodecInitializer::SetupCodec(field_trials_, config_, streams_);
-    bitrate_allocator_ = CreateBuiltinVideoBitrateAllocatorFactory()
-                             ->CreateVideoBitrateAllocator(codec_out_);
+    codec_out_ = VideoCodecInitializer::SetupCodec(env_.field_trials(), config_,
+                                                   streams_);
+    bitrate_allocator_ =
+        CreateBuiltinVideoBitrateAllocatorFactory()->Create(env_, codec_out_);
     RTC_CHECK(bitrate_allocator_);
 
     // Make sure temporal layers instances have been created.
@@ -137,7 +139,7 @@ class VideoCodecInitializerTest : public ::testing::Test {
     return stream;
   }
 
-  const test::ExplicitKeyValueConfig field_trials_{""};
+  const Environment env_ = CreateEnvironment();
   MockFecControllerOverride fec_controller_override_;
 
   // Input settings.
@@ -505,7 +507,7 @@ TEST_F(VideoCodecInitializerTest, Av1SingleSpatialLayerBitratesAreConsistent) {
   streams[0].scalability_mode = ScalabilityMode::kL1T2;
 
   VideoCodec codec =
-      VideoCodecInitializer::SetupCodec(field_trials_, config, streams);
+      VideoCodecInitializer::SetupCodec(env_.field_trials(), config, streams);
 
   EXPECT_GE(codec.spatialLayers[0].targetBitrate,
             codec.spatialLayers[0].minBitrate);
@@ -520,7 +522,7 @@ TEST_F(VideoCodecInitializerTest, Av1TwoSpatialLayersBitratesAreConsistent) {
   streams[0].scalability_mode = ScalabilityMode::kL2T2;
 
   VideoCodec codec =
-      VideoCodecInitializer::SetupCodec(field_trials_, config, streams);
+      VideoCodecInitializer::SetupCodec(env_.field_trials(), config, streams);
 
   EXPECT_GE(codec.spatialLayers[0].targetBitrate,
             codec.spatialLayers[0].minBitrate);
@@ -541,7 +543,7 @@ TEST_F(VideoCodecInitializerTest, Av1TwoSpatialLayersActiveByDefault) {
   config.spatial_layers = {};
 
   VideoCodec codec =
-      VideoCodecInitializer::SetupCodec(field_trials_, config, streams);
+      VideoCodecInitializer::SetupCodec(env_.field_trials(), config, streams);
 
   EXPECT_TRUE(codec.spatialLayers[0].active);
   EXPECT_TRUE(codec.spatialLayers[1].active);
@@ -557,7 +559,7 @@ TEST_F(VideoCodecInitializerTest, Av1TwoSpatialLayersOneDeactivated) {
   config.spatial_layers[1].active = false;
 
   VideoCodec codec =
-      VideoCodecInitializer::SetupCodec(field_trials_, config, streams);
+      VideoCodecInitializer::SetupCodec(env_.field_trials(), config, streams);
 
   EXPECT_TRUE(codec.spatialLayers[0].active);
   EXPECT_FALSE(codec.spatialLayers[1].active);
@@ -575,7 +577,7 @@ TEST_F(VideoCodecInitializerTest, Vp9SingleSpatialLayerBitratesAreConsistent) {
   streams[0].scalability_mode = ScalabilityMode::kL1T2;
 
   VideoCodec codec =
-      VideoCodecInitializer::SetupCodec(field_trials_, config, streams);
+      VideoCodecInitializer::SetupCodec(env_.field_trials(), config, streams);
 
   EXPECT_EQ(1u, codec.VP9()->numberOfSpatialLayers);
   // Target is consistent with min and max (min <= target <= max).
@@ -603,7 +605,7 @@ TEST_F(VideoCodecInitializerTest, Vp9TwoSpatialLayersBitratesAreConsistent) {
   streams[0].scalability_mode = ScalabilityMode::kL2T2;
 
   VideoCodec codec =
-      VideoCodecInitializer::SetupCodec(field_trials_, config, streams);
+      VideoCodecInitializer::SetupCodec(env_.field_trials(), config, streams);
 
   EXPECT_EQ(2u, codec.VP9()->numberOfSpatialLayers);
   EXPECT_GE(codec.spatialLayers[0].targetBitrate,
@@ -627,14 +629,15 @@ TEST_F(VideoCodecInitializerTest, UpdatesVp9SpecificFieldsWithScalabilityMode) {
   streams[0].scalability_mode = ScalabilityMode::kL2T3_KEY;
 
   VideoCodec codec =
-      VideoCodecInitializer::SetupCodec(field_trials_, config, streams);
+      VideoCodecInitializer::SetupCodec(env_.field_trials(), config, streams);
 
   EXPECT_EQ(codec.VP9()->numberOfSpatialLayers, 2u);
   EXPECT_EQ(codec.VP9()->numberOfTemporalLayers, 3u);
   EXPECT_EQ(codec.VP9()->interLayerPred, InterLayerPredMode::kOnKeyPic);
 
   streams[0].scalability_mode = ScalabilityMode::kS3T1;
-  codec = VideoCodecInitializer::SetupCodec(field_trials_, config, streams);
+  codec =
+      VideoCodecInitializer::SetupCodec(env_.field_trials(), config, streams);
 
   EXPECT_EQ(codec.VP9()->numberOfSpatialLayers, 3u);
   EXPECT_EQ(codec.VP9()->numberOfTemporalLayers, 1u);

diff --git a/rtc_tools/video_encoder/video_encoder.cc b/rtc_tools/video_encoder/video_encoder.cc
@@ -333,13 +333,13 @@ class TestVideoEncoderFactoryWrapper final {
     RTC_CHECK_EQ(ret, WEBRTC_VIDEO_CODEC_OK);
 
     // Set bitrates.
-    std::unique_ptr<webrtc::VideoBitrateAllocator> bitrate_allocator_;
-    bitrate_allocator_ = webrtc::CreateBuiltinVideoBitrateAllocatorFactory()
-                             ->CreateVideoBitrateAllocator(video_codec_setting);
-    RTC_CHECK(bitrate_allocator_);
+    std::unique_ptr<VideoBitrateAllocator> bitrate_allocator =
+        CreateBuiltinVideoBitrateAllocatorFactory()->Create(
+            env, video_codec_setting);
+    RTC_CHECK(bitrate_allocator);
 
     webrtc::VideoBitrateAllocation allocation =
-        bitrate_allocator_->GetAllocation(bitrate_kbps * 1000, frame_rate_fps);
+        bitrate_allocator->GetAllocation(bitrate_kbps * 1000, frame_rate_fps);
     RTC_LOG(LS_INFO) << allocation.ToString();
 
     video_encoder->SetRates(webrtc::VideoEncoder::RateControlParameters(