From 89e140ccf7831b12fd94169501d4c4f2d0ab6e30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Fri, 10 Mar 2023 10:43:19 +0100 Subject: [PATCH] VideoCodecInitializer: Only update width/height in VP9 SVC path. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The width/height need to be updated in the VP9 SVC case since resolution alignment may be applied inside GetSvcConfig(). This is not needed in the VP9 simulcast case since we don't support simulcast + SVC combo and resolution alignment is not needed for non-SVC. This CL gates the "resolution update" behind "numberOfSimulcastStreams == 1". Bug: webrtc:14884 Change-Id: Ic3551721dcf6775fea6ff0c85fba48e88069fa5a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296766 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/main@{#39524} --- .../video_coding/video_codec_initializer.cc | 11 ++++- .../video_codec_initializer_unittest.cc | 40 +++++++++++++++---- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc index bf59d89db7..b0b16d06f4 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc @@ -283,8 +283,15 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( // This difference must be propagated to the stream configuration. video_codec.width = spatial_layers.back().width; video_codec.height = spatial_layers.back().height; - video_codec.simulcastStream[0].width = spatial_layers.back().width; - video_codec.simulcastStream[0].height = spatial_layers.back().height; + // Only propagate if we're not doing simulcast. Simulcast is assumed not + // to have multiple spatial layers, if we wanted to support simulcast+SVC + // combos we would need to calculate unique spatial layers per simulcast + // layer, but VideoCodec is not capable of expressing per-simulcastStream + // spatialLayers. + if (video_codec.numberOfSimulcastStreams == 1) { + video_codec.simulcastStream[0].width = spatial_layers.back().width; + video_codec.simulcastStream[0].height = spatial_layers.back().height; + } // Update layering settings. video_codec.VP9()->numberOfSpatialLayers = diff --git a/modules/video_coding/video_codec_initializer_unittest.cc b/modules/video_coding/video_codec_initializer_unittest.cc index 04d6354914..e6286804d5 100644 --- a/modules/video_coding/video_codec_initializer_unittest.cc +++ b/modules/video_coding/video_codec_initializer_unittest.cc @@ -70,16 +70,16 @@ class VideoCodecInitializerTest : public ::testing::Test { config_.content_type = VideoEncoderConfig::ContentType::kScreen; } - if (type == VideoCodecType::kVideoCodecVP8) { - ASSERT_TRUE(num_simulcast_streams.has_value()); - ASSERT_FALSE(num_spatial_streams.has_value()); + if (num_simulcast_streams.has_value()) { config_.number_of_streams = num_simulcast_streams.value(); + } + if (type == VideoCodecType::kVideoCodecVP8) { + ASSERT_FALSE(num_spatial_streams.has_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.value(); @@ -114,10 +114,13 @@ class VideoCodecInitializerTest : public ::testing::Test { return true; } - VideoStream DefaultStream() { + VideoStream DefaultStream( + int width = kDefaultWidth, + int height = kDefaultHeight, + absl::optional scalability_mode = absl::nullopt) { VideoStream stream; - stream.width = kDefaultWidth; - stream.height = kDefaultHeight; + stream.width = width; + stream.height = height; stream.max_framerate = kDefaultFrameRate; stream.min_bitrate_bps = kDefaultMinBitrateBps; stream.target_bitrate_bps = kDefaultTargetBitrateBps; @@ -125,6 +128,7 @@ class VideoCodecInitializerTest : public ::testing::Test { stream.max_qp = kDefaultMaxQp; stream.num_temporal_layers = 1; stream.active = true; + stream.scalability_mode = scalability_mode; return stream; } @@ -446,6 +450,28 @@ TEST_F(VideoCodecInitializerTest, Vp9SvcResolutionAlignment) { EXPECT_EQ(codec_out_.simulcastStream[0].height, 720); } +TEST_F(VideoCodecInitializerTest, Vp9SimulcastResolutions) { + // 3 x L1T3 + SetUpFor(VideoCodecType::kVideoCodecVP9, 3, 1, 3, false); + // Scalability mode has to be set on all layers to avoid legacy SVC paths. + streams_ = {DefaultStream(320, 180, ScalabilityMode::kL1T3), + DefaultStream(640, 360, ScalabilityMode::kL1T3), + DefaultStream(1280, 720, ScalabilityMode::kL1T3)}; + + EXPECT_TRUE(InitializeCodec()); + // This is expected to be the largest layer. + EXPECT_EQ(codec_out_.width, 1280); + EXPECT_EQ(codec_out_.height, 720); + // `simulcastStream` is expected to be the same as the input (same order). + EXPECT_EQ(codec_out_.numberOfSimulcastStreams, 3); + EXPECT_EQ(codec_out_.simulcastStream[0].width, 320); + EXPECT_EQ(codec_out_.simulcastStream[0].height, 180); + EXPECT_EQ(codec_out_.simulcastStream[1].width, 640); + EXPECT_EQ(codec_out_.simulcastStream[1].height, 360); + EXPECT_EQ(codec_out_.simulcastStream[2].width, 1280); + EXPECT_EQ(codec_out_.simulcastStream[2].height, 720); +} + TEST_F(VideoCodecInitializerTest, Av1SingleSpatialLayerBitratesAreConsistent) { VideoEncoderConfig config; config.codec_type = VideoCodecType::kVideoCodecAV1;