Bugfix OnNetworkChanged not triggered for RTCP compund messages if TMMBR is higher than last value.

TBR=mflodman
Review URL: http://webrtc-codereview.appspot.com/342001

git-svn-id: http://webrtc.googlecode.com/svn/trunk@1344 4adac7df-926f-26a2-2b94-8c16560cd09d

src/modules/rtp_rtcp/source/rtcp_receiver.cc

@@ -1301,29 +1301,37 @@ RTCPReceiver::OnReceivedReferencePictureSelectionIndication(const WebRtc_UWord64
 }
 
 // Holding no Critical section
-void
+void RTCPReceiver::TriggerCallbacksFromRTCPPacket(
-RTCPReceiver::TriggerCallbacksFromRTCPPacket(RTCPPacketInformation& rtcpPacketInformation)
+    RTCPPacketInformation& rtcpPacketInformation)
 {
-    // callback if SR or RR
+    // Process TMMBR and REMB first to avoid multiple callbacks
+    // to OnNetworkChanged.  
+    if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpTmmbr)
+    {
+        WEBRTC_TRACE(kTraceStateInfo, kTraceRtpRtcp, _id,
+                     "SIG [RTCP] Incoming TMMBR to id:%d", _id);
+
+        // Might trigger a OnReceivedBandwidthEstimateUpdate.
+        _rtpRtcp.OnReceivedTMMBR();
+    }
+    if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpRemb)
+    {
+        WEBRTC_TRACE(kTraceStateInfo, kTraceRtpRtcp, _id,
+                     "SIG [RTCP] Incoming REMB to id:%d", _id);
+
+       // We need to bounce this to the default channel.
+        _rtpRtcp.OnReceivedEstimatedMaxBitrate(
+            rtcpPacketInformation.receiverEstimatedMaxBitrate);
+    }
     if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpSr ||
         rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpRr)
     {
-        if(rtcpPacketInformation.reportBlock)
+        if (rtcpPacketInformation.reportBlock)
         {
-            // We only want to trigger one OnNetworkChanged callback per RTCP
-            // packet. The callback is triggered by a SR, RR, REMB or TMMBR, so
-            // we don't want to trigger one from here if the packet also
-            // contains a REMB or TMMBR block.
-            bool triggerCallback = true;
-            if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpRemb ||
-                rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpTmmbr) {
-              triggerCallback = false;
-            }
             _rtpRtcp.OnPacketLossStatisticsUpdate(
                 rtcpPacketInformation.fractionLost,
                 rtcpPacketInformation.roundTripTime,
-                rtcpPacketInformation.lastReceivedExtendedHighSeqNum,
+                rtcpPacketInformation.lastReceivedExtendedHighSeqNum);
-                triggerCallback);
         }
     }
     if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpSr)
@@ -1338,27 +1346,24 @@ RTCPReceiver::TriggerCallbacksFromRTCPPacket(RTCPPacketInformation& rtcpPacketIn
     {
         if (rtcpPacketInformation.nackSequenceNumbersLength > 0)
         {
-            WEBRTC_TRACE(kTraceStateInfo, kTraceRtpRtcp, _id, "SIG [RTCP] Incoming NACK to id:%d", _id);
+            WEBRTC_TRACE(kTraceStateInfo, kTraceRtpRtcp, _id,
-            _rtpRtcp.OnReceivedNACK(rtcpPacketInformation.nackSequenceNumbersLength,
+                         "SIG [RTCP] Incoming NACK to id:%d", _id);
-                                          rtcpPacketInformation.nackSequenceNumbers);
+            _rtpRtcp.OnReceivedNACK(
+                rtcpPacketInformation.nackSequenceNumbersLength,
+                rtcpPacketInformation.nackSequenceNumbers);
         }
     }
-    if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpTmmbr)
-    {
-        WEBRTC_TRACE(kTraceStateInfo, kTraceRtpRtcp, _id, "SIG [RTCP] Incoming TMMBR to id:%d", _id);
-
-        // might trigger a OnReceivedBandwidthEstimateUpdate
-        _rtpRtcp.OnReceivedTMMBR();
-    }
     if ((rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpPli) ||
         (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpFir))
     {
         if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpPli)
         {
-            WEBRTC_TRACE(kTraceStateInfo, kTraceRtpRtcp, _id, "SIG [RTCP] Incoming PLI to id:%d", _id);
+            WEBRTC_TRACE(kTraceStateInfo, kTraceRtpRtcp, _id,
-        }else
+                         "SIG [RTCP] Incoming PLI to id:%d", _id);
+        } else
         {
-            WEBRTC_TRACE(kTraceStateInfo, kTraceRtpRtcp, _id, "SIG [RTCP] Incoming FIR to id:%d", _id);
+            WEBRTC_TRACE(kTraceStateInfo, kTraceRtpRtcp, _id,
+                         "SIG [RTCP] Incoming FIR to id:%d", _id);
         }
         _rtpRtcp.OnReceivedIntraFrameRequest(&_rtpRtcp);
     }
@@ -1368,12 +1373,6 @@ RTCPReceiver::TriggerCallbacksFromRTCPPacket(RTCPPacketInformation& rtcpPacketIn
         _rtpRtcp.OnReceivedSliceLossIndication(
             rtcpPacketInformation.sliPictureId);
     }
