Enabling rtcp-rsize negotiation and fixing some issues with it.

webrtc/media/engine/webrtcvideoengine2.cc

 /*
  *  Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 798 matching lines @@
   }
 
   if (changed_params.options)
     send_params_.options.SetAll(*changed_params.options);
 
   {
     rtc::CritScope stream_lock(&stream_crit_);
     for (auto& kv : send_streams_) {
       kv.second->SetSendParameters(changed_params);
     }
-    if (changed_params.codec) {
-      // Update receive feedback parameters from new codec.
+    if (changed_params.codec || changed_params.rtcp_mode) {
+      // Update receive feedback parameters from new codec or RTCP mode.
       LOG(LS_INFO)
           << "SetFeedbackOptions on all the receive streams because the send "
-             "codec has changed.";
+             "codec or RTCP mode has changed.";
       for (auto& kv : receive_streams_) {
         RTC_DCHECK(kv.second != nullptr);
-        kv.second->SetFeedbackParameters(HasNack(send_codec_->codec),
-                                         HasRemb(send_codec_->codec),
-                                         HasTransportCc(send_codec_->codec));
+        kv.second->SetFeedbackParameters(
+            HasNack(send_codec_->codec), HasRemb(send_codec_->codec),
+            HasTransportCc(send_codec_->codec),
+            params.rtcp.reduced_size ? webrtc::RtcpMode::kReducedSize
+                                     : webrtc::RtcpMode::kCompound);
       }
     }
   }
   send_params_ = params;
   return true;
 }
 
 bool WebRtcVideoChannel2::GetChangedRecvParameters(
     const VideoRecvParameters& params,
     ChangedRecvParameters* changed_params) const {
@@ skipping 24 matching lines @@
   }
 
   // Handle RTP header extensions.
   std::vector<webrtc::RtpExtension> filtered_extensions = FilterRtpExtensions(
       params.extensions, webrtc::RtpExtension::IsSupportedForVideo, false);
   if (filtered_extensions != recv_rtp_extensions_) {
     changed_params->rtp_header_extensions =
         rtc::Optional<std::vector<webrtc::RtpExtension>>(filtered_extensions);
   }
 
-  // Handle RTCP mode.
-  if (params.rtcp.reduced_size != recv_params_.rtcp.reduced_size) {
-    changed_params->rtcp_mode = rtc::Optional<webrtc::RtcpMode>(
-        params.rtcp.reduced_size ? webrtc::RtcpMode::kReducedSize
-                                 : webrtc::RtcpMode::kCompound);
-  }
-
   return true;
 }
 
 bool WebRtcVideoChannel2::SetRecvParameters(const VideoRecvParameters& params) {
   TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetRecvParameters");
   LOG(LS_INFO) << "SetRecvParameters: " << params.ToString();
   ChangedRecvParameters changed_params;
   if (!GetChangedRecvParameters(params, &changed_params)) {
     return false;
   }
@@ skipping 250 matching lines @@
 
 void WebRtcVideoChannel2::ConfigureReceiverRtp(
     webrtc::VideoReceiveStream::Config* config,
     const StreamParams& sp) const {
   uint32_t ssrc = sp.first_ssrc();
 
   config->rtp.remote_ssrc = ssrc;
   config->rtp.local_ssrc = rtcp_receiver_report_ssrc_;
 
   config->rtp.extensions = recv_rtp_extensions_;
-  config->rtp.rtcp_mode = recv_params_.rtcp.reduced_size
+  // Whether or not the receive stream sends reduced size RTCP is determined
+  // by the send params.
+  config->rtp.rtcp_mode = send_params_.rtcp.reduced_size
                               ? webrtc::RtcpMode::kReducedSize
                               : webrtc::RtcpMode::kCompound;

[pthatcher1, 2016/02/19 07:24:21]

I really don't like the idea that the send parameters dictate what the
RtpReceiver does.  RtpSenders and RtpReceivers can be completely independent.  A
PeerConnection may have only RtpReceivers, and no (active) RtpSenders. 
Similarly, ORTC may have only RtpReceivers, and no send parameters.  

Also, the RtcpMode does not only come from the remote description.  It's
possible to send RTCP after having only applied the local description, and it's
possible for the local description to set the mode to reduced-size (when it's
the answer).  While it's true that when the local description is an offer, we
need to wait for the remote answer to know if reduced-size will work, it should
be up to the PeerConnection to convert a remote answer into updated receive
parameters, not the responsibility of the RtpReceiver to use send parameters to
update itself to send different RTCP based on the RtpSenders around it.

Finally, in ORTC, the RtpSender and RtpReceiver has independent parameters for
whether or not they will use reduced-size RTCP, so eventually we have to allow
RtpReceivers to get them independently from their own parameters.

[pbos-webrtc, 2016/02/19 15:57:39]

But SetRemoteDescription determines whether the sender can understand rsize or
not, right? All receivers send RTCP, so receivers are in that sense senders as
well.

 
   // TODO(pbos): This protection is against setting the same local ssrc as
   // remote which is not permitted by the lower-level API. RTCP requires a
   // corresponding sender SSRC. Figure out what to do when we don't have
   // (receive-only) or know a good local SSRC.
   if (config->rtp.remote_ssrc == config->rtp.local_ssrc) {
     if (config->rtp.local_ssrc != kDefaultRtcpReceiverReportSsrc) {
       config->rtp.local_ssrc = kDefaultRtcpReceiverReportSsrc;
     } else {
       config->rtp.local_ssrc = kDefaultRtcpReceiverReportSsrc + 1;
@@ skipping 1061 matching lines @@
   config_.rtp.local_ssrc = local_ssrc;
   LOG(LS_INFO)
       << "RecreateWebRtcStream (recv) because of SetLocalSsrc; local_ssrc="
       << local_ssrc;
   RecreateWebRtcStream();
 }
 
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetFeedbackParameters(
     bool nack_enabled,
     bool remb_enabled,
-    bool transport_cc_enabled) {
+    bool transport_cc_enabled,
+    webrtc::RtcpMode rtcp_mode) {
   int nack_history_ms = nack_enabled ? kNackHistoryMs : 0;
   if (config_.rtp.nack.rtp_history_ms == nack_history_ms &&
       config_.rtp.remb == remb_enabled &&
-      config_.rtp.transport_cc == transport_cc_enabled) {
+      config_.rtp.transport_cc == transport_cc_enabled &&
+      config_.rtp.rtcp_mode == rtcp_mode) {
     LOG(LS_INFO)
         << "Ignoring call to SetFeedbackParameters because parameters are "
            "unchanged; nack="
         << nack_enabled << ", remb=" << remb_enabled
         << ", transport_cc=" << transport_cc_enabled;
     return;
   }
   config_.rtp.remb = remb_enabled;
   config_.rtp.nack.rtp_history_ms = nack_history_ms;
   config_.rtp.transport_cc = transport_cc_enabled;
+  config_.rtp.rtcp_mode = rtcp_mode;
   LOG(LS_INFO)
       << "RecreateWebRtcStream (recv) because of SetFeedbackParameters; nack="
       << nack_enabled << ", remb=" << remb_enabled
       << ", transport_cc=" << transport_cc_enabled;
   RecreateWebRtcStream();
 }
 
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvParameters(
     const ChangedRecvParameters& params) {
   bool needs_recreation = false;
   std::vector<AllocatedDecoder> old_decoders;
   if (params.codec_settings) {
     ConfigureCodecs(*params.codec_settings, &old_decoders);
     needs_recreation = true;
   }
   if (params.rtp_header_extensions) {
     config_.rtp.extensions = *params.rtp_header_extensions;
     needs_recreation = true;
   }
-  if (params.rtcp_mode) {
-    config_.rtp.rtcp_mode = *params.rtcp_mode;
-    needs_recreation = true;
-  }
   if (needs_recreation) {
     LOG(LS_INFO) << "RecreateWebRtcStream (recv) because of SetRecvParameters";
     RecreateWebRtcStream();
     ClearDecoders(&old_decoders);
   }
 }
 
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream() {
   if (stream_ != NULL) {
     call_->DestroyVideoReceiveStream(stream_);
@@ skipping 222 matching lines @@
         rtx_mapping[video_codecs[i].codec.id] !=
             fec_settings.red_payload_type) {
       video_codecs[i].rtx_payload_type = rtx_mapping[video_codecs[i].codec.id];
     }
   }
 
   return video_codecs;
 }
 
 }  // namespace cricket