From 01f2ec35a6913675dbda2990b3fdb3cc3e95987d Mon Sep 17 00:00:00 2001 From: "erikvarga@webrtc.org" Date: Wed, 15 Nov 2017 14:58:23 +0100 Subject: [PATCH] Add a new function to BitrateAllocation: HasBitrate. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This can be used to determine whether the bitrate of a given spatial and temporal layer has been set in the allocation, even if the value it's set to is zero. GetBitrate still returns 0 if the queried layer does not have the bitrate set. Bug: webrtc:8479 Change-Id: I1d982e211da9b052fcccdbf588b67da1a4550c60 Reviewed-on: https://webrtc-review.googlesource.com/17440 Reviewed-by: Stefan Holmer Reviewed-by: Erik Språng Commit-Queue: Erik Varga Cr-Commit-Position: refs/heads/master@{#20852} --- common_types.cc | 21 +++++- common_types.h | 7 ++ media/engine/simulcast_encoder_adapter.cc | 4 +- modules/rtp_rtcp/source/rtcp_sender.cc | 8 +-- video/end_to_end_tests.cc | 82 +++++++++++++++++++++-- video/payload_router.cc | 8 ++- 6 files changed, 114 insertions(+), 16 deletions(-) diff --git a/common_types.cc b/common_types.cc index 59867d95c0..0526d59540 100644 --- a/common_types.cc +++ b/common_types.cc @@ -181,7 +181,7 @@ VideoCodecType PayloadStringToCodecType(const std::string& name) { const uint32_t BitrateAllocation::kMaxBitrateBps = std::numeric_limits::max(); -BitrateAllocation::BitrateAllocation() : sum_(0), bitrates_{} {} +BitrateAllocation::BitrateAllocation() : sum_(0), bitrates_{}, has_bitrate_{} {} bool BitrateAllocation::SetBitrate(size_t spatial_index, size_t temporal_index, @@ -196,10 +196,18 @@ bool BitrateAllocation::SetBitrate(size_t spatial_index, return false; bitrates_[spatial_index][temporal_index] = bitrate_bps; + has_bitrate_[spatial_index][temporal_index] = true; sum_ = static_cast(new_bitrate_sum_bps); return true; } +bool BitrateAllocation::HasBitrate(size_t spatial_index, + size_t temporal_index) const { + RTC_CHECK_LT(spatial_index, kMaxSpatialLayers); + RTC_CHECK_LT(temporal_index, kMaxTemporalStreams); + return has_bitrate_[spatial_index][temporal_index]; +} + uint32_t BitrateAllocation::GetBitrate(size_t spatial_index, size_t temporal_index) const { RTC_CHECK_LT(spatial_index, kMaxSpatialLayers); @@ -207,6 +215,17 @@ uint32_t BitrateAllocation::GetBitrate(size_t spatial_index, return bitrates_[spatial_index][temporal_index]; } +// Whether the specific spatial layers has the bitrate set in any of its +// temporal layers. +bool BitrateAllocation::IsSpatialLayerUsed(size_t spatial_index) const { + RTC_CHECK_LT(spatial_index, kMaxSpatialLayers); + for (int i = 0; i < kMaxTemporalStreams; ++i) { + if (has_bitrate_[spatial_index][i]) + return true; + } + return false; +} + // Get the sum of all the temporal layer for a specific spatial layer. uint32_t BitrateAllocation::GetSpatialLayerSum(size_t spatial_index) const { RTC_CHECK_LT(spatial_index, kMaxSpatialLayers); diff --git a/common_types.h b/common_types.h index f7c88b87b8..69bf5cc8ab 100644 --- a/common_types.h +++ b/common_types.h @@ -594,8 +594,14 @@ class BitrateAllocation { size_t temporal_index, uint32_t bitrate_bps); + bool HasBitrate(size_t spatial_index, size_t temporal_index) const; + uint32_t GetBitrate(size_t spatial_index, size_t temporal_index) const; + // Whether the specific spatial layers has the bitrate set in any of its + // temporal layers. + bool IsSpatialLayerUsed(size_t spatial_index) const; + // Get the sum of all the temporal layer for a specific spatial layer. uint32_t GetSpatialLayerSum(size_t spatial_index) const; @@ -616,6 +622,7 @@ class BitrateAllocation { private: uint32_t sum_; uint32_t bitrates_[kMaxSpatialLayers][kMaxTemporalStreams]; + bool has_bitrate_[kMaxSpatialLayers][kMaxTemporalStreams]; }; // Bandwidth over-use detector options. These are used to drive diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc index 3a285b8eb1..38d84bf085 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -458,7 +458,9 @@ int SimulcastEncoderAdapter::SetRateAllocation(const BitrateAllocation& bitrate, // the encoder handling the current simulcast stream. BitrateAllocation stream_allocation; for (int i = 0; i < kMaxTemporalStreams; ++i) { - stream_allocation.SetBitrate(0, i, bitrate.GetBitrate(stream_idx, i)); + if (bitrate.HasBitrate(stream_idx, i)) { + stream_allocation.SetBitrate(0, i, bitrate.GetBitrate(stream_idx, i)); + } } streaminfos_[stream_idx].encoder->SetRateAllocation(stream_allocation, new_framerate); diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index 5db08cd7ee..15b7e2fa96 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -655,10 +655,10 @@ std::unique_ptr RTCPSender::BuildExtendedReports( for (int sl = 0; sl < kMaxSpatialLayers; ++sl) { for (int tl = 0; tl < kMaxTemporalStreams; ++tl) { - uint32_t layer_bitrate_bps = - video_bitrate_allocation_->GetBitrate(sl, tl); - if (layer_bitrate_bps > 0) - target_bitrate.AddTargetBitrate(sl, tl, layer_bitrate_bps / 1000); + if (video_bitrate_allocation_->HasBitrate(sl, tl)) { + target_bitrate.AddTargetBitrate( + sl, tl, video_bitrate_allocation_->GetBitrate(sl, tl) / 1000); + } } } diff --git a/video/end_to_end_tests.cc b/video/end_to_end_tests.cc index 041fa177c4..5b5be324bb 100644 --- a/video/end_to_end_tests.cc +++ b/video/end_to_end_tests.cc @@ -3690,14 +3690,17 @@ TEST_P(EndToEndTest, TimingFramesAreReported) { class RtcpXrObserver : public test::EndToEndTest { public: - RtcpXrObserver(bool enable_rrtr, bool enable_target_bitrate) + RtcpXrObserver(bool enable_rrtr, bool enable_target_bitrate, + bool enable_zero_target_bitrate) : EndToEndTest(test::CallTest::kDefaultTimeoutMs), enable_rrtr_(enable_rrtr), enable_target_bitrate_(enable_target_bitrate), + enable_zero_target_bitrate_(enable_zero_target_bitrate), sent_rtcp_sr_(0), sent_rtcp_rr_(0), sent_rtcp_rrtr_(0), sent_rtcp_target_bitrate_(false), + sent_zero_rtcp_target_bitrate_(false), sent_rtcp_dlrr_(0) {} private: @@ -3730,13 +3733,22 @@ class RtcpXrObserver : public test::EndToEndTest { EXPECT_FALSE(parser.xr()->rrtr()); if (parser.xr()->dlrr()) ++sent_rtcp_dlrr_; - if (parser.xr()->target_bitrate()) + if (parser.xr()->target_bitrate()) { sent_rtcp_target_bitrate_ = true; + for (const rtcp::TargetBitrate::BitrateItem& item : + parser.xr()->target_bitrate()->GetTargetBitrates()) { + if (item.target_bitrate_kbps == 0) { + sent_zero_rtcp_target_bitrate_ = true; + break; + } + } + } } if (sent_rtcp_sr_ > kNumRtcpReportPacketsToObserve && sent_rtcp_rr_ > kNumRtcpReportPacketsToObserve && - (sent_rtcp_target_bitrate_ || !enable_target_bitrate_)) { + (sent_rtcp_target_bitrate_ || !enable_target_bitrate_) && + (sent_zero_rtcp_target_bitrate_ || !enable_zero_target_bitrate_)) { if (enable_rrtr_) { EXPECT_GT(sent_rtcp_rrtr_, 0); EXPECT_GT(sent_rtcp_dlrr_, 0); @@ -3745,15 +3757,59 @@ class RtcpXrObserver : public test::EndToEndTest { EXPECT_EQ(sent_rtcp_dlrr_, 0); } EXPECT_EQ(enable_target_bitrate_, sent_rtcp_target_bitrate_); + EXPECT_EQ(enable_zero_target_bitrate_, sent_zero_rtcp_target_bitrate_); observation_complete_.Set(); } return SEND_PACKET; } + size_t GetNumVideoStreams() const override { + // When sending a zero target bitrate, we use two spatial layers so that + // we'll still have a layer with non-zero bitrate. + return enable_zero_target_bitrate_ ? 2 : 1; + } + + // This test uses VideoStream settings different from the the default one + // implemented in DefaultVideoStreamFactory, so it implements its own + // VideoEncoderConfig::VideoStreamFactoryInterface which is created + // in ModifyVideoConfigs. + class ZeroTargetVideoStreamFactory + : public VideoEncoderConfig::VideoStreamFactoryInterface { + public: + ZeroTargetVideoStreamFactory() {} + + private: + std::vector CreateEncoderStreams( + int width, + int height, + const VideoEncoderConfig& encoder_config) override { + std::vector streams = + test::CreateVideoStreams(width, height, encoder_config); + // Set one of the streams' target bitrates to zero to test that a + // bitrate of 0 can be signalled. + streams[encoder_config.number_of_streams-1].min_bitrate_bps = 0; + streams[encoder_config.number_of_streams-1].target_bitrate_bps = 0; + streams[encoder_config.number_of_streams-1].max_bitrate_bps = 0; + return streams; + } + }; + void ModifyVideoConfigs( VideoSendStream::Config* send_config, std::vector* receive_configs, VideoEncoderConfig* encoder_config) override { + if (enable_zero_target_bitrate_) { + encoder_config->video_stream_factory = + new rtc::RefCountedObject(); + + // Configure VP8 to be able to use simulcast. + send_config->encoder_settings.payload_name = "VP8"; + (*receive_configs)[0].decoders.resize(1); + (*receive_configs)[0].decoders[0].payload_type = + send_config->encoder_settings.payload_type; + (*receive_configs)[0].decoders[0].payload_name = + send_config->encoder_settings.payload_name; + } if (enable_target_bitrate_) { // TargetBitrate only signaled for screensharing. encoder_config->content_type = VideoEncoderConfig::ContentType::kScreen; @@ -3773,30 +3829,42 @@ class RtcpXrObserver : public test::EndToEndTest { rtc::CriticalSection crit_; const bool enable_rrtr_; const bool enable_target_bitrate_; + const bool enable_zero_target_bitrate_; int sent_rtcp_sr_; int sent_rtcp_rr_ RTC_GUARDED_BY(&crit_); int sent_rtcp_rrtr_ RTC_GUARDED_BY(&crit_); bool sent_rtcp_target_bitrate_ RTC_GUARDED_BY(&crit_); + bool sent_zero_rtcp_target_bitrate_ RTC_GUARDED_BY(&crit_); int sent_rtcp_dlrr_; }; TEST_P(EndToEndTest, TestExtendedReportsWithRrtrWithoutTargetBitrate) { - RtcpXrObserver test(true, false); + RtcpXrObserver test(/*enable_rrtr=*/true, /*enable_target_bitrate=*/false, + /*enable_zero_target_bitrate=*/false); RunBaseTest(&test); } TEST_P(EndToEndTest, TestExtendedReportsWithoutRrtrWithoutTargetBitrate) { - RtcpXrObserver test(false, false); + RtcpXrObserver test(/*enable_rrtr=*/false, /*enable_target_bitrate=*/false, + /*enable_zero_target_bitrate=*/false); RunBaseTest(&test); } TEST_P(EndToEndTest, TestExtendedReportsWithRrtrWithTargetBitrate) { - RtcpXrObserver test(true, true); + RtcpXrObserver test(/*enable_rrtr=*/true, /*enable_target_bitrate=*/true, + /*enable_zero_target_bitrate=*/false); RunBaseTest(&test); } TEST_P(EndToEndTest, TestExtendedReportsWithoutRrtrWithTargetBitrate) { - RtcpXrObserver test(false, true); + RtcpXrObserver test(/*enable_rrtr=*/false, /*enable_target_bitrate=*/true, + /*enable_zero_target_bitrate=*/false); + RunBaseTest(&test); +} + +TEST_P(EndToEndTest, TestExtendedReportsCanSignalZeroTargetBitrate) { + RtcpXrObserver test(/*enable_rrtr=*/false, /*enable_target_bitrate=*/true, + /*enable_zero_target_bitrate=*/true); RunBaseTest(&test); } diff --git a/video/payload_router.cc b/video/payload_router.cc index 496186c069..f43a773e5d 100644 --- a/video/payload_router.cc +++ b/video/payload_router.cc @@ -238,12 +238,14 @@ 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.GetSpatialLayerSum(si) == 0) + if (!bitrate.IsSpatialLayerUsed(si)) break; BitrateAllocation layer_bitrate; - for (int tl = 0; tl < kMaxTemporalStreams; ++tl) - layer_bitrate.SetBitrate(0, tl, bitrate.GetBitrate(si, tl)); + for (int tl = 0; tl < kMaxTemporalStreams; ++tl) { + if (bitrate.HasBitrate(si, tl)) + layer_bitrate.SetBitrate(0, tl, bitrate.GetBitrate(si, tl)); + } rtp_modules_[si]->SetVideoBitrateAllocation(layer_bitrate); } }