Wire up BitrateAllocation to be sent as RTCP TargetBitrate

webrtc/modules/rtp_rtcp/source/rtcp_sender.cc

Index: webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
index e6cf6695226fcd803558ddf5b4f2b80aae8f2c7a..2aff409ba6b9a6dbd329db845406df14bad7fa4f 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -197,10 +197,7 @@ RTCPSender::RTCPSender(
   builders_[kRtcpTmmbr] = &RTCPSender::BuildTMMBR;
   builders_[kRtcpTmmbn] = &RTCPSender::BuildTMMBN;
   builders_[kRtcpNack] = &RTCPSender::BuildNACK;
-  builders_[kRtcpXrVoipMetric] = &RTCPSender::BuildVoIPMetric;
-  builders_[kRtcpXrReceiverReferenceTime] =
-      &RTCPSender::BuildReceiverReferenceTime;
-  builders_[kRtcpXrDlrrReportBlock] = &RTCPSender::BuildDlrr;
+  builders_[kRtcpAnyExtendedReports] = &RTCPSender::BuildExtendedReports;
 }
 
 RTCPSender::~RTCPSender() {}
@@ -692,44 +689,47 @@ std::unique_ptr<rtcp::RtcpPacket> RTCPSender::BuildBYE(const RtcpContext& ctx) {
   return std::unique_ptr<rtcp::RtcpPacket>(bye);
 }
 
-std::unique_ptr<rtcp::RtcpPacket> RTCPSender::BuildReceiverReferenceTime(
+std::unique_ptr<rtcp::RtcpPacket> RTCPSender::BuildExtendedReports(
     const RtcpContext& ctx) {
-
-  rtcp::ExtendedReports* xr = new rtcp::ExtendedReports();
+  std::unique_ptr<rtcp::ExtendedReports> xr(new rtcp::ExtendedReports());
   xr->SetSenderSsrc(ssrc_);
 
-  rtcp::Rrtr rrtr;
-  rrtr.SetNtp(NtpTime(ctx.ntp_sec_, ctx.ntp_frac_));
-
-  xr->SetRrtr(rrtr);
-
-  // TODO(sprang): Merge XR report sending to contain all of RRTR, DLRR, VOIP?
+  if (!sending_ && xr_send_receiver_reference_time_enabled_) {
+    rtcp::Rrtr rrtr;
+    rrtr.SetNtp(NtpTime(ctx.ntp_sec_, ctx.ntp_frac_));
+    xr->SetRrtr(rrtr);
+  }
 
-  return std::unique_ptr<rtcp::RtcpPacket>(xr);
-}
+  if (ctx.feedback_state_.has_last_xr_rr) {
+    xr->AddDlrrItem(ctx.feedback_state_.last_xr_rr);
+  }
 
-std::unique_ptr<rtcp::RtcpPacket> RTCPSender::BuildDlrr(
-    const RtcpContext& ctx) {
-  rtcp::ExtendedReports* xr = new rtcp::ExtendedReports();
-  xr->SetSenderSsrc(ssrc_);
-  RTC_DCHECK(ctx.feedback_state_.has_last_xr_rr);
-  xr->AddDlrrItem(ctx.feedback_state_.last_xr_rr);
+  if (video_bitrate_allocation_) {
+    rtcp::TargetBitrate target_bitrate;
 
-  return std::unique_ptr<rtcp::RtcpPacket>(xr);
-}
+    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);
+      }
+    }
 
-std::unique_ptr<rtcp::RtcpPacket> RTCPSender::BuildVoIPMetric(
-    const RtcpContext& context) {
-  rtcp::ExtendedReports* xr = new rtcp::ExtendedReports();
-  xr->SetSenderSsrc(ssrc_);
+    xr->SetTargetBitrate(target_bitrate);
+    video_bitrate_allocation_.reset();
+  }
 
-  rtcp::VoipMetric voip;
-  voip.SetMediaSsrc(remote_ssrc_);
-  voip.SetVoipMetric(xr_voip_metric_);
+  if (xr_voip_metric_) {
+    rtcp::VoipMetric voip;
+    voip.SetMediaSsrc(remote_ssrc_);
+    voip.SetVoipMetric(*xr_voip_metric_);
+    xr_voip_metric_.reset();
 
-  xr->SetVoipMetric(voip);
+    xr->SetVoipMetric(voip);
+  }
 
