From 9bbd9598b80207eef5e7e8d0c7415e9a7e1a66ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Mon, 3 Apr 2023 14:21:15 +0200 Subject: [PATCH] Also apply VP9 bitrate's singlecast tweak in single active stream case. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We shouldn't treat VP9 simulcast {active,inactive,inactive} different from VP9 singlecast when it comes to bitrates, so the condition `config.simulcast_layers.size() <= 1` is updated to `video_codec.numberOfSimulcastStreams <= 1` which holds true in the "single active stream" case as well. This is consistent with existing logic, such as the fact that we use `SvcRateAllocator` instead of `SimulcastRateAllocator` when `numberOfSimulcastStreams <= 1`. Bug: webrtc:15061 Change-Id: I67fc78b9c0f97f1d607c91bbc689b06c3fd5cb48 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298920 Commit-Queue: Henrik Boström Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/main@{#39791} --- modules/video_coding/video_codec_initializer.cc | 2 +- modules/video_coding/video_codec_initializer_unittest.cc | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc index bcc82a1de7..43fab8c432 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc @@ -242,7 +242,7 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( if (spatial_layers.empty()) break; // Use codec bitrate limits if spatial layering is not requested. - if (config.simulcast_layers.size() <= 1 && + if (video_codec.numberOfSimulcastStreams <= 1 && ScalabilityModeToNumSpatialLayers(*scalability_mode) == 1) { spatial_layers.back().minBitrate = video_codec.minBitrate; spatial_layers.back().targetBitrate = video_codec.maxBitrate; diff --git a/modules/video_coding/video_codec_initializer_unittest.cc b/modules/video_coding/video_codec_initializer_unittest.cc index b9d8465aa8..b0edab6004 100644 --- a/modules/video_coding/video_codec_initializer_unittest.cc +++ b/modules/video_coding/video_codec_initializer_unittest.cc @@ -589,11 +589,17 @@ TEST_F(VideoCodecInitializerTest, Vp9SingleSpatialLayerBitratesAreConsistent) { EXPECT_TRUE(VideoCodecInitializer::SetupCodec(config, streams, &codec)); EXPECT_EQ(1u, codec.VP9()->numberOfSpatialLayers); + // Target is consistent with min and max (min <= target <= max). EXPECT_GE(codec.spatialLayers[0].targetBitrate, codec.spatialLayers[0].minBitrate); EXPECT_LE(codec.spatialLayers[0].targetBitrate, codec.spatialLayers[0].maxBitrate); - EXPECT_LT(codec.spatialLayers[0].minBitrate, kDefaultMinBitrateBps / 1000); + // In the single spatial layer case, the spatial layer bitrates are copied + // from the codec's bitrate which is the sum if VideoStream bitrates. In this + // case we only have a single VideoStream using default values. + EXPECT_EQ(codec.spatialLayers[0].minBitrate, kDefaultMinBitrateBps / 1000); + EXPECT_EQ(codec.spatialLayers[0].targetBitrate, kDefaultMaxBitrateBps / 1000); + EXPECT_EQ(codec.spatialLayers[0].maxBitrate, kDefaultMaxBitrateBps / 1000); } TEST_F(VideoCodecInitializerTest, Vp9TwoSpatialLayersBitratesAreConsistent) {