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

webrtc/media/engine/webrtcvideoengine2.cc

Index: webrtc/media/engine/webrtcvideoengine2.cc
diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc
index c6bdafee5a336666a4fd46f5aab78c0590525105..9fd597e873ee11a0ad9ae378c4c8b47f075d09dd 100644
--- a/webrtc/media/engine/webrtcvideoengine2.cc
+++ b/webrtc/media/engine/webrtcvideoengine2.cc
@@ -816,16 +816,18 @@ bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) {
     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);
       }
     }
   }
@@ -870,13 +872,6 @@ bool WebRtcVideoChannel2::GetChangedRecvParameters(
         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;
 }
 
@@ -1147,7 +1142,9 @@ void WebRtcVideoChannel2::ConfigureReceiverRtp(
   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.

 
@@ -2231,11 +2228,13 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetLocalSsrc(
 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="
@@ -2246,6 +2245,7 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetFeedbackParameters(
   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
@@ -2265,10 +2265,6 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvParameters(
     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();