Clanup handling of key frame requests and FIR.

Review URL: https://webrtc-codereview.appspot.com/387004

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

src/modules/rtp_rtcp/interface/rtp_rtcp.h

@@ -1078,7 +1078,7 @@ public:
     *
     *   return -1 on failure else 0
     */
-    virtual WebRtc_Word32 RequestKeyFrame(const FrameType frameType = kVideoFrameKey) = 0;
+    virtual WebRtc_Word32 RequestKeyFrame() = 0;
 };
 } // namespace webrtc
 #endif // WEBRTC_MODULES_RTP_RTCP_INTERFACE_RTP_RTCP_H_

src/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h

@@ -266,10 +266,8 @@ class MockRtpRtcp : public RtpRtcp {
       WebRtc_Word32(const bool keyUseUepProtection, const bool deltaUseUepProtection));
   MOCK_METHOD1(SetKeyFrameRequestMethod,
       WebRtc_Word32(const KeyFrameRequestMethod method));
-  MOCK_METHOD1(RequestKeyFrame,
+  MOCK_METHOD0(RequestKeyFrame,
-      WebRtc_Word32(const FrameType frameType));
+      WebRtc_Word32());
-  MOCK_METHOD1(SetH263InverseLogic,
-      WebRtc_Word32(const bool enable));
 
   MOCK_CONST_METHOD3(Version,
       int32_t(char* version, uint32_t& remaining_buffer_in_bytes, uint32_t& position));

src/modules/rtp_rtcp/source/rtcp_sender.cc

@@ -61,7 +61,6 @@ RTCPSender::RTCPSender(const WebRtc_Word32 id,
     _includeCSRCs(true),
 
     _sequenceNumberFIR(0),
-    _lastTimeFIR(0),
 
     _lengthRembSSRC(0),
     _sizeRembSSRC(0),
@@ -877,67 +876,46 @@ RTCPSender::BuildPLI(WebRtc_UWord8* rtcpbuffer, WebRtc_UWord32& pos)
     return 0;
 }
 