-    if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpRemb)
-    {
-       // We need to bounce this to the default channel.
-        _rtpRtcp.OnReceivedEstimatedMaxBitrate(
-            rtcpPacketInformation.receiverEstimatedMaxBitrate);
-    }
     if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpRpsi)
     {
          // we need use a bounce it up to handle default channel

src/modules/rtp_rtcp/source/rtp_rtcp_impl.cc

@@ -2719,7 +2719,7 @@ void ModuleRtpRtcpImpl::OnReceivedBandwidthEstimateUpdate(
     // We received a TMMBR
     const bool defaultInstance(_childModules.empty() ? false : true);
     if (defaultInstance) {
-        ProcessDefaultModuleBandwidth(true);
+        ProcessDefaultModuleBandwidth();
         return;
     }
     if (_audio) {
@@ -2760,8 +2760,7 @@ void ModuleRtpRtcpImpl::OnReceivedBandwidthEstimateUpdate(
 void ModuleRtpRtcpImpl::OnPacketLossStatisticsUpdate(
     const WebRtc_UWord8 fractionLost,
     const WebRtc_UWord16 roundTripTime,
-    const WebRtc_UWord32 lastReceivedExtendedHighSeqNum,
+    const WebRtc_UWord32 lastReceivedExtendedHighSeqNum) {
-    bool triggerOnNetworkChanged) {
 
     const bool defaultInstance(_childModules.empty() ? false : true);
     if (!defaultInstance) {
@@ -2797,22 +2796,16 @@ void ModuleRtpRtcpImpl::OnPacketLossStatisticsUpdate(
                 _defaultModule->OnPacketLossStatisticsUpdate(
                         loss,  // send in the filtered loss
                         roundTripTime,
-                        lastReceivedExtendedHighSeqNum,
+                        lastReceivedExtendedHighSeqNum);
-                        triggerOnNetworkChanged);
             }
             return;
         }
-        // No default module check if we should trigger OnNetworkChanged
+        _rtpReceiver.UpdateBandwidthManagement(newBitrate,
-        // via video callback
+                                               fractionLost,
-        if (triggerOnNetworkChanged)
+                                               roundTripTime);
-        {
-              _rtpReceiver.UpdateBandwidthManagement(newBitrate,
-                                                     fractionLost,
-                                                     roundTripTime);
-        }
     } else {
         if (!_simulcast) {
-            ProcessDefaultModuleBandwidth(triggerOnNetworkChanged);
+            ProcessDefaultModuleBandwidth();
         } else {
             // default and simulcast
             WebRtc_UWord32 newBitrate = 0;
@@ -2831,14 +2824,9 @@ void ModuleRtpRtcpImpl::OnPacketLossStatisticsUpdate(
                 return;
             }
             _rtpSender.SetTargetSendBitrate(newBitrate);
-            // check if we should trigger OnNetworkChanged
+            _rtpReceiver.UpdateBandwidthManagement(newBitrate,
-            // via video callback
+                                                   loss,
-            if (triggerOnNetworkChanged)
+                                                   roundTripTime);
-            {
-                _rtpReceiver.UpdateBandwidthManagement(newBitrate,
-                                                       loss,
-                                                       roundTripTime);
-            }
             // sanity
             if (_sendVideoCodec.codecType == kVideoCodecUnknown) {
                 return;
@@ -2875,8 +2863,7 @@ void ModuleRtpRtcpImpl::OnPacketLossStatisticsUpdate(
     }
 }
 
-void ModuleRtpRtcpImpl::ProcessDefaultModuleBandwidth(
+void ModuleRtpRtcpImpl::ProcessDefaultModuleBandwidth() {
-    bool triggerOnNetworkChanged) {
 
     WebRtc_UWord32 minBitrateBps = 0xffffffff;
     WebRtc_UWord32 maxBitrateBps = 0;
@@ -2933,14 +2920,11 @@ void ModuleRtpRtcpImpl::ProcessDefaultModuleBandwidth(
     }
     _bandwidthManagement.SetSendBitrate(minBitrateBps, 0, 0);
 
-    if (triggerOnNetworkChanged) {
+    // Update default module bitrate. Don't care about min max.
-      // Update default module bitrate. Don't care about min max.
+    WebRtc_UWord8 fractionLostAvg = WebRtc_UWord8(fractionLostAcc / count);
-      // Check if we should trigger OnNetworkChanged via video callback
+    _rtpReceiver.UpdateBandwidthManagement(minBitrateBps,
-      WebRtc_UWord8 fractionLostAvg = WebRtc_UWord8(fractionLostAcc / count);
+                                           fractionLostAvg ,
-      _rtpReceiver.UpdateBandwidthManagement(minBitrateBps,
+                                           maxRoundTripTime);
-                                             fractionLostAvg ,
-                                             maxRoundTripTime);
-    }
 }
 
 void ModuleRtpRtcpImpl::OnRequestSendReport() {

src/modules/rtp_rtcp/source/rtp_rtcp_impl.h

@@ -508,8 +508,7 @@ public:
     void OnPacketLossStatisticsUpdate(
         const WebRtc_UWord8 fractionLost,
         const WebRtc_UWord16 roundTripTime,
-        const WebRtc_UWord32 lastReceivedExtendedHighSeqNum,
+        const WebRtc_UWord32 lastReceivedExtendedHighSeqNum);
-        bool triggerOnNetworkChanged);
 
     void OnReceivedTMMBR();
 
@@ -562,7 +561,7 @@ protected:
     RtpRtcpClock&             _clock;
 private:
     void SendKeyFrame();
-    void ProcessDefaultModuleBandwidth(bool triggerOnNetworkChanged);
+    void ProcessDefaultModuleBandwidth();
 
     WebRtc_Word32             _id;
     const bool                _audio;