From 596ed251e16eba0c2d19bd8f80626ff57133c99d Mon Sep 17 00:00:00 2001 From: Florent Castelli Date: Wed, 17 Jul 2019 21:16:05 +0200 Subject: [PATCH] Don't assume all simulcast screenshare have 2 temporal layers The simulcast allocator would only set bitrates for the first 2 layers in conference_screenshare_mode. That would trigger an issue in the VP8 encoder initialization that expects to have growing bitrates for the layers (3rd layer would have the same bitrate as the 2nd one). Bug: webrtc:8785 Change-Id: Ic6c940b78022387841b28074b373be6b2f45cb15 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145922 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Florent Castelli Cr-Commit-Position: refs/heads/master@{#28598} --- .../utility/simulcast_rate_allocator.cc | 3 ++- .../utility/simulcast_rate_allocator_unittest.cc | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/modules/video_coding/utility/simulcast_rate_allocator.cc b/modules/video_coding/utility/simulcast_rate_allocator.cc index 8cc58138c2..ced01a7713 100644 --- a/modules/video_coding/utility/simulcast_rate_allocator.cc +++ b/modules/video_coding/utility/simulcast_rate_allocator.cc @@ -212,7 +212,8 @@ void SimulcastRateAllocator::DistributeAllocationToTemporalLayers( const bool conference_screenshare_mode = codec_.mode == VideoCodecMode::kScreensharing && ((num_spatial_streams == 1 && num_temporal_streams == 2) || // Legacy. - (num_spatial_streams > 1 && simulcast_id == 0)); // Simulcast. + (num_spatial_streams > 1 && simulcast_id == 0 && + num_temporal_streams == 2)); // Simulcast. if (conference_screenshare_mode) { // TODO(holmer): This is a "temporary" hack for screensharing, where we // interpret the startBitrate as the encoder target bitrate. This is diff --git a/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc b/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc index 471fcd0d60..d2918fb923 100644 --- a/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc +++ b/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc @@ -507,6 +507,22 @@ TEST_F(SimulcastRateAllocatorTest, ThreeStreamsMiddleInactive) { } } +TEST_F(SimulcastRateAllocatorTest, NonConferenceModeScreenshare) { + codec_.mode = VideoCodecMode::kScreensharing; + SetupCodec3SL3TL({true, true, true}); + CreateAllocator(); + + // Make sure we have enough bitrate for all 3 simulcast layers + const uint32_t bitrate = codec_.simulcastStream[0].maxBitrate + + codec_.simulcastStream[1].maxBitrate + + codec_.simulcastStream[2].maxBitrate; + const VideoBitrateAllocation alloc = GetAllocation(bitrate); + + EXPECT_EQ(alloc.GetTemporalLayerAllocation(0).size(), 3u); + EXPECT_EQ(alloc.GetTemporalLayerAllocation(1).size(), 3u); + EXPECT_EQ(alloc.GetTemporalLayerAllocation(2).size(), 3u); +} + class ScreenshareRateAllocationTest : public SimulcastRateAllocatorTest { public: void SetupConferenceScreenshare(bool use_simulcast, bool active = true) {