From e94dcefbd4050e9c10f74b3af6cccb7a5245fbc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Fri, 17 Mar 2023 14:14:04 +0100 Subject: [PATCH] Fix bug in SvcRateAllocator capping to VideoCodec.maxBitrate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When allocating bitrate, some parts of the coded directly uses the bitrate parameter, while others lets it be capped by VideoCodec.maxBitrate. This may result in an inconsistency between expected and actual number of temporal layers, causing a crash. Even better would be to update VideoCodecInitializer to not create VideoCodec instances where there's not enough maxBitrate to activate all spatial layers - but that's a much more complex issue. Bug: chromium:1423365 Change-Id: Ic74b68261ea6043f1795accdd9864319ab535435 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298041 Commit-Queue: Erik Språng Reviewed-by: Henrik Boström Cr-Commit-Position: refs/heads/main@{#39593} --- modules/video_coding/svc/svc_rate_allocator.cc | 5 ++--- .../svc/svc_rate_allocator_unittest.cc | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/modules/video_coding/svc/svc_rate_allocator.cc b/modules/video_coding/svc/svc_rate_allocator.cc index b6ae0d7430..f3514a1a77 100644 --- a/modules/video_coding/svc/svc_rate_allocator.cc +++ b/modules/video_coding/svc/svc_rate_allocator.cc @@ -253,8 +253,7 @@ VideoBitrateAllocation SvcRateAllocator::Allocate( hysteresis_factor = experiment_settings_.GetVideoHysteresisFactor(); } - DataRate stable_rate = - std::min(parameters.total_bitrate, parameters.stable_bitrate); + DataRate stable_rate = std::min(total_bitrate, parameters.stable_bitrate); // First check if bitrate has grown large enough to enable new layers. size_t num_enabled_with_hysteresis = FindNumEnabledLayers(stable_rate / hysteresis_factor); @@ -266,7 +265,7 @@ VideoBitrateAllocation SvcRateAllocator::Allocate( std::min(last_active_layer_count_, FindNumEnabledLayers(stable_rate)); } } else { - num_spatial_layers = FindNumEnabledLayers(parameters.total_bitrate); + num_spatial_layers = FindNumEnabledLayers(total_bitrate); } last_active_layer_count_ = num_spatial_layers; diff --git a/modules/video_coding/svc/svc_rate_allocator_unittest.cc b/modules/video_coding/svc/svc_rate_allocator_unittest.cc index b3a365d722..44d1eae667 100644 --- a/modules/video_coding/svc/svc_rate_allocator_unittest.cc +++ b/modules/video_coding/svc/svc_rate_allocator_unittest.cc @@ -361,6 +361,21 @@ TEST(SvcRateAllocatorTest, UsesScalabilityModeToGetNumberOfLayers) { EXPECT_EQ(allocation.GetSpatialLayerSum(2), 0u); } +TEST(SvcRateAllocatorTest, CapsAllocationToMaxBitrate) { + VideoCodec codec = Configure(1280, 720, 3, 3, false); + codec.maxBitrate = 70; // Cap the overall max bitrate to 70kbps. + SvcRateAllocator allocator = SvcRateAllocator(codec); + + // Allocate 3Mbps which should be enough for all layers. + VideoBitrateAllocation allocation = + allocator.Allocate(VideoBitrateAllocationParameters(3'000'000, 30)); + + // The 3Mbps should be capped to 70kbps, so only first layer is active. + EXPECT_EQ(allocation.GetSpatialLayerSum(0), 70'000u); + EXPECT_EQ(allocation.GetSpatialLayerSum(1), 0u); + EXPECT_EQ(allocation.GetSpatialLayerSum(2), 0u); +} + class SvcRateAllocatorTestParametrizedContentType : public ::testing::Test, public ::testing::WithParamInterface {