Add a new function to BitrateAllocation: HasBitrate.

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 <stefan@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Erik Varga <erikvarga@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20852}

common_types.cc

@@ -181,7 +181,7 @@ VideoCodecType PayloadStringToCodecType(const std::string& name) {
 const uint32_t BitrateAllocation::kMaxBitrateBps =
     std::numeric_limits<uint32_t>::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<uint32_t>(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);

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

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

modules/rtp_rtcp/source/rtcp_sender.cc

@@ -655,10 +655,10 @@ std::unique_ptr<rtcp::RtcpPacket> 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);
+        }
       }
     }
 

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<VideoStream> CreateEncoderStreams(
+        int width,
+        int height,
+        const VideoEncoderConfig& encoder_config) override {
+      std::vector<VideoStream> 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<VideoReceiveStream::Config>* receive_configs,
       VideoEncoderConfig* encoder_config) override {
+    if (enable_zero_target_bitrate_) {
+      encoder_config->video_stream_factory =
+          new rtc::RefCountedObject<ZeroTargetVideoStreamFactory>();
+
+      // 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);
 }
 

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);
       }
     }