Turning off a stream should results in target bitrate 0 signal

Bug: webrtc:9734, b:116850043
Change-Id: Ia7b4a8ecf2099c3026c83b06febca833d428d0a2
Reviewed-on: https://webrtc-review.googlesource.com/c/103981
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25014}

modules/rtp_rtcp/source/rtcp_sender.cc

@@ -891,29 +891,40 @@ void RTCPSender::SetVideoBitrateAllocation(
   // 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)) {
+  absl::optional<VideoBitrateAllocation> new_bitrate =
+      CheckAndUpdateLayerStructure(bitrate);
+  if (new_bitrate) {
+    video_bitrate_allocation_ = *new_bitrate;
     next_time_to_send_rtcp_ = clock_->TimeInMilliseconds();
+  } else {
+    video_bitrate_allocation_ = bitrate;
   }
 
-  video_bitrate_allocation_ = bitrate;
   send_video_bitrate_allocation_ = true;
   SetFlag(kRtcpAnyExtendedReports, true);
 }
 
-bool RTCPSender::HasNewLayerStructure(
+absl::optional<VideoBitrateAllocation> RTCPSender::CheckAndUpdateLayerStructure(
     const VideoBitrateAllocation& bitrate) const {
+  absl::optional<VideoBitrateAllocation> updated_bitrate;
   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;
+      if (!updated_bitrate &&
+          (bitrate.HasBitrate(si, ti) !=
+               video_bitrate_allocation_.HasBitrate(si, ti) ||
+           (bitrate.GetBitrate(si, ti) == 0) !=
+               (video_bitrate_allocation_.GetBitrate(si, ti) == 0))) {
+        updated_bitrate = bitrate;
+      }
+      if (video_bitrate_allocation_.GetBitrate(si, ti) > 0 &&
+          bitrate.GetBitrate(si, ti) == 0) {
+        // Make sure this stream disabling is explicitly signaled.
+        updated_bitrate->SetBitrate(si, ti, 0);
       }
     }
   }
 
-  return false;
+  return updated_bitrate;
 }
 
 bool RTCPSender::SendFeedbackPacket(const rtcp::TransportFeedback& packet) {

modules/rtp_rtcp/source/rtcp_sender.h

@@ -244,7 +244,8 @@ class RTCPSender {
       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
+  absl::optional<VideoBitrateAllocation> CheckAndUpdateLayerStructure(
+      const VideoBitrateAllocation& bitrate) const
       RTC_EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
 
   void SetFlag(uint32_t type, bool is_volatile)

modules/rtp_rtcp/source/rtcp_sender_unittest.cc

@@ -681,4 +681,41 @@ TEST_F(RtcpSenderTest, SendImmediateXrWithTargetBitrate) {
   EXPECT_TRUE(rtcp_sender_->TimeToSendRTCPReport(false));
 }
 
+TEST_F(RtcpSenderTest, SendTargetBitrateExplicitZeroOnStreamRemoval) {
+  // Set up and send a bitrate allocation with two layers.
+
+  rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound);
+  VideoBitrateAllocation allocation;
+  allocation.SetBitrate(0, 0, 100000);
+  allocation.SetBitrate(1, 0, 200000);
+  rtcp_sender_->SetVideoBitrateAllocation(allocation);
+  EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpReport));
+  absl::optional<rtcp::TargetBitrate> target_bitrate =
+      parser()->xr()->target_bitrate();
+  ASSERT_TRUE(target_bitrate);
+  std::vector<rtcp::TargetBitrate::BitrateItem> bitrates =
+      target_bitrate->GetTargetBitrates();
+  ASSERT_EQ(2u, bitrates.size());
+  EXPECT_EQ(bitrates[0].target_bitrate_kbps,
+            allocation.GetBitrate(0, 0) / 1000);
+  EXPECT_EQ(bitrates[1].target_bitrate_kbps,
+            allocation.GetBitrate(1, 0) / 1000);
+
+  // Create a new allocation, where the second stream is no longer available.
+  VideoBitrateAllocation new_allocation;
+  new_allocation.SetBitrate(0, 0, 150000);
+  rtcp_sender_->SetVideoBitrateAllocation(new_allocation);
+  EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpReport));
+  target_bitrate = parser()->xr()->target_bitrate();
+  ASSERT_TRUE(target_bitrate);
+  bitrates = target_bitrate->GetTargetBitrates();
+
+  // Two bitrates should still be set, with an explicit entry indicating the
+  // removed stream is gone.
+  ASSERT_EQ(2u, bitrates.size());
+  EXPECT_EQ(bitrates[0].target_bitrate_kbps,
+            new_allocation.GetBitrate(0, 0) / 1000);
+  EXPECT_EQ(bitrates[1].target_bitrate_kbps, 0u);
+}
+
 }  // namespace webrtc