From f8bc1169a88660b9bba04654bc2f8c8289ba99c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Thu, 9 Mar 2023 12:51:52 +0100 Subject: [PATCH] Test that VideoCodecInitializer propagates VP9 resolution alignment. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GetSvcConfig() call here[1] can result in resolution alignment due to [2], which gets propagated to the output VideoCodec due to [3]. This CL adds test coverage for this part. It also removes some comments that are no longer true and updates VideoCodecInitializerTest's SetUpFor() to make number of simulcast streams explicit. [1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/modules/video_coding/video_codec_initializer.cc;l=251;drc=e4a304ed4da869ab6131a06b3e8b7e985f50229d [2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/modules/video_coding/codecs/vp9/svc_config.cc;l=112;drc=31acc7339cf658ce82c7ec490ba38d67170920ed [3] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/modules/video_coding/video_codec_initializer.cc;l=285;drc=e4a304ed4da869ab6131a06b3e8b7e985f50229d Bug: webrtc:14884 Change-Id: Id22e36aebab573f53d15dca688642d32c8c4be7a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296762 Commit-Queue: Henrik Boström Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/main@{#39514} --- .../include/video_codec_initializer.h | 5 -- .../video_coding/video_codec_initializer.cc | 1 - .../video_codec_initializer_unittest.cc | 55 +++++++++++++------ 3 files changed, 38 insertions(+), 23 deletions(-) diff --git a/modules/video_coding/include/video_codec_initializer.h b/modules/video_coding/include/video_codec_initializer.h index 270c4dbcd1..2d10ee45a8 100644 --- a/modules/video_coding/include/video_codec_initializer.h +++ b/modules/video_coding/include/video_codec_initializer.h @@ -19,17 +19,12 @@ namespace webrtc { -class VideoBitrateAllocator; class VideoCodec; class VideoCodecInitializer { public: // Takes a VideoEncoderConfig and the VideoStream configuration and // translates them into the old school VideoCodec type. - // It also creates a VideoBitrateAllocator instance, suitable for the codec - // type used. For instance, VP8 will create an allocator than can handle - // simulcast and temporal layering. - // GetBitrateAllocator is called implicitly from here, no need to call again. static bool SetupCodec(const VideoEncoderConfig& config, const std::vector& streams, VideoCodec* codec); diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc index e1885d74c8..bf59d89db7 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc @@ -18,7 +18,6 @@ #include "absl/types/optional.h" #include "api/scoped_refptr.h" #include "api/units/data_rate.h" -#include "api/video/video_bitrate_allocation.h" #include "api/video_codecs/video_encoder.h" #include "modules/video_coding/codecs/av1/av1_svc_config.h" #include "modules/video_coding/codecs/vp8/vp8_scalability.h" diff --git a/modules/video_coding/video_codec_initializer_unittest.cc b/modules/video_coding/video_codec_initializer_unittest.cc index 0e6f2dfca2..04d6354914 100644 --- a/modules/video_coding/video_codec_initializer_unittest.cc +++ b/modules/video_coding/video_codec_initializer_unittest.cc @@ -58,7 +58,8 @@ class VideoCodecInitializerTest : public ::testing::Test { protected: void SetUpFor(VideoCodecType type, - int num_spatial_streams, + absl::optional num_simulcast_streams, + absl::optional num_spatial_streams, int num_temporal_streams, bool screenshare) { config_ = VideoEncoderConfig(); @@ -70,14 +71,18 @@ class VideoCodecInitializerTest : public ::testing::Test { } if (type == VideoCodecType::kVideoCodecVP8) { - config_.number_of_streams = num_spatial_streams; + ASSERT_TRUE(num_simulcast_streams.has_value()); + ASSERT_FALSE(num_spatial_streams.has_value()); + config_.number_of_streams = num_simulcast_streams.value(); VideoCodecVP8 vp8_settings = VideoEncoder::GetDefaultVp8Settings(); vp8_settings.numberOfTemporalLayers = num_temporal_streams; config_.encoder_specific_settings = rtc::make_ref_counted< webrtc::VideoEncoderConfig::Vp8EncoderSpecificSettings>(vp8_settings); } else if (type == VideoCodecType::kVideoCodecVP9) { + ASSERT_FALSE(num_simulcast_streams.has_value()); + ASSERT_TRUE(num_spatial_streams.has_value()); VideoCodecVP9 vp9_settings = VideoEncoder::GetDefaultVp9Settings(); - vp9_settings.numberOfSpatialLayers = num_spatial_streams; + vp9_settings.numberOfSpatialLayers = num_spatial_streams.value(); vp9_settings.numberOfTemporalLayers = num_temporal_streams; config_.encoder_specific_settings = rtc::make_ref_counted< webrtc::VideoEncoderConfig::Vp9EncoderSpecificSettings>(vp9_settings); @@ -147,7 +152,7 @@ class VideoCodecInitializerTest : public ::testing::Test { }; TEST_F(VideoCodecInitializerTest, SingleStreamVp8Screenshare) { - SetUpFor(VideoCodecType::kVideoCodecVP8, 1, 1, true); + SetUpFor(VideoCodecType::kVideoCodecVP8, 1, absl::nullopt, 1, true); streams_.push_back(DefaultStream()); EXPECT_TRUE(InitializeCodec()); @@ -160,7 +165,7 @@ TEST_F(VideoCodecInitializerTest, SingleStreamVp8Screenshare) { } TEST_F(VideoCodecInitializerTest, SingleStreamVp8ScreenshareInactive) { - SetUpFor(VideoCodecType::kVideoCodecVP8, 1, 1, true); + SetUpFor(VideoCodecType::kVideoCodecVP8, 1, absl::nullopt, 1, true); VideoStream inactive_stream = DefaultStream(); inactive_stream.active = false; streams_.push_back(inactive_stream); @@ -175,7 +180,7 @@ TEST_F(VideoCodecInitializerTest, SingleStreamVp8ScreenshareInactive) { } TEST_F(VideoCodecInitializerTest, TemporalLayeredVp8ScreenshareConference) { - SetUpFor(VideoCodecType::kVideoCodecVP8, 1, 2, true); + SetUpFor(VideoCodecType::kVideoCodecVP8, 1, absl::nullopt, 2, true); streams_.push_back(DefaultScreenshareStream()); EXPECT_TRUE(InitializeCodec()); bitrate_allocator_->SetLegacyConferenceMode(true); @@ -192,7 +197,7 @@ TEST_F(VideoCodecInitializerTest, TemporalLayeredVp8ScreenshareConference) { } TEST_F(VideoCodecInitializerTest, TemporalLayeredVp8Screenshare) { - SetUpFor(VideoCodecType::kVideoCodecVP8, 1, 2, true); + SetUpFor(VideoCodecType::kVideoCodecVP8, 1, absl::nullopt, 2, true); streams_.push_back(DefaultScreenshareStream()); EXPECT_TRUE(InitializeCodec()); @@ -207,7 +212,7 @@ TEST_F(VideoCodecInitializerTest, TemporalLayeredVp8Screenshare) { } TEST_F(VideoCodecInitializerTest, SimulcastVp8Screenshare) { - SetUpFor(VideoCodecType::kVideoCodecVP8, 2, 1, true); + SetUpFor(VideoCodecType::kVideoCodecVP8, 2, absl::nullopt, 1, true); streams_.push_back(DefaultScreenshareStream()); VideoStream video_stream = DefaultStream(); video_stream.max_framerate = kScreenshareDefaultFramerate; @@ -231,7 +236,7 @@ TEST_F(VideoCodecInitializerTest, SimulcastVp8Screenshare) { // Tests that when a video stream is inactive, then the bitrate allocation will // be 0 for that stream. TEST_F(VideoCodecInitializerTest, SimulcastVp8ScreenshareInactive) { - SetUpFor(VideoCodecType::kVideoCodecVP8, 2, 1, true); + SetUpFor(VideoCodecType::kVideoCodecVP8, 2, absl::nullopt, 1, true); streams_.push_back(DefaultScreenshareStream()); VideoStream inactive_video_stream = DefaultStream(); inactive_video_stream.active = false; @@ -256,7 +261,7 @@ TEST_F(VideoCodecInitializerTest, SimulcastVp8ScreenshareInactive) { TEST_F(VideoCodecInitializerTest, HighFpsSimulcastVp8Screenshare) { // Two simulcast streams, the lower one using legacy settings (two temporal // streams, 5fps), the higher one using 3 temporal streams and 30fps. - SetUpFor(VideoCodecType::kVideoCodecVP8, 2, 3, true); + SetUpFor(VideoCodecType::kVideoCodecVP8, 2, absl::nullopt, 3, true); streams_.push_back(DefaultScreenshareStream()); VideoStream video_stream = DefaultStream(); video_stream.num_temporal_layers = 3; @@ -280,13 +285,13 @@ TEST_F(VideoCodecInitializerTest, HighFpsSimulcastVp8Screenshare) { } TEST_F(VideoCodecInitializerTest, SingleStreamMultiplexCodec) { - SetUpFor(VideoCodecType::kVideoCodecMultiplex, 1, 1, true); + SetUpFor(VideoCodecType::kVideoCodecMultiplex, absl::nullopt, 1, 1, true); streams_.push_back(DefaultStream()); EXPECT_TRUE(InitializeCodec()); } TEST_F(VideoCodecInitializerTest, Vp9SvcDefaultLayering) { - SetUpFor(VideoCodecType::kVideoCodecVP9, 3, 3, false); + SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 3, 3, false); VideoStream stream = DefaultStream(); stream.num_temporal_layers = 3; streams_.push_back(stream); @@ -297,7 +302,7 @@ TEST_F(VideoCodecInitializerTest, Vp9SvcDefaultLayering) { } TEST_F(VideoCodecInitializerTest, Vp9SvcAdjustedLayering) { - SetUpFor(VideoCodecType::kVideoCodecVP9, 3, 3, false); + SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 3, 3, false); VideoStream stream = DefaultStream(); stream.num_temporal_layers = 3; // Set resolution which is only enough to produce 2 spatial layers. @@ -312,7 +317,7 @@ TEST_F(VideoCodecInitializerTest, Vp9SvcAdjustedLayering) { TEST_F(VideoCodecInitializerTest, Vp9SingleSpatialLayerMaxBitrateIsEqualToCodecMaxBitrate) { - SetUpFor(VideoCodecType::kVideoCodecVP9, 1, 3, false); + SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 1, 3, false); VideoStream stream = DefaultStream(); stream.num_temporal_layers = 3; streams_.push_back(stream); @@ -324,7 +329,7 @@ TEST_F(VideoCodecInitializerTest, TEST_F(VideoCodecInitializerTest, Vp9SingleSpatialLayerTargetBitrateIsEqualToCodecMaxBitrate) { - SetUpFor(VideoCodecType::kVideoCodecVP9, 1, 1, true); + SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 1, 1, true); VideoStream stream = DefaultStream(); stream.num_temporal_layers = 1; streams_.push_back(stream); @@ -339,7 +344,7 @@ TEST_F(VideoCodecInitializerTest, // Request 3 spatial layers for 320x180 input. Actual number of layers will be // reduced to 1 due to low input resolution but SVC bitrate limits should be // applied. - SetUpFor(VideoCodecType::kVideoCodecVP9, 3, 3, false); + SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 3, 3, false); VideoStream stream = DefaultStream(); stream.width = 320; stream.height = 180; @@ -352,7 +357,7 @@ TEST_F(VideoCodecInitializerTest, } TEST_F(VideoCodecInitializerTest, Vp9DeactivateLayers) { - SetUpFor(VideoCodecType::kVideoCodecVP9, 3, 1, false); + SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 3, 1, false); VideoStream stream = DefaultStream(); streams_.push_back(stream); @@ -425,6 +430,22 @@ TEST_F(VideoCodecInitializerTest, Vp9DeactivateLayers) { EXPECT_FALSE(codec_out_.spatialLayers[2].active); } +TEST_F(VideoCodecInitializerTest, Vp9SvcResolutionAlignment) { + SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 3, 3, false); + VideoStream stream = DefaultStream(); + stream.width = 1281; + stream.height = 721; + stream.num_temporal_layers = 3; + streams_.push_back(stream); + + EXPECT_TRUE(InitializeCodec()); + EXPECT_EQ(codec_out_.width, 1280); + EXPECT_EQ(codec_out_.height, 720); + EXPECT_EQ(codec_out_.numberOfSimulcastStreams, 1); + EXPECT_EQ(codec_out_.simulcastStream[0].width, 1280); + EXPECT_EQ(codec_out_.simulcastStream[0].height, 720); +} + TEST_F(VideoCodecInitializerTest, Av1SingleSpatialLayerBitratesAreConsistent) { VideoEncoderConfig config; config.codec_type = VideoCodecType::kVideoCodecAV1;