Send rtcp target bitrate immediately on new bitrate allocation structure

If layers have been enabled or disabled, send immediate instead of on
next available report.

Bug: webrtc:9823
Change-Id: Ifd774641d4b8c03a9efa8ad48ff5e88328ed2ba9
Reviewed-on: https://webrtc-review.googlesource.com/c/103802
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24997}

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<rtcp::RtcpPacket> 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;
   {

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<VideoBitrateAllocation> 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_);

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