Parse FlexFEC RTP headers in Call and add integration with BWE.

webrtc/modules/congestion_controller/congestion_controller.cc

Index: webrtc/modules/congestion_controller/congestion_controller.cc
diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc
index 44b2f41e86b85117b1e121ec6c56de1a028c29d4..9f38e5bf674b3d2dd1234c02db1b5e38e8f72d28 100644
--- a/webrtc/modules/congestion_controller/congestion_controller.cc
+++ b/webrtc/modules/congestion_controller/congestion_controller.cc
@@ -263,6 +263,17 @@ void CongestionController::EnablePeriodicAlrProbing(bool enable) {
   probe_controller_->EnablePeriodicAlrProbing(enable);
 }
 
+bool CongestionController::UseSendSideBwe(

[philipel, 2016/12/13 13:58:27]

If this function is still used, could it be moved to that .cc file?

[brandtr, 2016/12/14 12:55:49]

Moved it back to video_receive_stream.cc. When the receive media pipeline is
upgrade to use RtpPacketReceived, it can be removed from there.

+    bool transport_cc,
+    const RtpHeaderExtensionMap& rtp_header_extensions) {
+  // Has the RTCP feedback message been negotiated?
+  if (!transport_cc)
+    return false;
+  // Has the RTP header extension been negotiated?

[nisse-webrtc, 2016/12/13 11:46:42]

Do you still need it? The version I deleted didn't live here, but in
audio_receive_stream.cc, see

https://codereview.webrtc.org/2516983004/diff/140001/webrtc/audio/audio_recei...

For that change, we concluded that there wasn't any need to check the list of
negotiated extensions, but instead just check what's in the parsed headers of
each rtp packet. And then trust the parser to never insert anything which wasn't
properly negotiated.

[brandtr, 2016/12/13 12:37:36]

Ok, I understand. In this CL, I moved the implementation that existed in
video_receive_stream.cc:
https://codereview.webrtc.org/2553863003/diff/100001/webrtc/video/video_recei...

If I understand this function correctly, we use it to make sure that we only
return the RemoteEstimatorProxy whenever both the RTCP report block and the RTP
header extensions have been negotiated properly. Do you mean that it's ok to
only check that the RTCP report block has been negotiated instead?

Or is the difference to your CL that if we do audio BWE, we only ever use
send-side BWE? Whereas for video, we might be using some of the older BWEs.

I also rely on the parsing to decide if a packet should go into the BWE, or not:
https://codereview.webrtc.org/2553863003/diff/100001/webrtc/modules/rtp_rtcp/....

[nisse-webrtc, 2016/12/13 12:59:14]

I'm afraid I don't understand why the logic is different in
audio_receive_stream.cc and video_receive_stream.cc. The former uses a check
like

   if (config_.rtp.transport_cc &&
      header.extension.hasTransportSequenceNumber) {

in the DeliverRtp method. And that's the only use of the transport_cc flag in
that file.

[brandtr, 2016/12/14 12:55:49]

The audio path has simpler logic because only send-side BWE is used there, if
any BWE is used at all. (To my understanding.) In the video path, we need to
support the old BWEs as well. Anyway, I have used your idea in
internal::Call:NotifyBweOfReceivedPacket :)

+  return rtp_header_extensions.IsRegistered(
+      kRtpExtensionTransportSequenceNumber);
+}
+
 void CongestionController::SetAllocatedSendBitrateLimits(
     int min_send_bitrate_bps,
     int max_padding_bitrate_bps) {