-WebRtc_Word32
+WebRtc_Word32 RTCPSender::BuildFIR(WebRtc_UWord8* rtcpbuffer,
-RTCPSender::BuildFIR(WebRtc_UWord8* rtcpbuffer, WebRtc_UWord32& pos, const WebRtc_UWord32 RTT)
+                                   WebRtc_UWord32& pos,
-{
+                                   bool repeat) {
-    bool firRepeat = false;
+  // sanity
-    WebRtc_UWord32 diff = _clock.GetTimeInMS() - _lastTimeFIR;
+  if(pos + 20 >= IP_PACKET_SIZE)  {
-    if(diff < RTT + 3) // 3 is processing jitter
+    return -2;
-    {
+  }
-        // we have recently sent a FIR
+  if (!repeat) {
-        // don't send another
+    _sequenceNumberFIR++;   // do not increase if repetition
-        return 0;
+  }
 
-    } else
+  // add full intra request indicator
-    {
+  WebRtc_UWord8 FMT = 4;
-        if(diff < (RTT*2 + RTCP_MIN_FRAME_LENGTH_MS))
+  rtcpbuffer[pos++] = (WebRtc_UWord8)0x80 + FMT;
-        {
+  rtcpbuffer[pos++] = (WebRtc_UWord8)206;
-            // send a FIR_REPEAT instead of a FIR
-            firRepeat = true;
-        }
-    }
-    _lastTimeFIR = _clock.GetTimeInMS();
-    if(!firRepeat)
-    {
-        _sequenceNumberFIR++;   // do not increase if repetition
-    }
 
-    // sanity
+  //Length of 4
-    if(pos + 20 >= IP_PACKET_SIZE)
+  rtcpbuffer[pos++] = (WebRtc_UWord8)0;
-    {
+  rtcpbuffer[pos++] = (WebRtc_UWord8)(4);
-        return -2;
-    }
 
-    // add full intra request indicator
+  // Add our own SSRC
-    WebRtc_UWord8 FMT = 4;
+  ModuleRTPUtility::AssignUWord32ToBuffer(rtcpbuffer + pos, _SSRC);
-    rtcpbuffer[pos++]=(WebRtc_UWord8)0x80 + FMT;
+  pos += 4;
-    rtcpbuffer[pos++]=(WebRtc_UWord8)206;
 
-    //Length of 4
+  // RFC 5104     4.3.1.2.  Semantics
-    rtcpbuffer[pos++]=(WebRtc_UWord8)0;
+  // SSRC of media source
-    rtcpbuffer[pos++]=(WebRtc_UWord8)(4);
+  rtcpbuffer[pos++] = (WebRtc_UWord8)0;
+  rtcpbuffer[pos++] = (WebRtc_UWord8)0;
+  rtcpbuffer[pos++] = (WebRtc_UWord8)0;
+  rtcpbuffer[pos++] = (WebRtc_UWord8)0;
 
-    // Add our own SSRC
+  // Additional Feedback Control Information (FCI)
-    ModuleRTPUtility::AssignUWord32ToBuffer(rtcpbuffer+pos, _SSRC);
+  ModuleRTPUtility::AssignUWord32ToBuffer(rtcpbuffer + pos, _remoteSSRC);
-    pos += 4;
+  pos += 4;
 
-    // RFC 5104     4.3.1.2.  Semantics
+  rtcpbuffer[pos++] = (WebRtc_UWord8)(_sequenceNumberFIR);
-
+  rtcpbuffer[pos++] = (WebRtc_UWord8)0;
-    // SSRC of media source
+  rtcpbuffer[pos++] = (WebRtc_UWord8)0;
-    rtcpbuffer[pos++]=(WebRtc_UWord8)0;
+  rtcpbuffer[pos++] = (WebRtc_UWord8)0;
-    rtcpbuffer[pos++]=(WebRtc_UWord8)0;
+  return 0;
-    rtcpbuffer[pos++]=(WebRtc_UWord8)0;
-    rtcpbuffer[pos++]=(WebRtc_UWord8)0;
-
-    // Additional Feedback Control Information (FCI)
-    ModuleRTPUtility::AssignUWord32ToBuffer(rtcpbuffer+pos, _remoteSSRC);
-    pos += 4;
-
-    rtcpbuffer[pos++]=(WebRtc_UWord8)(_sequenceNumberFIR);
-    rtcpbuffer[pos++]=(WebRtc_UWord8)0;
-    rtcpbuffer[pos++]=(WebRtc_UWord8)0;
-    rtcpbuffer[pos++]=(WebRtc_UWord8)0;
-    return 0;
 }
 
 /*
@@ -1589,7 +1567,7 @@ WebRtc_Word32
 RTCPSender::SendRTCP(const WebRtc_UWord32 packetTypeFlags,
                      const WebRtc_Word32 nackSize,       // NACK
                      const WebRtc_UWord16* nackList,     // NACK
-                     const WebRtc_UWord32 RTT,           // FIR
+                     const bool repeat,                  // FIR
                      const WebRtc_UWord64 pictureID)     // SLI & RPSI
 {
     WebRtc_UWord32 rtcpPacketTypeFlags = packetTypeFlags;
@@ -1848,7 +1826,7 @@ RTCPSender::SendRTCP(const WebRtc_UWord32 packetTypeFlags,
         }
         if(rtcpPacketTypeFlags & kRtcpFir)
         {
-            buildVal = BuildFIR(rtcpbuffer, pos, RTT);
+            buildVal = BuildFIR(rtcpbuffer, pos, repeat);
             if(buildVal == -1)
             {
                 return -1; // error

src/modules/rtp_rtcp/source/rtcp_sender.h

@@ -68,7 +68,7 @@ public:
     WebRtc_Word32 SendRTCP(const WebRtc_UWord32 rtcpPacketTypeFlags,
                            const WebRtc_Word32 nackSize = 0,
                            const WebRtc_UWord16* nackList = 0,
-                           const WebRtc_UWord32 RTT = 0,
+                           const bool repeat = false,
                            const WebRtc_UWord64 pictureID = 0);
 
     WebRtc_Word32 AddReportBlock(const WebRtc_UWord32 SSRC,
@@ -178,8 +178,8 @@ private:
     WebRtc_Word32 BuildVoIPMetric(WebRtc_UWord8* rtcpbuffer, WebRtc_UWord32& pos);
     WebRtc_Word32 BuildBYE(WebRtc_UWord8* rtcpbuffer, WebRtc_UWord32& pos);
     WebRtc_Word32 BuildFIR(WebRtc_UWord8* rtcpbuffer,
-                         WebRtc_UWord32& pos,
+                           WebRtc_UWord32& pos,
-                         const WebRtc_UWord32 RTT);
+                           bool repeat);
     WebRtc_Word32 BuildSLI(WebRtc_UWord8* rtcpbuffer,
                          WebRtc_UWord32& pos,
                          const WebRtc_UWord8 pictureID);
@@ -235,7 +235,6 @@ private:
 
     // Full intra request
     WebRtc_UWord8         _sequenceNumberFIR;
-    WebRtc_UWord32        _lastTimeFIR;
 
     // REMB    
     WebRtc_UWord8       _lengthRembSSRC;

src/modules/rtp_rtcp/source/rtp_rtcp_impl.cc

@@ -387,7 +387,7 @@ WebRtc_Word32 ModuleRtpRtcpImpl::Process() {
     } else if (TMMBR()) {
       _rtcpSender.CalculateNewTargetBitrate(RTT);
     }
-    _rtcpSender.SendRTCP(kRtcpReport, 0, 0, RTT);
+    _rtcpSender.SendRTCP(kRtcpReport);
   }
 
   if (_rtpSender.RTPKeepalive()) {
@@ -1167,9 +1167,7 @@ WebRtc_Word32 ModuleRtpRtcpImpl::SendOutgoingData(
   if (!haveChildModules) {
     // Don't sent RTCP from default module
     if (_rtcpSender.TimeToSendRTCPReport(kVideoFrameKey == frameType)) {
-      WebRtc_UWord16 RTT = 0;
+      _rtcpSender.SendRTCP(kRtcpReport);
-      _rtcpReceiver.RTT(_rtpReceiver.SSRC(), &RTT, NULL, NULL, NULL);
-      _rtcpSender.SendRTCP(kRtcpReport, 0, 0, RTT);
     }
     return _rtpSender.SendOutgoingData(frameType,
                                        payloadType,
@@ -1800,7 +1798,7 @@ WebRtc_Word32 ModuleRtpRtcpImpl::SendNACK(const WebRtc_UWord16* nackList,
                "SendNACK(size:%u)", size);
 
   if (size > NACK_PACKETS_MAX_SIZE) {
-    RequestKeyFrame(kVideoFrameKey);
+    RequestKeyFrame();
     return -1;
   }
   WebRtc_UWord16 avgRTT = 0;
@@ -2040,28 +2038,20 @@ WebRtc_Word32 ModuleRtpRtcpImpl::SetKeyFrameRequestMethod(
   return 0;
 }
 
-WebRtc_Word32 ModuleRtpRtcpImpl::RequestKeyFrame(const FrameType frameType) {
+WebRtc_Word32 ModuleRtpRtcpImpl::RequestKeyFrame() {
   WEBRTC_TRACE(kTraceModuleCall,
                kTraceRtpRtcp,
                _id,
-               "RequestKeyFrame(frameType:%d)",
+               "RequestKeyFrame");
-               frameType);
 
   switch (_keyFrameReqMethod) {
     case kKeyFrameReqFirRtp:
       return _rtpSender.SendRTPIntraRequest();
-
     case kKeyFrameReqPliRtcp:
       return _rtcpSender.SendRTCP(kRtcpPli);
-
+    case kKeyFrameReqFirRtcp:
-    case kKeyFrameReqFirRtcp: {
+      return _rtcpSender.SendRTCP(kRtcpFir);
-      // conference scenario
-      WebRtc_UWord16 RTT = 0;
-      _rtcpReceiver.RTT(_rtpReceiver.SSRC(), &RTT, NULL, NULL, NULL);
-      return _rtcpSender.SendRTCP(kRtcpFir, 0, NULL, RTT);
-    }
   }
-  assert(false);
   return -1;
 }
 
@@ -2072,7 +2062,7 @@ WebRtc_Word32 ModuleRtpRtcpImpl::SendRTCPSliceLossIndication(
                _id,
                "SendRTCPSliceLossIndication (pictureID:%d)",
                pictureID);
-  return _rtcpSender.SendRTCP(kRtcpSli, 0, 0, 0, pictureID);
+  return _rtcpSender.SendRTCP(kRtcpSli, 0, 0, false, pictureID);
 }
 
 WebRtc_Word32 ModuleRtpRtcpImpl::SetCameraDelay(const WebRtc_Word32 delayMS) {
@@ -2432,8 +2422,8 @@ RateControlRegion ModuleRtpRtcpImpl::OnOverUseStateUpdate(
 }
 
 // bad state of RTP receiver request a keyframe
-void ModuleRtpRtcpImpl::OnRequestIntraFrame(const FrameType frameType) {
+void ModuleRtpRtcpImpl::OnRequestIntraFrame() {
-  RequestKeyFrame(frameType);
+  RequestKeyFrame();
 }
 
 void ModuleRtpRtcpImpl::OnReceivedIntraFrameRequest(const RtpRtcp* caller) {
@@ -2794,7 +2784,7 @@ void ModuleRtpRtcpImpl::OnRequestSendReport() {
 
 WebRtc_Word32 ModuleRtpRtcpImpl::SendRTCPReferencePictureSelection(
   const WebRtc_UWord64 pictureID) {
-  return _rtcpSender.SendRTCP(kRtcpRpsi, 0, 0, 0, pictureID);
+  return _rtcpSender.SendRTCP(kRtcpRpsi, 0, 0, false, pictureID);
 }
 
 WebRtc_UWord32 ModuleRtpRtcpImpl::SendTimeOfSendReport(

src/modules/rtp_rtcp/source/rtp_rtcp_impl.h

@@ -456,7 +456,7 @@ public:
     virtual WebRtc_Word32 SetKeyFrameRequestMethod(const KeyFrameRequestMethod method);
 
     // send a request for a keyframe
-    virtual WebRtc_Word32 RequestKeyFrame(const FrameType frameType);
+    virtual WebRtc_Word32 RequestKeyFrame();
 
     virtual WebRtc_Word32 SetCameraDelay(const WebRtc_Word32 delayMS);
 
@@ -519,7 +519,7 @@ public:
     void OnReceivedBandwidthEstimateUpdate(const WebRtc_UWord16 bwEstimateKbit);
 
     // bad state of RTP receiver request a keyframe
-    void OnRequestIntraFrame(const FrameType frameType);
+    void OnRequestIntraFrame();
 
     void OnReceivedIntraFrameRequest(const RtpRtcp* caller);
 

src/modules/video_coding/main/interface/mock/mock_vcm_callbacks.h

@@ -1,5 +1,5 @@
 /*
- *  Copyright (c) 2011 The WebRTC project authors. All Rights Reserved.
+ *  Copyright (c) 2012 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
@@ -18,7 +18,7 @@ namespace webrtc {
 
 class MockVCMFrameTypeCallback : public VCMFrameTypeCallback {
  public:
-  MOCK_METHOD1(FrameTypeRequest, int32_t(const FrameType frameType));
+  MOCK_METHOD0(RequestKeyFrame, int32_t());
   MOCK_METHOD1(SliceLossIndicationRequest,
                WebRtc_Word32(const WebRtc_UWord64 pictureId));
 };

src/modules/video_coding/main/interface/video_coding_defines.h

@@ -1,5 +1,5 @@
 /*
- *  Copyright (c) 2011 The WebRTC project authors. All Rights Reserved.
+ *  Copyright (c) 2012 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
@@ -158,7 +158,7 @@ protected:
 class VCMFrameTypeCallback
 {
 public:
-    virtual WebRtc_Word32 FrameTypeRequest(const FrameType frameType) = 0;
+    virtual WebRtc_Word32 RequestKeyFrame() = 0;
     virtual WebRtc_Word32 SliceLossIndicationRequest(const WebRtc_UWord64 pictureId) {return -1;}
 
 protected:

src/modules/video_coding/main/source/video_coding_impl.cc

@@ -1125,8 +1125,7 @@ VideoCodingModuleImpl::RequestKeyFrame()
 {
     if (_frameTypeCallback != NULL)
     {
-        const WebRtc_Word32 ret = _frameTypeCallback->FrameTypeRequest(
+        const WebRtc_Word32 ret = _frameTypeCallback->RequestKeyFrame();
-                                                                kVideoFrameKey);
         if (ret < 0)
         {
             WEBRTC_TRACE(webrtc::kTraceError,

src/modules/video_coding/main/source/video_coding_robustness_unittest.cc

@@ -1,5 +1,5 @@
 /*
- *  Copyright (c) 2011 The WebRTC project authors. All Rights Reserved.
+ *  Copyright (c) 2012 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
@@ -135,7 +135,7 @@ TEST_F(VCMRobustnessTest, TestHardNack) {
 TEST_F(VCMRobustnessTest, TestHardNackNoneDecoded) {
   EXPECT_CALL(request_callback_, ResendPackets(_, _))
       .Times(0);
-  EXPECT_CALL(frame_type_callback_, FrameTypeRequest(kVideoFrameKey))
+  EXPECT_CALL(frame_type_callback_, RequestKeyFrame())
         .Times(1);
 
   ASSERT_EQ(VCM_OK, vcm_->SetReceiverRobustnessMode(
@@ -358,7 +358,7 @@ TEST_F(VCMRobustnessTest, TestModeNoneWithoutErrors) {
                                false, _, _, _))
         .Times(1)
         .InSequence(s1);
-  EXPECT_CALL(frame_type_callback_, FrameTypeRequest(kVideoFrameKey))
+  EXPECT_CALL(frame_type_callback_, RequestKeyFrame())
         .Times(1);
 
   ASSERT_EQ(VCM_OK, vcm_->SetReceiverRobustnessMode(

src/modules/video_coding/main/test/test_callbacks.cc

@@ -405,19 +405,9 @@ SendStatsTest::SendStatistics(const WebRtc_UWord32 bitRate,
     return 0;
 }
 
-WebRtc_Word32
+WebRtc_Word32 KeyFrameReqTest::RequestKeyFrame() {
-KeyFrameReqTest::FrameTypeRequest(const FrameType frameType)
+  printf("Key frame requested\n");
-{
+  return 0;
-    TEST(frameType == kVideoFrameKey);
-    if (frameType == kVideoFrameKey)
-    {
-        printf("Key frame requested\n");
-    }
-    else
-    {
-        printf("Non-key frame requested: %d\n", frameType);
-    }
-    return 0;
 }
 
 

src/modules/video_coding/main/test/test_callbacks.h

@@ -211,7 +211,7 @@ private:
 class KeyFrameReqTest: public VCMFrameTypeCallback
 {
 public:
-    WebRtc_Word32 FrameTypeRequest(const FrameType frameType);
+    WebRtc_Word32 RequestKeyFrame();
 };
 
 

src/video_engine/vie_channel.cc

@@ -1,5 +1,5 @@
 /*
- *  Copyright (c) 2011 The WebRTC project authors. All Rights Reserved.
+ *  Copyright (c) 2012 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
@@ -2175,16 +2175,16 @@ WebRtc_Word32 ViEChannel::ReceiveStatistics(const WebRtc_UWord32 bit_rate,
   return 0;
 }
 
-WebRtc_Word32 ViEChannel::FrameTypeRequest(const FrameType frame_type) {
+WebRtc_Word32 ViEChannel::RequestKeyFrame() {
   WEBRTC_TRACE(kTraceStream, kTraceVideo, ViEId(engine_id_, channel_id_),
-               "%s(frame_type: %d)", __FUNCTION__, frame_type);
+               "%s", __FUNCTION__);
   {
     CriticalSectionScoped cs(callback_cs_.get());
     if (codec_observer_ && do_key_frame_callbackRequest_) {
       codec_observer_->RequestNewKeyFrame(channel_id_);
     }
   }
-  return rtp_rtcp_.RequestKeyFrame(frame_type);
+  return rtp_rtcp_.RequestKeyFrame();
 }
 
 WebRtc_Word32 ViEChannel::SliceLossIndicationRequest(

src/video_engine/vie_channel.h

@@ -1,5 +1,5 @@
 /*
- *  Copyright (c) 2011 The WebRTC project authors. All Rights Reserved.
+ *  Copyright (c) 2012 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
@@ -312,7 +312,7 @@ class ViEChannel
                                           const WebRtc_UWord32 frame_rate);
 
   // Implements VideoFrameTypeCallback.
-  virtual WebRtc_Word32 FrameTypeRequest(const FrameType frame_type);
+  virtual WebRtc_Word32 RequestKeyFrame();
 
   // Implements VideoFrameTypeCallback.
   virtual WebRtc_Word32 SliceLossIndicationRequest(