From 0f17f9ce28a6599def2c07f47b979937911b5efb Mon Sep 17 00:00:00 2001 From: Lu Liu Date: Wed, 17 Jan 2018 00:28:14 +0000 Subject: [PATCH] Revert "Enables/disables simulcast streams by allocating a bitrate of 0 to the spatial layer." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 18c4261339dc76b220e7c805e36b4ea6f3dd161d. Reason for revert: Broke internal tests Original change's description: > Enables/disables simulcast streams by allocating a bitrate of 0 to the spatial layer. > > Creates VideoStreams & VideoCodec.simulcastStreams with an active field, and then allocates 0 bitrate to simulcast streams that are inactive. This turns off the encoder for specific simulcast streams. > > Bug: webrtc:8653 > Change-Id: Id93b03dcd8d1191a7d3300bd77882c8af96ee469 > Reviewed-on: https://webrtc-review.googlesource.com/37740 > Reviewed-by: Stefan Holmer > Reviewed-by: Taylor Brandstetter > Reviewed-by: Erik Språng > Commit-Queue: Seth Hampson > Cr-Commit-Position: refs/heads/master@{#21646} TBR=deadbeef@webrtc.org,sprang@webrtc.org,stefan@webrtc.org,shampson@webrtc.org Change-Id: I0aeb743cbd2e8d564aa732c937587c25a4c49b09 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:8653 Reviewed-on: https://webrtc-review.googlesource.com/39883 Reviewed-by: Lu Liu Commit-Queue: Lu Liu Cr-Commit-Position: refs/heads/master@{#21647} --- common_types.cc | 1 - common_types.h | 5 - .../simulcast_encoder_adapter_unittest.cc | 4 - .../codecs/vp8/simulcast_rate_allocator.cc | 133 ++++----- .../codecs/vp8/simulcast_rate_allocator.h | 6 - .../codecs/vp8/simulcast_test_utility.h | 73 +---- .../codecs/vp8/simulcast_unittest.cc | 4 - .../default_video_bitrate_allocator.cc | 2 +- ...efault_video_bitrate_allocator_unittest.cc | 7 - .../simulcast_rate_allocator_unittest.cc | 256 +----------------- .../video_coding/video_codec_initializer.cc | 10 - .../video_codec_initializer_unittest.cc | 44 +-- test/encoder_settings.cc | 1 - test/video_codec_settings.h | 4 - video/payload_router.cc | 7 +- video/payload_router_unittest.cc | 28 +- video/video_quality_test.cc | 1 - video/video_send_stream.cc | 6 +- 18 files changed, 76 insertions(+), 516 deletions(-) diff --git a/common_types.cc b/common_types.cc index 61e58b766f..07f77b7777 100644 --- a/common_types.cc +++ b/common_types.cc @@ -31,7 +31,6 @@ VideoCodec::VideoCodec() minBitrate(0), targetBitrate(0), maxFramerate(0), - active(true), qpMax(0), numberOfSimulcastStreams(0), simulcastStream(), diff --git a/common_types.h b/common_types.h index 5df6b02669..9f1ad8f9f7 100644 --- a/common_types.h +++ b/common_types.h @@ -509,7 +509,6 @@ struct SimulcastStream { unsigned int targetBitrate; // kilobits/sec. unsigned int minBitrate; // kilobits/sec. unsigned int qpMax; // minimum quality - bool active; // encoded and sent. }; struct SpatialLayer { @@ -541,10 +540,6 @@ class VideoCodec { uint32_t maxFramerate; - // This enables/disables VP9 and H264 encoding/sending by allocating - // 0 bitrate if inactive. - bool active; - unsigned int qpMax; unsigned char numberOfSimulcastStreams; SimulcastStream simulcastStream[kMaxSimulcastStreams]; diff --git a/media/engine/simulcast_encoder_adapter_unittest.cc b/media/engine/simulcast_encoder_adapter_unittest.cc index d8fc4b6d78..8087d7be0b 100644 --- a/media/engine/simulcast_encoder_adapter_unittest.cc +++ b/media/engine/simulcast_encoder_adapter_unittest.cc @@ -73,10 +73,6 @@ TEST_F(TestSimulcastEncoderAdapter, TestDisablingStreams) { TestVp8Simulcast::TestDisablingStreams(); } -TEST_F(TestSimulcastEncoderAdapter, TestActiveStreams) { - TestVp8Simulcast::TestActiveStreams(); -} - TEST_F(TestSimulcastEncoderAdapter, TestSwitchingToOneStream) { TestVp8Simulcast::TestSwitchingToOneStream(); } diff --git a/modules/video_coding/codecs/vp8/simulcast_rate_allocator.cc b/modules/video_coding/codecs/vp8/simulcast_rate_allocator.cc index b865f5eff6..b2b3334815 100644 --- a/modules/video_coding/codecs/vp8/simulcast_rate_allocator.cc +++ b/modules/video_coding/codecs/vp8/simulcast_rate_allocator.cc @@ -37,92 +37,56 @@ void SimulcastRateAllocator::OnTemporalLayersCreated(int simulcast_id, BitrateAllocation SimulcastRateAllocator::GetAllocation( uint32_t total_bitrate_bps, uint32_t framerate) { - BitrateAllocation allocated_bitrates_bps; - DistributeAllocationToSimulcastLayers(total_bitrate_bps, - &allocated_bitrates_bps); - DistributeAllocationToTemporalLayers(framerate, &allocated_bitrates_bps); - return allocated_bitrates_bps; -} - -void SimulcastRateAllocator::DistributeAllocationToSimulcastLayers( - uint32_t total_bitrate_bps, - BitrateAllocation* allocated_bitrates_bps) { uint32_t left_to_allocate = total_bitrate_bps; if (codec_.maxBitrate && codec_.maxBitrate * 1000 < left_to_allocate) left_to_allocate = codec_.maxBitrate * 1000; + BitrateAllocation allocated_bitrates_bps; if (codec_.numberOfSimulcastStreams == 0) { // No simulcast, just set the target as this has been capped already. - if (codec_.active) { - allocated_bitrates_bps->SetBitrate( - 0, 0, std::max(codec_.minBitrate * 1000, left_to_allocate)); + allocated_bitrates_bps.SetBitrate( + 0, 0, std::max(codec_.minBitrate * 1000, left_to_allocate)); + } else { + // Always allocate enough bitrate for the minimum bitrate of the first + // layer. Suspending below min bitrate is controlled outside the codec + // implementation and is not overridden by this. + left_to_allocate = + std::max(codec_.simulcastStream[0].minBitrate * 1000, left_to_allocate); + + // Begin by allocating bitrate to simulcast streams, putting all bitrate in + // temporal layer 0. We'll then distribute this bitrate, across potential + // temporal layers, when stream allocation is done. + + // Allocate up to the target bitrate for each simulcast layer. + size_t layer = 0; + for (; layer < codec_.numberOfSimulcastStreams; ++layer) { + const SimulcastStream& stream = codec_.simulcastStream[layer]; + if (left_to_allocate < stream.minBitrate * 1000) + break; + uint32_t allocation = + std::min(left_to_allocate, stream.targetBitrate * 1000); + allocated_bitrates_bps.SetBitrate(layer, 0, allocation); + RTC_DCHECK_LE(allocation, left_to_allocate); + left_to_allocate -= allocation; } - return; - } - // Find the first active layer. We don't allocate to inactive layers. - size_t active_layer = 0; - for (; active_layer < codec_.numberOfSimulcastStreams; ++active_layer) { - if (codec_.simulcastStream[active_layer].active) { - // Found the first active layer. - break; + + // Next, try allocate remaining bitrate, up to max bitrate, in top stream. + // TODO(sprang): Allocate up to max bitrate for all layers once we have a + // better idea of possible performance implications. + if (left_to_allocate > 0) { + size_t active_layer = layer - 1; + const SimulcastStream& stream = codec_.simulcastStream[active_layer]; + uint32_t bitrate_bps = + allocated_bitrates_bps.GetSpatialLayerSum(active_layer); + uint32_t allocation = + std::min(left_to_allocate, stream.maxBitrate * 1000 - bitrate_bps); + bitrate_bps += allocation; + RTC_DCHECK_LE(allocation, left_to_allocate); + left_to_allocate -= allocation; + allocated_bitrates_bps.SetBitrate(active_layer, 0, bitrate_bps); } } - // All streams could be inactive, and nothing more to do. - if (active_layer == codec_.numberOfSimulcastStreams) { - return; - } - // Always allocate enough bitrate for the minimum bitrate of the first - // active layer. Suspending below min bitrate is controlled outside the - // codec implementation and is not overridden by this. - left_to_allocate = std::max( - codec_.simulcastStream[active_layer].minBitrate * 1000, left_to_allocate); - - // Begin by allocating bitrate to simulcast streams, putting all bitrate in - // temporal layer 0. We'll then distribute this bitrate, across potential - // temporal layers, when stream allocation is done. - - size_t top_active_layer = active_layer; - // Allocate up to the target bitrate for each active simulcast layer. - for (; active_layer < codec_.numberOfSimulcastStreams; ++active_layer) { - const SimulcastStream& stream = codec_.simulcastStream[active_layer]; - if (!stream.active) { - continue; - } - // If we can't allocate to the current layer we can't allocate to higher - // layers because they require a higher minimum bitrate. - if (left_to_allocate < stream.minBitrate * 1000) { - break; - } - // We are allocating to this layer so it is the current active allocation. - top_active_layer = active_layer; - uint32_t allocation = - std::min(left_to_allocate, stream.targetBitrate * 1000); - allocated_bitrates_bps->SetBitrate(active_layer, 0, allocation); - RTC_DCHECK_LE(allocation, left_to_allocate); - left_to_allocate -= allocation; - } - - // Next, try allocate remaining bitrate, up to max bitrate, in top active - // stream. - // TODO(sprang): Allocate up to max bitrate for all layers once we have a - // better idea of possible performance implications. - if (left_to_allocate > 0) { - const SimulcastStream& stream = codec_.simulcastStream[top_active_layer]; - uint32_t bitrate_bps = - allocated_bitrates_bps->GetSpatialLayerSum(top_active_layer); - uint32_t allocation = - std::min(left_to_allocate, stream.maxBitrate * 1000 - bitrate_bps); - bitrate_bps += allocation; - RTC_DCHECK_LE(allocation, left_to_allocate); - left_to_allocate -= allocation; - allocated_bitrates_bps->SetBitrate(top_active_layer, 0, bitrate_bps); - } -} - -void SimulcastRateAllocator::DistributeAllocationToTemporalLayers( - uint32_t framerate, - BitrateAllocation* allocated_bitrates_bps) { const int num_spatial_streams = std::max(1, static_cast(codec_.numberOfSimulcastStreams)); @@ -130,21 +94,16 @@ void SimulcastRateAllocator::DistributeAllocationToTemporalLayers( // available temporal layers. for (int simulcast_id = 0; simulcast_id < num_spatial_streams; ++simulcast_id) { - // TODO(shampson): Consider adding a continue here if the simulcast stream - // is inactive. Currently this is not added because the call - // below to OnRatesUpdated changes the TemporalLayer's - // state. auto tl_it = temporal_layers_.find(simulcast_id); if (tl_it == temporal_layers_.end()) continue; // TODO(sprang): If > 1 SS, assume default TL alloc? uint32_t target_bitrate_kbps = - allocated_bitrates_bps->GetBitrate(simulcast_id, 0) / 1000; - + allocated_bitrates_bps.GetBitrate(simulcast_id, 0) / 1000; const uint32_t expected_allocated_bitrate_kbps = target_bitrate_kbps; RTC_DCHECK_EQ( target_bitrate_kbps, - allocated_bitrates_bps->GetSpatialLayerSum(simulcast_id) / 1000); + allocated_bitrates_bps.GetSpatialLayerSum(simulcast_id) / 1000); const int num_temporal_streams = std::max( 1, codec_.numberOfSimulcastStreams == 0 ? codec_.VP8().numberOfTemporalLayers @@ -178,14 +137,14 @@ void SimulcastRateAllocator::DistributeAllocationToTemporalLayers( uint64_t tl_allocation_sum_kbps = 0; for (size_t tl_index = 0; tl_index < tl_allocation.size(); ++tl_index) { uint32_t layer_rate_kbps = tl_allocation[tl_index]; - if (layer_rate_kbps > 0) { - allocated_bitrates_bps->SetBitrate(simulcast_id, tl_index, - layer_rate_kbps * 1000); - } + allocated_bitrates_bps.SetBitrate(simulcast_id, tl_index, + layer_rate_kbps * 1000); tl_allocation_sum_kbps += layer_rate_kbps; } RTC_DCHECK_LE(tl_allocation_sum_kbps, expected_allocated_bitrate_kbps); } + + return allocated_bitrates_bps; } uint32_t SimulcastRateAllocator::GetPreferredBitrateBps(uint32_t framerate) { diff --git a/modules/video_coding/codecs/vp8/simulcast_rate_allocator.h b/modules/video_coding/codecs/vp8/simulcast_rate_allocator.h index b4b6f315e0..929abba413 100644 --- a/modules/video_coding/codecs/vp8/simulcast_rate_allocator.h +++ b/modules/video_coding/codecs/vp8/simulcast_rate_allocator.h @@ -39,12 +39,6 @@ class SimulcastRateAllocator : public VideoBitrateAllocator, const VideoCodec& GetCodec() const; private: - void DistributeAllocationToSimulcastLayers( - uint32_t total_bitrate_bps, - BitrateAllocation* allocated_bitrates_bps); - void DistributeAllocationToTemporalLayers( - uint32_t framerate, - BitrateAllocation* allocated_bitrates_bps); const VideoCodec codec_; std::map temporal_layers_; std::unique_ptr tl_factory_; diff --git a/modules/video_coding/codecs/vp8/simulcast_test_utility.h b/modules/video_coding/codecs/vp8/simulcast_test_utility.h index e13bad4c85..7f32ad2f8e 100644 --- a/modules/video_coding/codecs/vp8/simulcast_test_utility.h +++ b/modules/video_coding/codecs/vp8/simulcast_test_utility.h @@ -203,7 +203,6 @@ class TestVp8Simulcast : public ::testing::Test { settings->width = kDefaultWidth; settings->height = kDefaultHeight; settings->numberOfSimulcastStreams = kNumberOfSimulcastStreams; - settings->active = true; ASSERT_EQ(3, kNumberOfSimulcastStreams); settings->timing_frame_thresholds = {kDefaultTimingFramesDelayMs, kDefaultOutlierFrameSizePercent}; @@ -239,7 +238,6 @@ class TestVp8Simulcast : public ::testing::Test { stream->targetBitrate = target_bitrate; stream->numberOfTemporalLayers = num_temporal_layers; stream->qpMax = 45; - stream->active = true; } protected: @@ -284,22 +282,10 @@ class TestVp8Simulcast : public ::testing::Test { rate_allocator_->GetAllocation(bitrate_kbps * 1000, fps), fps); } - void UpdateActiveStreams(const std::vector active_streams) { - ASSERT_EQ(static_cast(active_streams.size()), - kNumberOfSimulcastStreams); - for (size_t i = 0; i < active_streams.size(); ++i) { - settings_.simulcastStream[i].active = active_streams[i]; - } - // Re initialize the allocator and encoder with the new settings. - SetUpRateAllocator(); - EXPECT_EQ(0, encoder_->InitEncode(&settings_, 1, 1200)); - } - - void ExpectStreams(FrameType frame_type, - const std::vector expected_streams_active) { - ASSERT_EQ(static_cast(expected_streams_active.size()), - kNumberOfSimulcastStreams); - if (expected_streams_active[0]) { + void ExpectStreams(FrameType frame_type, int expected_video_streams) { + ASSERT_GE(expected_video_streams, 0); + ASSERT_LE(expected_video_streams, kNumberOfSimulcastStreams); + if (expected_video_streams >= 1) { EXPECT_CALL( encoder_callback_, OnEncodedImage( @@ -311,7 +297,7 @@ class TestVp8Simulcast : public ::testing::Test { .WillRepeatedly(Return(EncodedImageCallback::Result( EncodedImageCallback::Result::OK, 0))); } - if (expected_streams_active[1]) { + if (expected_video_streams >= 2) { EXPECT_CALL( encoder_callback_, OnEncodedImage( @@ -323,7 +309,7 @@ class TestVp8Simulcast : public ::testing::Test { .WillRepeatedly(Return(EncodedImageCallback::Result( EncodedImageCallback::Result::OK, 0))); } - if (expected_streams_active[2]) { + if (expected_video_streams >= 3) { EXPECT_CALL( encoder_callback_, OnEncodedImage( @@ -337,16 +323,6 @@ class TestVp8Simulcast : public ::testing::Test { } } - void ExpectStreams(FrameType frame_type, int expected_video_streams) { - ASSERT_GE(expected_video_streams, 0); - ASSERT_LE(expected_video_streams, kNumberOfSimulcastStreams); - std::vector expected_streams_active(kNumberOfSimulcastStreams, false); - for (int i = 0; i < expected_video_streams; ++i) { - expected_streams_active[i] = true; - } - ExpectStreams(frame_type, expected_streams_active); - } - void VerifyTemporalIdxAndSyncForAllSpatialLayers( Vp8TestEncodedImageCallback* encoder_callback, const int* expected_temporal_idx, @@ -525,43 +501,6 @@ class TestVp8Simulcast : public ::testing::Test { EXPECT_EQ(0, encoder_->Encode(*input_frame_, NULL, &frame_types)); } - void TestActiveStreams() { - const int kEnoughBitrateAllStreams = - kMaxBitrates[0] + kMaxBitrates[1] + kMaxBitrates[2]; - std::vector frame_types(kNumberOfSimulcastStreams, - kVideoFrameDelta); - // TODO(shampson): Currently turning off the base stream causes unexpected - // behavior in the libvpx encoder. The libvpx encoder labels key frames - // based upon the base stream. If the base stream is never enabled, it - // will continue to spit out encoded images labeled as key frames for the - // other streams that are enabled. Once this is fixed in libvpx, update this - // test to reflect that change. - - // Only turn on the the base stream. - std::vector active_streams = {true, false, false}; - UpdateActiveStreams(active_streams); - SetRates(kEnoughBitrateAllStreams, 30); - ExpectStreams(kVideoFrameKey, active_streams); - input_frame_->set_timestamp(input_frame_->timestamp() + 3000); - EXPECT_EQ(0, encoder_->Encode(*input_frame_, NULL, &frame_types)); - - ExpectStreams(kVideoFrameDelta, active_streams); - input_frame_->set_timestamp(input_frame_->timestamp() + 3000); - EXPECT_EQ(0, encoder_->Encode(*input_frame_, NULL, &frame_types)); - - // Turn off only the middle stream. - active_streams = {true, false, true}; - UpdateActiveStreams(active_streams); - SetRates(kEnoughBitrateAllStreams, 30); - ExpectStreams(kVideoFrameKey, active_streams); - input_frame_->set_timestamp(input_frame_->timestamp() + 3000); - EXPECT_EQ(0, encoder_->Encode(*input_frame_, NULL, &frame_types)); - - ExpectStreams(kVideoFrameDelta, active_streams); - input_frame_->set_timestamp(input_frame_->timestamp() + 3000); - EXPECT_EQ(0, encoder_->Encode(*input_frame_, NULL, &frame_types)); - } - void SwitchingToOneStream(int width, int height) { // Disable all streams except the last and set the bitrate of the last to // 100 kbps. This verifies the way GTP switches to screenshare mode. diff --git a/modules/video_coding/codecs/vp8/simulcast_unittest.cc b/modules/video_coding/codecs/vp8/simulcast_unittest.cc index f72d880149..b1dd794f29 100644 --- a/modules/video_coding/codecs/vp8/simulcast_unittest.cc +++ b/modules/video_coding/codecs/vp8/simulcast_unittest.cc @@ -55,10 +55,6 @@ TEST_F(TestVp8Impl, TestDisablingStreams) { TestVp8Simulcast::TestDisablingStreams(); } -TEST_F(TestVp8Impl, TestActiveStreams) { - TestVp8Simulcast::TestActiveStreams(); -} - TEST_F(TestVp8Impl, TestSwitchingToOneStream) { TestVp8Simulcast::TestSwitchingToOneStream(); } diff --git a/modules/video_coding/utility/default_video_bitrate_allocator.cc b/modules/video_coding/utility/default_video_bitrate_allocator.cc index 0c4ae9bae5..5e3b3a2c5d 100644 --- a/modules/video_coding/utility/default_video_bitrate_allocator.cc +++ b/modules/video_coding/utility/default_video_bitrate_allocator.cc @@ -24,7 +24,7 @@ BitrateAllocation DefaultVideoBitrateAllocator::GetAllocation( uint32_t total_bitrate_bps, uint32_t framerate) { BitrateAllocation allocation; - if (total_bitrate_bps == 0 || !codec_.active) + if (total_bitrate_bps == 0) return allocation; if (total_bitrate_bps < codec_.minBitrate * 1000) { diff --git a/modules/video_coding/utility/default_video_bitrate_allocator_unittest.cc b/modules/video_coding/utility/default_video_bitrate_allocator_unittest.cc index e27b3d30a0..695ac689e8 100644 --- a/modules/video_coding/utility/default_video_bitrate_allocator_unittest.cc +++ b/modules/video_coding/utility/default_video_bitrate_allocator_unittest.cc @@ -45,13 +45,6 @@ TEST_F(DefaultVideoBitrateAllocatorTest, ZeroIsOff) { EXPECT_EQ(0u, allocation.get_sum_bps()); } -TEST_F(DefaultVideoBitrateAllocatorTest, Inactive) { - codec_.active = false; - allocator_.reset(new DefaultVideoBitrateAllocator(codec_)); - BitrateAllocation allocation = allocator_->GetAllocation(1, kMaxFramerate); - EXPECT_EQ(0u, allocation.get_sum_bps()); -} - TEST_F(DefaultVideoBitrateAllocatorTest, CapsToMin) { BitrateAllocation allocation = allocator_->GetAllocation(1, kMaxFramerate); EXPECT_EQ(kMinBitrateBps, allocation.get_sum_bps()); diff --git a/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc b/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc index 44d0081bcd..15ac938ae5 100644 --- a/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc +++ b/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc @@ -51,7 +51,6 @@ class SimulcastRateAllocatorTest : public ::testing::TestWithParam { codec_.minBitrate = kMinBitrateKbps; codec_.targetBitrate = kTargetBitrateKbps; codec_.maxBitrate = kMaxBitrateKbps; - codec_.active = true; CreateAllocator(); } virtual ~SimulcastRateAllocatorTest() {} @@ -70,9 +69,6 @@ class SimulcastRateAllocatorTest : public ::testing::TestWithParam { uint32_t sum = 0; for (size_t i = 0; i < S; ++i) { uint32_t layer_bitrate = actual.GetSpatialLayerSum(i); - if (layer_bitrate == 0) { - EXPECT_FALSE(actual.IsSpatialLayerUsed(i)); - } EXPECT_EQ(expected[i] * 1000U, layer_bitrate) << "Mismatch at index " << i; sum += layer_bitrate; @@ -100,34 +96,6 @@ class SimulcastRateAllocatorTest : public ::testing::TestWithParam { } } - void SetupCodecThreeSimulcastStreams( - const std::vector& active_streams) { - size_t num_streams = 3; - RTC_DCHECK_GE(active_streams.size(), num_streams); - SetupCodecTwoSimulcastStreams(active_streams); - codec_.numberOfSimulcastStreams = num_streams; - codec_.simulcastStream[2].minBitrate = 2000; - codec_.simulcastStream[2].targetBitrate = 3000; - codec_.simulcastStream[2].maxBitrate = 4000; - codec_.simulcastStream[2].active = active_streams[2]; - } - - void SetupCodecTwoSimulcastStreams(const std::vector& active_streams) { - size_t num_streams = 2; - RTC_DCHECK_GE(active_streams.size(), num_streams); - codec_.numberOfSimulcastStreams = num_streams; - codec_.maxBitrate = 0; - codec_.simulcastStream[0].minBitrate = 10; - codec_.simulcastStream[0].targetBitrate = 100; - codec_.simulcastStream[0].maxBitrate = 500; - codec_.simulcastStream[1].minBitrate = 50; - codec_.simulcastStream[1].targetBitrate = 500; - codec_.simulcastStream[1].maxBitrate = 1000; - for (size_t i = 0; i < num_streams; ++i) { - codec_.simulcastStream[i].active = active_streams[i]; - } - } - virtual std::unique_ptr GetTlFactory() { return std::unique_ptr(new TemporalLayersFactory()); } @@ -145,7 +113,6 @@ class SimulcastRateAllocatorTest : public ::testing::TestWithParam { TEST_F(SimulcastRateAllocatorTest, NoSimulcastBelowMin) { uint32_t expected[] = {codec_.minBitrate}; - codec_.active = true; ExpectEqual(expected, GetAllocation(codec_.minBitrate - 1)); ExpectEqual(expected, GetAllocation(1)); ExpectEqual(expected, GetAllocation(0)); @@ -153,14 +120,12 @@ TEST_F(SimulcastRateAllocatorTest, NoSimulcastBelowMin) { TEST_F(SimulcastRateAllocatorTest, NoSimulcastAboveMax) { uint32_t expected[] = {codec_.maxBitrate}; - codec_.active = true; ExpectEqual(expected, GetAllocation(codec_.maxBitrate + 1)); ExpectEqual(expected, GetAllocation(std::numeric_limits::max())); } TEST_F(SimulcastRateAllocatorTest, NoSimulcastNoMax) { const uint32_t kMax = BitrateAllocation::kMaxBitrateBps / 1000; - codec_.active = true; codec_.maxBitrate = 0; CreateAllocator(); @@ -169,7 +134,6 @@ TEST_F(SimulcastRateAllocatorTest, NoSimulcastNoMax) { } TEST_F(SimulcastRateAllocatorTest, NoSimulcastWithinLimits) { - codec_.active = true; for (uint32_t bitrate = codec_.minBitrate; bitrate <= codec_.maxBitrate; ++bitrate) { uint32_t expected[] = {bitrate}; @@ -177,25 +141,12 @@ TEST_F(SimulcastRateAllocatorTest, NoSimulcastWithinLimits) { } } -// Tests that when we aren't using simulcast and the codec is marked inactive no -// bitrate will be allocated. -TEST_F(SimulcastRateAllocatorTest, NoSimulcastInactive) { - codec_.active = false; - uint32_t expected[] = {0}; - CreateAllocator(); - - ExpectEqual(expected, GetAllocation(kMinBitrateKbps - 10)); - ExpectEqual(expected, GetAllocation(kTargetBitrateKbps)); - ExpectEqual(expected, GetAllocation(kMaxBitrateKbps + 10)); -} - TEST_F(SimulcastRateAllocatorTest, SingleSimulcastBelowMin) { // With simulcast, use the min bitrate from the ss spec instead of the global. codec_.numberOfSimulcastStreams = 1; const uint32_t kMin = codec_.minBitrate - 10; codec_.simulcastStream[0].minBitrate = kMin; codec_.simulcastStream[0].targetBitrate = kTargetBitrateKbps; - codec_.simulcastStream[0].active = true; CreateAllocator(); uint32_t expected[] = {kMin}; @@ -209,7 +160,6 @@ TEST_F(SimulcastRateAllocatorTest, SingleSimulcastAboveMax) { codec_.simulcastStream[0].minBitrate = kMinBitrateKbps; const uint32_t kMax = codec_.simulcastStream[0].maxBitrate + 1000; codec_.simulcastStream[0].maxBitrate = kMax; - codec_.simulcastStream[0].active = true; CreateAllocator(); uint32_t expected[] = {kMax}; @@ -223,7 +173,6 @@ TEST_F(SimulcastRateAllocatorTest, SingleSimulcastWithinLimits) { codec_.simulcastStream[0].minBitrate = kMinBitrateKbps; codec_.simulcastStream[0].targetBitrate = kTargetBitrateKbps; codec_.simulcastStream[0].maxBitrate = kMaxBitrateKbps; - codec_.simulcastStream[0].active = true; CreateAllocator(); for (uint32_t bitrate = kMinBitrateKbps; bitrate <= kMaxBitrateKbps; @@ -233,23 +182,18 @@ TEST_F(SimulcastRateAllocatorTest, SingleSimulcastWithinLimits) { } } -TEST_F(SimulcastRateAllocatorTest, SingleSimulcastInactive) { - codec_.numberOfSimulcastStreams = 1; - codec_.simulcastStream[0].minBitrate = kMinBitrateKbps; - codec_.simulcastStream[0].targetBitrate = kTargetBitrateKbps; - codec_.simulcastStream[0].maxBitrate = kMaxBitrateKbps; - codec_.simulcastStream[0].active = false; - CreateAllocator(); - - uint32_t expected[] = {0}; - ExpectEqual(expected, GetAllocation(kMinBitrateKbps - 10)); - ExpectEqual(expected, GetAllocation(kTargetBitrateKbps)); - ExpectEqual(expected, GetAllocation(kMaxBitrateKbps + 10)); -} - TEST_F(SimulcastRateAllocatorTest, OneToThreeStreams) { - const std::vector active_streams(3, true); - SetupCodecThreeSimulcastStreams(active_streams); + codec_.numberOfSimulcastStreams = 3; + codec_.maxBitrate = 0; + codec_.simulcastStream[0].minBitrate = 10; + codec_.simulcastStream[0].targetBitrate = 100; + codec_.simulcastStream[0].maxBitrate = 500; + codec_.simulcastStream[1].minBitrate = 50; + codec_.simulcastStream[1].targetBitrate = 500; + codec_.simulcastStream[1].maxBitrate = 1000; + codec_.simulcastStream[2].minBitrate = 2000; + codec_.simulcastStream[2].targetBitrate = 3000; + codec_.simulcastStream[2].maxBitrate = 4000; CreateAllocator(); { @@ -323,164 +267,6 @@ TEST_F(SimulcastRateAllocatorTest, OneToThreeStreams) { codec_.simulcastStream[2].maxBitrate}; ExpectEqual(expected, GetAllocation(bitrate)); } - - { - // Enough to max out all streams which will allocate the target amount to - // the lower streams. - const uint32_t bitrate = codec_.simulcastStream[0].maxBitrate + - codec_.simulcastStream[1].maxBitrate + - codec_.simulcastStream[2].maxBitrate; - uint32_t expected[] = {codec_.simulcastStream[0].targetBitrate, - codec_.simulcastStream[1].targetBitrate, - codec_.simulcastStream[2].maxBitrate}; - ExpectEqual(expected, GetAllocation(bitrate)); - } -} - -// If three simulcast streams that are all inactive, none of them should be -// allocated bitrate. -TEST_F(SimulcastRateAllocatorTest, ThreeStreamsInactive) { - const std::vector active_streams(3, false); - SetupCodecThreeSimulcastStreams(active_streams); - CreateAllocator(); - - // Just enough to allocate the min. - const uint32_t min_bitrate = codec_.simulcastStream[0].minBitrate + - codec_.simulcastStream[1].minBitrate + - codec_.simulcastStream[2].minBitrate; - // Enough bitrate to allocate target to all streams. - const uint32_t target_bitrate = codec_.simulcastStream[0].targetBitrate + - codec_.simulcastStream[1].targetBitrate + - codec_.simulcastStream[2].targetBitrate; - // Enough bitrate to allocate max to all streams. - const uint32_t max_bitrate = codec_.simulcastStream[0].maxBitrate + - codec_.simulcastStream[1].maxBitrate + - codec_.simulcastStream[2].maxBitrate; - uint32_t expected[] = {0, 0, 0}; - ExpectEqual(expected, GetAllocation(0)); - ExpectEqual(expected, GetAllocation(min_bitrate)); - ExpectEqual(expected, GetAllocation(target_bitrate)); - ExpectEqual(expected, GetAllocation(max_bitrate)); -} - -// If there are two simulcast streams, we expect the high active stream to be -// allocated as if it is a single active stream. -TEST_F(SimulcastRateAllocatorTest, TwoStreamsLowInactive) { - const std::vector active_streams({false, true}); - SetupCodecTwoSimulcastStreams(active_streams); - CreateAllocator(); - - const uint32_t kActiveStreamMinBitrate = codec_.simulcastStream[1].minBitrate; - const uint32_t kActiveStreamTargetBitrate = - codec_.simulcastStream[1].targetBitrate; - const uint32_t kActiveStreamMaxBitrate = codec_.simulcastStream[1].maxBitrate; - { - // Expect that the stream is always allocated its min bitrate. - uint32_t expected[] = {0, kActiveStreamMinBitrate}; - ExpectEqual(expected, GetAllocation(0)); - ExpectEqual(expected, GetAllocation(kActiveStreamMinBitrate - 10)); - ExpectEqual(expected, GetAllocation(kActiveStreamMinBitrate)); - } - - { - // The stream should be allocated its target bitrate. - uint32_t expected[] = {0, kActiveStreamTargetBitrate}; - ExpectEqual(expected, GetAllocation(kActiveStreamTargetBitrate)); - } - - { - // The stream should be allocated its max if the target input is sufficient. - uint32_t expected[] = {0, kActiveStreamMaxBitrate}; - ExpectEqual(expected, GetAllocation(kActiveStreamMaxBitrate)); - ExpectEqual(expected, GetAllocation(std::numeric_limits::max())); - } -} - -// If there are two simulcast streams, we expect the low active stream to be -// allocated as if it is a single active stream. -TEST_F(SimulcastRateAllocatorTest, TwoStreamsHighInactive) { - const std::vector active_streams({true, false}); - SetupCodecTwoSimulcastStreams(active_streams); - CreateAllocator(); - - const uint32_t kActiveStreamMinBitrate = codec_.simulcastStream[0].minBitrate; - const uint32_t kActiveStreamTargetBitrate = - codec_.simulcastStream[0].targetBitrate; - const uint32_t kActiveStreamMaxBitrate = codec_.simulcastStream[0].maxBitrate; - { - // Expect that the stream is always allocated its min bitrate. - uint32_t expected[] = {kActiveStreamMinBitrate, 0}; - ExpectEqual(expected, GetAllocation(0)); - ExpectEqual(expected, GetAllocation(kActiveStreamMinBitrate - 10)); - ExpectEqual(expected, GetAllocation(kActiveStreamMinBitrate)); - } - - { - // The stream should be allocated its target bitrate. - uint32_t expected[] = {kActiveStreamTargetBitrate, 0}; - ExpectEqual(expected, GetAllocation(kActiveStreamTargetBitrate)); - } - - { - // The stream should be allocated its max if the target input is sufficent. - uint32_t expected[] = {kActiveStreamMaxBitrate, 0}; - ExpectEqual(expected, GetAllocation(kActiveStreamMaxBitrate)); - ExpectEqual(expected, GetAllocation(std::numeric_limits::max())); - } -} - -// If there are three simulcast streams and the middle stream is inactive, the -// other two streams should be allocated bitrate the same as if they are two -// active simulcast streams. -TEST_F(SimulcastRateAllocatorTest, ThreeStreamsMiddleInactive) { - const std::vector active_streams({true, false, true}); - SetupCodecThreeSimulcastStreams(active_streams); - CreateAllocator(); - - { - const uint32_t kLowStreamMinBitrate = codec_.simulcastStream[0].minBitrate; - // The lowest stream should always be allocated its minimum bitrate. - uint32_t expected[] = {kLowStreamMinBitrate, 0, 0}; - ExpectEqual(expected, GetAllocation(0)); - ExpectEqual(expected, GetAllocation(kLowStreamMinBitrate - 10)); - ExpectEqual(expected, GetAllocation(kLowStreamMinBitrate)); - } - - { - // The lowest stream gets its target bitrate. - uint32_t expected[] = {codec_.simulcastStream[0].targetBitrate, 0, 0}; - ExpectEqual(expected, - GetAllocation(codec_.simulcastStream[0].targetBitrate)); - } - - { - // The lowest stream gets its max bitrate, but not enough for the high - // stream. - const uint32_t bitrate = codec_.simulcastStream[0].targetBitrate + - codec_.simulcastStream[2].minBitrate - 1; - uint32_t expected[] = {codec_.simulcastStream[0].maxBitrate, 0, 0}; - ExpectEqual(expected, GetAllocation(bitrate)); - } - - { - // Both active streams get allocated target bitrate. - const uint32_t bitrate = codec_.simulcastStream[0].targetBitrate + - codec_.simulcastStream[2].targetBitrate; - uint32_t expected[] = {codec_.simulcastStream[0].targetBitrate, 0, - codec_.simulcastStream[2].targetBitrate}; - ExpectEqual(expected, GetAllocation(bitrate)); - } - - { - // Lowest stream gets its target bitrate, high stream gets its max bitrate. - uint32_t bitrate = codec_.simulcastStream[0].targetBitrate + - codec_.simulcastStream[2].maxBitrate; - uint32_t expected[] = {codec_.simulcastStream[0].targetBitrate, 0, - codec_.simulcastStream[2].maxBitrate}; - ExpectEqual(expected, GetAllocation(bitrate)); - ExpectEqual(expected, GetAllocation(bitrate + 10)); - ExpectEqual(expected, GetAllocation(std::numeric_limits::max())); - } } TEST_F(SimulcastRateAllocatorTest, GetPreferredBitrateBps) { @@ -497,18 +283,15 @@ TEST_F(SimulcastRateAllocatorTest, GetPreferredBitrateSimulcast) { codec_.maxBitrate = 999999; codec_.simulcastStream[0].minBitrate = 10; codec_.simulcastStream[0].targetBitrate = 100; - codec_.simulcastStream[0].active = true; codec_.simulcastStream[0].maxBitrate = 500; codec_.simulcastStream[1].minBitrate = 50; codec_.simulcastStream[1].targetBitrate = 500; codec_.simulcastStream[1].maxBitrate = 1000; - codec_.simulcastStream[1].active = true; codec_.simulcastStream[2].minBitrate = 2000; codec_.simulcastStream[2].targetBitrate = 3000; codec_.simulcastStream[2].maxBitrate = 4000; - codec_.simulcastStream[2].active = true; CreateAllocator(); uint32_t preferred_bitrate_kbps; @@ -522,7 +305,7 @@ TEST_F(SimulcastRateAllocatorTest, GetPreferredBitrateSimulcast) { class ScreenshareRateAllocationTest : public SimulcastRateAllocatorTest { public: - void SetupConferenceScreenshare(bool use_simulcast, bool active = true) { + void SetupConferenceScreenshare(bool use_simulcast) { codec_.mode = VideoCodecMode::kScreensharing; codec_.minBitrate = kMinBitrateKbps; codec_.maxBitrate = kMaxBitrateKbps; @@ -532,12 +315,10 @@ class ScreenshareRateAllocationTest : public SimulcastRateAllocatorTest { codec_.simulcastStream[0].targetBitrate = kTargetBitrateKbps; codec_.simulcastStream[0].maxBitrate = kMaxBitrateKbps; codec_.simulcastStream[0].numberOfTemporalLayers = 2; - codec_.simulcastStream[0].active = active; } else { codec_.numberOfSimulcastStreams = 0; codec_.targetBitrate = kTargetBitrateKbps; codec_.VP8()->numberOfTemporalLayers = 2; - codec_.active = active; } } @@ -592,17 +373,4 @@ TEST_P(ScreenshareRateAllocationTest, BitrateAboveTl1) { allocation.GetBitrate(0, 1) / 1000); } -// This tests when the screenshare is inactive it should be allocated 0 bitrate -// for all layers. -TEST_P(ScreenshareRateAllocationTest, InactiveScreenshare) { - SetupConferenceScreenshare(GetParam(), false); - CreateAllocator(); - - // Enough bitrate for TL0 and TL1. - uint32_t target_bitrate_kbps = (kTargetBitrateKbps + kMaxBitrateKbps) / 2; - BitrateAllocation allocation = - allocator_->GetAllocation(target_bitrate_kbps * 1000, kFramerateFps); - - EXPECT_EQ(0U, allocation.get_sum_kbps()); -} } // namespace webrtc diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc index 24c9e7cd17..9b259f6fdd 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc @@ -192,15 +192,6 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( video_codec.numberOfSimulcastStreams = static_cast(streams.size()); video_codec.minBitrate = streams[0].min_bitrate_bps / 1000; - bool codec_active = false; - for (const VideoStream& stream : streams) { - if (stream.active) { - codec_active = true; - break; - } - } - // Set active for the entire video codec for the non simulcast case. - video_codec.active = codec_active; if (video_codec.minBitrate < kEncoderMinBitrateKbps) video_codec.minBitrate = kEncoderMinBitrateKbps; video_codec.timing_frame_thresholds = {kDefaultTimingFramesDelayMs, @@ -240,7 +231,6 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( sim_stream->qpMax = streams[i].max_qp; sim_stream->numberOfTemporalLayers = static_cast( streams[i].temporal_layer_thresholds_bps.size() + 1); - sim_stream->active = streams[i].active; video_codec.width = std::max(video_codec.width, static_cast(streams[i].width)); diff --git a/modules/video_coding/video_codec_initializer_unittest.cc b/modules/video_coding/video_codec_initializer_unittest.cc index e8925e1854..0862b7282e 100644 --- a/modules/video_coding/video_codec_initializer_unittest.cc +++ b/modules/video_coding/video_codec_initializer_unittest.cc @@ -118,7 +118,6 @@ class VideoCodecInitializerTest : public ::testing::Test { stream.target_bitrate_bps = kDefaultTargetBitrateBps; stream.max_bitrate_bps = kDefaultMaxBitrateBps; stream.max_qp = kDefaultMaxQp; - stream.active = true; return stream; } @@ -129,7 +128,6 @@ class VideoCodecInitializerTest : public ::testing::Test { stream.max_bitrate_bps = 1000000; stream.max_framerate = kScreenshareDefaultFramerate; stream.temporal_layer_thresholds_bps.push_back(kScreenshareTl0BitrateBps); - stream.active = true; return stream; } @@ -157,20 +155,6 @@ TEST_F(VideoCodecInitializerTest, SingleStreamVp8Screenshare) { EXPECT_EQ(kDefaultTargetBitrateBps, bitrate_allocation.get_sum_bps()); } -TEST_F(VideoCodecInitializerTest, SingleStreamVp8ScreenshareInactive) { - SetUpFor(VideoCodecType::kVideoCodecVP8, 1, 1, true); - VideoStream inactive_stream = DefaultStream(); - inactive_stream.active = false; - streams_.push_back(inactive_stream); - EXPECT_TRUE(InitializeCodec()); - - BitrateAllocation bitrate_allocation = bitrate_allocator_out_->GetAllocation( - kDefaultTargetBitrateBps, kDefaultFrameRate); - EXPECT_EQ(1u, codec_out_.numberOfSimulcastStreams); - EXPECT_EQ(1u, codec_out_.VP8()->numberOfTemporalLayers); - EXPECT_EQ(0U, bitrate_allocation.get_sum_bps()); -} - TEST_F(VideoCodecInitializerTest, TemporalLayeredVp8Screenshare) { SetUpFor(VideoCodecType::kVideoCodecVP8, 1, 2, true); streams_.push_back(DefaultScreenshareStream()); @@ -185,7 +169,7 @@ TEST_F(VideoCodecInitializerTest, TemporalLayeredVp8Screenshare) { EXPECT_EQ(kScreenshareTl0BitrateBps, bitrate_allocation.GetBitrate(0, 0)); } -TEST_F(VideoCodecInitializerTest, SimulcastVp8Screenshare) { +TEST_F(VideoCodecInitializerTest, SimlucastVp8Screenshare) { SetUpFor(VideoCodecType::kVideoCodecVP8, 2, 1, true); streams_.push_back(DefaultScreenshareStream()); VideoStream video_stream = DefaultStream(); @@ -206,31 +190,7 @@ TEST_F(VideoCodecInitializerTest, SimulcastVp8Screenshare) { bitrate_allocation.GetSpatialLayerSum(1)); } -// 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); - streams_.push_back(DefaultScreenshareStream()); - VideoStream inactive_video_stream = DefaultStream(); - inactive_video_stream.active = false; - inactive_video_stream.max_framerate = kScreenshareDefaultFramerate; - streams_.push_back(inactive_video_stream); - EXPECT_TRUE(InitializeCodec()); - - EXPECT_EQ(2u, codec_out_.numberOfSimulcastStreams); - EXPECT_EQ(1u, codec_out_.VP8()->numberOfTemporalLayers); - const uint32_t target_bitrate = - streams_[0].target_bitrate_bps + streams_[1].target_bitrate_bps; - BitrateAllocation bitrate_allocation = bitrate_allocator_out_->GetAllocation( - target_bitrate, kScreenshareDefaultFramerate); - EXPECT_EQ(static_cast(streams_[0].max_bitrate_bps), - bitrate_allocation.get_sum_bps()); - EXPECT_EQ(static_cast(streams_[0].max_bitrate_bps), - bitrate_allocation.GetSpatialLayerSum(0)); - EXPECT_EQ(0U, bitrate_allocation.GetSpatialLayerSum(1)); -} - -TEST_F(VideoCodecInitializerTest, HighFpsSimulcastVp8Screenshare) { +TEST_F(VideoCodecInitializerTest, HighFpsSimlucastVp8Screenshare) { // 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); diff --git a/test/encoder_settings.cc b/test/encoder_settings.cc index 205af7bdac..84d391669e 100644 --- a/test/encoder_settings.cc +++ b/test/encoder_settings.cc @@ -50,7 +50,6 @@ std::vector CreateVideoStreams( std::min(bitrate_left_bps, DefaultVideoStreamFactory::kMaxBitratePerStream[i]); stream_settings[i].max_qp = 56; - stream_settings[i].active = true; bitrate_left_bps -= stream_settings[i].target_bitrate_bps; } diff --git a/test/video_codec_settings.h b/test/video_codec_settings.h index 5dabc70265..54cf717a77 100644 --- a/test/video_codec_settings.h +++ b/test/video_codec_settings.h @@ -43,7 +43,6 @@ static void CodecSettings(VideoCodecType codec_type, VideoCodec* settings) { settings->timing_frame_thresholds = { kTestTimingFramesDelayMs, kTestOutlierFrameSizePercent, }; - settings->active = true; *(settings->VP8()) = VideoEncoder::GetDefaultVp8Settings(); return; case kVideoCodecVP9: @@ -62,7 +61,6 @@ static void CodecSettings(VideoCodecType codec_type, VideoCodec* settings) { settings->timing_frame_thresholds = { kTestTimingFramesDelayMs, kTestOutlierFrameSizePercent, }; - settings->active = true; *(settings->VP9()) = VideoEncoder::GetDefaultVp9Settings(); return; case kVideoCodecH264: @@ -81,7 +79,6 @@ static void CodecSettings(VideoCodecType codec_type, VideoCodec* settings) { settings->timing_frame_thresholds = { kTestTimingFramesDelayMs, kTestOutlierFrameSizePercent, }; - settings->active = true; *(settings->H264()) = VideoEncoder::GetDefaultH264Settings(); return; case kVideoCodecI420: @@ -98,7 +95,6 @@ static void CodecSettings(VideoCodecType codec_type, VideoCodec* settings) { settings->height = kTestHeight; settings->minBitrate = kTestMinBitrateKbps; settings->numberOfSimulcastStreams = 0; - settings->active = true; return; case kVideoCodecRED: case kVideoCodecULPFEC: diff --git a/video/payload_router.cc b/video/payload_router.cc index 68d4ee89ce..f22183287d 100644 --- a/video/payload_router.cc +++ b/video/payload_router.cc @@ -258,11 +258,8 @@ void PayloadRouter::OnBitrateAllocationUpdated( // rtp stream, moving over the temporal layer allocation. for (size_t si = 0; si < rtp_modules_.size(); ++si) { // Don't send empty TargetBitrate messages on streams not being relayed. - if (!bitrate.IsSpatialLayerUsed(si)) { - // The next spatial layer could be used if the current one is - // inactive. - continue; - } + if (!bitrate.IsSpatialLayerUsed(si)) + break; BitrateAllocation layer_bitrate; for (int tl = 0; tl < kMaxTemporalStreams; ++tl) { diff --git a/video/payload_router_unittest.cc b/video/payload_router_unittest.cc index a512582670..d670124ddf 100644 --- a/video/payload_router_unittest.cc +++ b/video/payload_router_unittest.cc @@ -32,7 +32,6 @@ namespace { const int8_t kPayloadType = 96; const uint32_t kSsrc1 = 12345; const uint32_t kSsrc2 = 23456; -const uint32_t kSsrc3 = 34567; const int16_t kPictureId = 123; const int16_t kTl0PicIdx = 20; const uint8_t kTemporalIdx = 1; @@ -187,38 +186,23 @@ TEST(PayloadRouterTest, SimulcastTargetBitrate) { payload_router.OnBitrateAllocationUpdated(bitrate); } -// If the middle of three streams is inactive the first and last streams should -// be asked to send the TargetBitrate message. TEST(PayloadRouterTest, SimulcastTargetBitrateWithInactiveStream) { - // Set up three active rtp modules. + // Set up two active rtp modules. NiceMock rtp_1; NiceMock rtp_2; - NiceMock rtp_3; - std::vector modules = {&rtp_1, &rtp_2, &rtp_3}; - PayloadRouter payload_router(modules, {kSsrc1, kSsrc2, kSsrc3}, kPayloadType, - {}); + std::vector modules = {&rtp_1, &rtp_2}; + PayloadRouter payload_router(modules, {kSsrc1, kSsrc2}, kPayloadType, {}); payload_router.SetActive(true); - // Create bitrate allocation with bitrate only for the first and third stream. + // Create bitrate allocation with bitrate only for the first stream. BitrateAllocation bitrate; bitrate.SetBitrate(0, 0, 10000); bitrate.SetBitrate(0, 1, 20000); - bitrate.SetBitrate(2, 0, 40000); - bitrate.SetBitrate(2, 1, 80000); - BitrateAllocation layer0_bitrate; - layer0_bitrate.SetBitrate(0, 0, 10000); - layer0_bitrate.SetBitrate(0, 1, 20000); - - BitrateAllocation layer2_bitrate; - layer2_bitrate.SetBitrate(0, 0, 40000); - layer2_bitrate.SetBitrate(0, 1, 80000); - - // Expect the first and third rtp module to be asked to send a TargetBitrate + // Expect only the first rtp module to be asked to send a TargetBitrate // message. (No target bitrate with 0bps sent from the second one.) - EXPECT_CALL(rtp_1, SetVideoBitrateAllocation(layer0_bitrate)).Times(1); + EXPECT_CALL(rtp_1, SetVideoBitrateAllocation(bitrate)).Times(1); EXPECT_CALL(rtp_2, SetVideoBitrateAllocation(_)).Times(0); - EXPECT_CALL(rtp_3, SetVideoBitrateAllocation(layer2_bitrate)).Times(1); payload_router.OnBitrateAllocationUpdated(bitrate); } diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 6322dcbdc8..ec110ff694 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -1200,7 +1200,6 @@ VideoStream VideoQualityTest::DefaultVideoStream(const Params& params, stream.target_bitrate_bps = params.video[video_idx].target_bitrate_bps; stream.max_bitrate_bps = params.video[video_idx].max_bitrate_bps; stream.max_qp = kDefaultMaxQp; - stream.active = true; // TODO(sprang): Can we make this less of a hack? if (params.video[video_idx].num_temporal_layers == 2) { stream.temporal_layer_thresholds_bps.push_back(stream.target_bitrate_bps); diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index c8ab8a4799..a4d657be34 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -960,8 +960,7 @@ void VideoSendStreamImpl::OnEncoderConfigurationChanged( encoder_max_bitrate_bps_ = 0; double stream_bitrate_priority_sum = 0; for (const auto& stream : streams) { - // We don't want to allocate more bitrate than needed to inactive streams. - encoder_max_bitrate_bps_ += stream.active ? stream.max_bitrate_bps : 0; + encoder_max_bitrate_bps_ += stream.max_bitrate_bps; if (stream.bitrate_priority) { RTC_DCHECK_GT(*stream.bitrate_priority, 0); stream_bitrate_priority_sum += *stream.bitrate_priority; @@ -969,9 +968,6 @@ void VideoSendStreamImpl::OnEncoderConfigurationChanged( } RTC_DCHECK_GT(stream_bitrate_priority_sum, 0); encoder_bitrate_priority_ = stream_bitrate_priority_sum; - encoder_max_bitrate_bps_ = - std::max(static_cast(encoder_min_bitrate_bps_), - encoder_max_bitrate_bps_); max_padding_bitrate_ = CalculateMaxPadBitrateBps( streams, min_transmit_bitrate_bps, config_->suspend_below_min_bitrate);