diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index 55f545c967..e49fc62ddd 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -150,7 +150,8 @@ RTCPSender::RTCPSender( app_length_(0), xr_send_receiver_reference_time_enabled_(false), - packet_type_counter_observer_(packet_type_counter_observer) { + packet_type_counter_observer_(packet_type_counter_observer), + send_video_bitrate_allocation_(false) { RTC_DCHECK(transport_ != nullptr); builders_[kRtcpSr] = &RTCPSender::BuildSR; @@ -607,20 +608,20 @@ std::unique_ptr RTCPSender::BuildExtendedReports( xr->AddDlrrItem(rti); } - if (video_bitrate_allocation_) { + if (send_video_bitrate_allocation_) { rtcp::TargetBitrate target_bitrate; for (int sl = 0; sl < kMaxSpatialLayers; ++sl) { for (int tl = 0; tl < kMaxTemporalStreams; ++tl) { - if (video_bitrate_allocation_->HasBitrate(sl, tl)) { + if (video_bitrate_allocation_.HasBitrate(sl, tl)) { target_bitrate.AddTargetBitrate( - sl, tl, video_bitrate_allocation_->GetBitrate(sl, tl) / 1000); + sl, tl, video_bitrate_allocation_.GetBitrate(sl, tl) / 1000); } } } xr->SetTargetBitrate(target_bitrate); - video_bitrate_allocation_.reset(); + send_video_bitrate_allocation_ = false; } if (xr_voip_metric_) { @@ -750,7 +751,8 @@ void RTCPSender::PrepareReport(const FeedbackState& feedback_state) { if (generate_report) { if ((!sending_ && xr_send_receiver_reference_time_enabled_) || - !feedback_state.last_xr_rtis.empty() || video_bitrate_allocation_) { + !feedback_state.last_xr_rtis.empty() || + send_video_bitrate_allocation_) { SetFlag(kRtcpAnyExtendedReports, true); } @@ -904,10 +906,34 @@ bool RTCPSender::AllVolatileFlagsConsumed() const { void RTCPSender::SetVideoBitrateAllocation( const VideoBitrateAllocation& bitrate) { rtc::CritScope lock(&critical_section_rtcp_sender_); - video_bitrate_allocation_.emplace(bitrate); + // Check if this allocation is first ever, or has a different set of + // spatial/temporal layers signaled and enabled, if so trigger an rtcp report + // as soon as possible. + if (HasNewLayerStructure(bitrate)) { + next_time_to_send_rtcp_ = clock_->TimeInMilliseconds(); + } + + video_bitrate_allocation_ = bitrate; + send_video_bitrate_allocation_ = true; SetFlag(kRtcpAnyExtendedReports, true); } +bool RTCPSender::HasNewLayerStructure( + const VideoBitrateAllocation& bitrate) const { + for (size_t si = 0; si < kMaxSpatialLayers; ++si) { + for (size_t ti = 0; ti < kMaxTemporalStreams; ++ti) { + if (bitrate.HasBitrate(si, ti) != + video_bitrate_allocation_.HasBitrate(si, ti) || + (bitrate.GetBitrate(si, ti) == 0) != + (video_bitrate_allocation_.GetBitrate(si, ti) == 0)) { + return true; + } + } + } + + return false; +} + bool RTCPSender::SendFeedbackPacket(const rtcp::TransportFeedback& packet) { size_t max_packet_size; { diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h index 133bf6809c..6ab3c1f1e0 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.h +++ b/modules/rtp_rtcp/source/rtcp_sender.h @@ -245,8 +245,12 @@ class RTCPSender { RtcpNackStats nack_stats_ RTC_GUARDED_BY(critical_section_rtcp_sender_); - absl::optional video_bitrate_allocation_ + VideoBitrateAllocation video_bitrate_allocation_ RTC_GUARDED_BY(critical_section_rtcp_sender_); + bool send_video_bitrate_allocation_ + RTC_GUARDED_BY(critical_section_rtcp_sender_); + bool HasNewLayerStructure(const VideoBitrateAllocation& bitrate) const + RTC_EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_); void SetFlag(uint32_t type, bool is_volatile) RTC_EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_); diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index a08ce0e83e..146cf41c38 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -700,4 +700,39 @@ TEST_F(RtcpSenderTest, SendXrWithTargetBitrate) { } } +TEST_F(RtcpSenderTest, SendImmediateXrWithTargetBitrate) { + // Initialize. Send a first report right away. + rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound); + EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpReport)); + clock_.AdvanceTimeMilliseconds(5); + + // Video bitrate allocation generated, save until next time we send a report. + VideoBitrateAllocation allocation; + allocation.SetBitrate(0, 0, 100000); + rtcp_sender_->SetVideoBitrateAllocation(allocation); + // First seen instance will be sent immediately. + EXPECT_TRUE(rtcp_sender_->TimeToSendRTCPReport(false)); + EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpReport)); + clock_.AdvanceTimeMilliseconds(5); + + // Update bitrate of existing layer, does not quality for immediate sending. + allocation.SetBitrate(0, 0, 150000); + rtcp_sender_->SetVideoBitrateAllocation(allocation); + EXPECT_FALSE(rtcp_sender_->TimeToSendRTCPReport(false)); + + // A new spatial layer enabled, signal this as soon as possible. + allocation.SetBitrate(1, 0, 200000); + rtcp_sender_->SetVideoBitrateAllocation(allocation); + EXPECT_TRUE(rtcp_sender_->TimeToSendRTCPReport(false)); + EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpReport)); + clock_.AdvanceTimeMilliseconds(5); + + // Explicitly disable top layer. The same set of layers now has a bitrate + // defined, but the explicit 0 indicates shutdown. Signal immediately. + allocation.SetBitrate(1, 0, 0); + EXPECT_FALSE(rtcp_sender_->TimeToSendRTCPReport(false)); + rtcp_sender_->SetVideoBitrateAllocation(allocation); + EXPECT_TRUE(rtcp_sender_->TimeToSendRTCPReport(false)); +} + } // namespace webrtc