-  return std::unique_ptr<rtcp::RtcpPacket>(xr);
+  return std::move(xr);
 }
 
 int32_t RTCPSender::SendRTCP(const FeedbackState& feedback_state,
@@ -794,7 +794,8 @@ int32_t RTCPSender::SendCompoundRTCP(
     auto it = report_flags_.begin();
     while (it != report_flags_.end()) {
       auto builder_it = builders_.find(it->type);
-      RTC_DCHECK(builder_it != builders_.end());
+      RTC_DCHECK(builder_it != builders_.end())
+          << "Could not find builder for packet type " << it->type;
       if (it->is_volatile) {
         report_flags_.erase(it++);
       } else {
@@ -849,10 +850,10 @@ void RTCPSender::PrepareReport(const FeedbackState& feedback_state) {
     SetFlag(kRtcpSdes, true);
 
   if (generate_report) {
-    if (!sending_ && xr_send_receiver_reference_time_enabled_)
-      SetFlag(kRtcpXrReceiverReferenceTime, true);
-    if (feedback_state.has_last_xr_rr)
-      SetFlag(kRtcpXrDlrrReportBlock, true);
+    if ((!sending_ && xr_send_receiver_reference_time_enabled_) ||
+        feedback_state.has_last_xr_rr || video_bitrate_allocation_) {

[stefan-webrtc, 2016/12/01 08:42:20]

Seems like we now have two checks for this. First here we check if any extended
reports should be added, then in BuildExtendedReports we check each of these
again to see which of them should be added. Why not keep as it was and just
SetFlag on those which are available here?

[sprang_webrtc, 2016/12/01 10:47:58]

Because we don't want to have separate builders for each type, creating
individual extended reports containing a single type. Any of them should trigger
just a single builder. If we set all the flags here, and map each of these types
to the same extended reports builder, we would call that multiple times, and
would then there have to check again if the various xr types have already been
built. Or we would have to do a concurrent modification of the set that's being
iterated over. Neither are very nice options.

+      SetFlag(kRtcpAnyExtendedReports, true);
+    }
 
     // generate next time to send an RTCP report
     uint32_t minIntervalMs = RTCP_INTERVAL_AUDIO_MS;
@@ -959,9 +960,9 @@ int32_t RTCPSender::SetApplicationSpecificData(uint8_t subType,
 
 int32_t RTCPSender::SetRTCPVoIPMetrics(const RTCPVoIPMetric* VoIPMetric) {
   rtc::CritScope lock(&critical_section_rtcp_sender_);
-  memcpy(&xr_voip_metric_, VoIPMetric, sizeof(RTCPVoIPMetric));
+  xr_voip_metric_.emplace(*VoIPMetric);
 
-  SetFlag(kRtcpXrVoipMetric, true);
+  SetFlag(kRtcpAnyExtendedReports, true);
   return 0;
 }
 
@@ -987,8 +988,13 @@ void RTCPSender::SetFlag(RTCPPacketType type, bool is_volatile) {
 
 void RTCPSender::SetFlags(const std::set<RTCPPacketType>& types,
                           bool is_volatile) {
-  for (RTCPPacketType type : types)
-    SetFlag(type, is_volatile);
+  for (RTCPPacketType type : types) {
+    if (type & kRtcpAnyExtendedReports) {
+      SetFlag(kRtcpAnyExtendedReports, is_volatile);
+    } else {
+      SetFlag(type, is_volatile);
+    }
+  }
 }
 
 bool RTCPSender::IsFlagPresent(RTCPPacketType type) const {
@@ -1012,6 +1018,11 @@ bool RTCPSender::AllVolatileFlagsConsumed() const {
   return true;
 }
 
+void RTCPSender::SetVideoBitrateAllocation(const BitrateAllocation& bitrate) {
+  rtc::CritScope lock(&critical_section_rtcp_sender_);
+  video_bitrate_allocation_.emplace(bitrate);
+}
+
 bool RTCPSender::SendFeedbackPacket(const rtcp::TransportFeedback& packet) {
   class Sender : public rtcp::RtcpPacket::PacketReadyCallback {
    public: