Bug 880067 - Part 4: Video Conduit configuration for RTCP feedback r=ekr
☠☠ backed out by 4fdd3cddee3a ☠ ☠
authorAdam Roach [:abr] <adam@nostrum.com>
Thu, 05 Sep 2013 15:11:47 -0500
changeset 146456 bdcd192bda52c7fceab99be7fe86eb63caabecc3
parent 146455 5a2ab286a97723a5000d45a1453243a746c78cad
child 146457 abf37dafb08ca9f89fb71d3ec9d281d2ba492265
push id33559
push useradam@nostrum.com
push dateTue, 10 Sep 2013 23:00:44 +0000
treeherdermozilla-inbound@bdcd192bda52 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersekr
bugs880067
milestone26.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 880067 - Part 4: Video Conduit configuration for RTCP feedback r=ekr
media/webrtc/signaling/signaling.gyp
media/webrtc/signaling/src/media-conduit/CodecConfig.h
media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp
media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c
media/webrtc/signaling/src/sipcc/core/sdp/sdp.h
media/webrtc/signaling/src/sipcc/include/ccsdp.h
media/webrtc/signaling/src/sipcc/include/ccsdp_rtcp_fb.h
media/webrtc/signaling/test/mediaconduit_unittests.cpp
--- a/media/webrtc/signaling/signaling.gyp
+++ b/media/webrtc/signaling/signaling.gyp
@@ -582,16 +582,17 @@
         './src/sipcc/include/ccapi_device_listener.h',
         './src/sipcc/include/ccapi_feature_info.h',
         './src/sipcc/include/ccapi_line.h',
         './src/sipcc/include/ccapi_line_info.h',
         './src/sipcc/include/ccapi_line_listener.h',
         './src/sipcc/include/ccapi_service.h',
         './src/sipcc/include/ccapi_types.h',
         './src/sipcc/include/ccsdp.h',
+        './src/sipcc/include/ccsdp_rtcp_fb.h',
         './src/sipcc/include/config_api.h',
         './src/sipcc/include/dns_util.h',
         './src/sipcc/include/plat_api.h',
         './src/sipcc/include/reset_api.h',
         './src/sipcc/include/sll_lite.h',
         './src/sipcc/include/vcm.h',
         './src/sipcc/include/xml_parser_defines.h',
 
--- a/media/webrtc/signaling/src/media-conduit/CodecConfig.h
+++ b/media/webrtc/signaling/src/media-conduit/CodecConfig.h
@@ -2,17 +2,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef CODEC_CONFIG_H_
 #define CODEC_CONFIG_H_
 
 #include <string>
-
+#include "ccsdp_rtcp_fb.h"
 
 namespace mozilla {
 
 /**
  * Minimalistic Audio Codec Config Params
  */
 struct AudioCodecConfig
 {
@@ -51,21 +51,34 @@ struct AudioCodecConfig
 struct VideoCodecConfig
 {
   /*
    * The data-types for these properties mimic the
    * corresponding webrtc::VideoCodec data-types.
    */
   int mType;
   std::string mName;
+  uint32_t mRtcpFbTypes;
 
-  /* When we have resolution negotiation information (RFC 6236)
-   * it will be stored here.
-   */
+  VideoCodecConfig(int type, std::string name, int rtcpFbTypes):
+    mType(type), mName(name), mRtcpFbTypes(rtcpFbTypes)
+  {
+  }
+
 
-  VideoCodecConfig(int type, std::string name): mType(type),
-                                                mName(name)
+  bool RtcpFbIsSet(sdp_rtcp_fb_nack_type_e type) const
+  {
+    return mRtcpFbTypes & sdp_rtcp_fb_nack_to_bitmap(type);
+  }
+
+  bool RtcpFbIsSet(sdp_rtcp_fb_ack_type_e type) const
   {
+    return mRtcpFbTypes & sdp_rtcp_fb_ack_to_bitmap(type);
+  }
+
+  bool RtcpFbIsSet(sdp_rtcp_fb_ccm_type_e type) const
+  {
+    return mRtcpFbTypes & sdp_rtcp_fb_ccm_to_bitmap(type);
   }
 
 };
 }
 #endif
--- a/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ -1,15 +1,18 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "CSFLog.h"
 #include "nspr.h"
 
+// For rtcp-fb constants
+#include "ccsdp.h"
+
 #include "VideoConduit.h"
 #include "AudioConduit.h"
 #include "webrtc/video_engine/include/vie_errors.h"
 
 #ifdef MOZ_WIDGET_ANDROID
 #include "AndroidJNIWrapper.h"
 #endif
 
@@ -240,32 +243,16 @@ MediaConduitErrorCode WebrtcVideoConduit
   }
   // Turn on RTCP and loss feedback reporting.
   if(mPtrRTP->SetRTCPStatus(mChannel, webrtc::kRtcpCompound_RFC4585) != 0)
   {
     CSFLogError(logTag,  "%s RTCPStatus Failed %d ", __FUNCTION__,
                 mPtrViEBase->LastError());
     return kMediaConduitRTCPStatusError;
   }
-  // Enable pli as key frame request method.
-  if(mPtrRTP->SetKeyFrameRequestMethod(mChannel,
-                                    webrtc::kViEKeyFrameRequestPliRtcp) != 0)
-  {
-    CSFLogError(logTag,  "%s KeyFrameRequest Failed %d ", __FUNCTION__,
-                mPtrViEBase->LastError());
-    return kMediaConduitKeyFrameRequestError;
-  }
-  // Enable lossless transport
-  // XXX Note: We may want to disable this or limit it
-  if (mPtrRTP->SetNACKStatus(mChannel, true) != 0)
-  {
-    CSFLogError(logTag,  "%s NACKStatus Failed %d ", __FUNCTION__,
-                mPtrViEBase->LastError());
-    return kMediaConduitNACKStatusError;
-  }
   CSFLogError(logTag, "%s Initialization Done", __FUNCTION__);
   return kMediaConduitNoError;
 }
 
 void
 WebrtcVideoConduit::SyncTo(WebrtcAudioConduit *aConduit)
 {
   CSFLogDebug(logTag, "%s Synced to %p", __FUNCTION__, aConduit);
@@ -413,28 +400,37 @@ WebrtcVideoConduit::ConfigureSendMediaCo
     }
     CSFLogError(logTag, "%s SetSendCodec Failed %d ", __FUNCTION__,
                 mPtrViEBase->LastError());
     return kMediaConduitUnknownError;
   }
   mSendingWidth = 0;
   mSendingHeight = 0;
 
+  if(codecConfig->RtcpFbIsSet(SDP_RTCP_FB_NACK_BASIC)) {
+    CSFLogDebug(logTag, "Enabling NACK (send) for video stream\n");
+    if (mPtrRTP->SetNACKStatus(mChannel, true) != 0)
+    {
+      CSFLogError(logTag,  "%s NACKStatus Failed %d ", __FUNCTION__,
+                  mPtrViEBase->LastError());
+      return kMediaConduitNACKStatusError;
+    }
+  }
+
   if(mPtrViEBase->StartSend(mChannel) == -1)
   {
     CSFLogError(logTag, "%s Start Send Error %d ", __FUNCTION__,
                 mPtrViEBase->LastError());
     return kMediaConduitUnknownError;
   }
 
   //Copy the applied codec for future reference
   delete mCurSendCodecConfig;
 
-  mCurSendCodecConfig = new VideoCodecConfig(codecConfig->mType,
-                                             codecConfig->mName);
+  mCurSendCodecConfig = new VideoCodecConfig(*codecConfig);
 
   mPtrRTP->SetRembStatus(mChannel, true, false);
 
   // by now we should be successfully started the transmission
   mEngineTransmitting = true;
   return kMediaConduitNoError;
 }
 
@@ -467,28 +463,49 @@ WebrtcVideoConduit::ConfigureRecvMediaCo
   }
 
   if(codecConfigList.empty())
   {
     CSFLogError(logTag, "%s Zero number of codecs to configure", __FUNCTION__);
     return kMediaConduitMalformedArgument;
   }
 
-  //Try Applying the codecs in the list
+  webrtc::ViEKeyFrameRequestMethod kf_request = webrtc::kViEKeyFrameRequestNone;
+  bool use_nack_basic = false;
+
+  // Try Applying the codecs in the list
   // we treat as success if atleast one codec was applied and reception was
   // started successfully.
   for(std::vector<VideoCodecConfig*>::size_type i=0;i < codecConfigList.size();i++)
   {
     //if the codec param is invalid or diplicate, return error
     if((condError = ValidateCodecConfig(codecConfigList[i],false)) != kMediaConduitNoError)
     {
       return condError;
     }
 
+    // Check for the keyframe request type: PLI is preferred
+    // over FIR, and FIR is preferred over none.
+    if (codecConfigList[i]->RtcpFbIsSet(SDP_RTCP_FB_NACK_PLI))
+    {
+      kf_request = webrtc::kViEKeyFrameRequestPliRtcp;
+    } else if(kf_request == webrtc::kViEKeyFrameRequestNone &&
+              codecConfigList[i]->RtcpFbIsSet(SDP_RTCP_FB_CCM_FIR))
+    {
+      kf_request = webrtc::kViEKeyFrameRequestFirRtcp;
+    }
+
+    // Check whether NACK is requested
+    if(codecConfigList[i]->RtcpFbIsSet(SDP_RTCP_FB_NACK_BASIC))
+    {
+      use_nack_basic = true;
+    }
+
     webrtc::VideoCodec  video_codec;
+
     mEngineReceiving = false;
     memset(&video_codec, 0, sizeof(webrtc::VideoCodec));
     //Retrieve pre-populated codec structure for our codec.
     for(int idx=0; idx < mPtrViECodec->NumberOfCodecs(); idx++)
     {
       if(mPtrViECodec->GetCodec(idx, video_codec) == 0)
       {
         payloadName = video_codec.plName;
@@ -518,16 +535,61 @@ WebrtcVideoConduit::ConfigureRecvMediaCo
   }//end for
 
   if(!success)
   {
     CSFLogError(logTag, "%s Setting Receive Codec Failed ", __FUNCTION__);
     return kMediaConduitInvalidReceiveCodec;
   }
 
+  // XXX Currently, we gather up all of the feedback types that the remote
+  // party indicated it supports for all video codecs and configure the entire
+  // conduit based on those capabilities. This is technically out of spec,
+  // as these values should be configured on a per-codec basis. However,
+  // the video engine only provides this API on a per-conduit basis, so that's
+  // how we have to do it. The approach of considering the remote capablities
+  // for the entire conduit to be a union of all remote codec capabilities
+  // (rather than the more conservative approach of using an intersection)
+  // is made to provide as many feedback mechanisms as are likely to be
+  // processed by the remote party (and should be relatively safe, since the
+  // remote party is required to ignore feedback types that it does not
+  // understand).
+  //
+  // Note that our configuration uses this union of remote capabilites as
+  // input to the configuration. It is not isomorphic to the configuration.
+  // For example, it only makes sense to have one frame request mechanism
+  // active at a time; so, if the remote party indicates more than one
+  // supported mechanism, we're only configuring the one we most prefer.
+  //
+  // See http://code.google.com/p/webrtc/issues/detail?id=2331
+
+  if (kf_request != webrtc::kViEKeyFrameRequestNone)
+  {
+    CSFLogDebug(logTag, "Enabling %s frame requests for video stream\n",
+                (kf_request == webrtc::kViEKeyFrameRequestPliRtcp ?
+                 "PLI" : "FIR"));
+    if(mPtrRTP->SetKeyFrameRequestMethod(mChannel, kf_request) != 0)
+    {
+      CSFLogError(logTag,  "%s KeyFrameRequest Failed %d ", __FUNCTION__,
+                  mPtrViEBase->LastError());
+      return kMediaConduitKeyFrameRequestError;
+    }
+  }
+
+  if(use_nack_basic)
+  {
+    CSFLogDebug(logTag, "Enabling NACK (recv) for video stream\n");
+    if (mPtrRTP->SetNACKStatus(mChannel, true) != 0)
+    {
+      CSFLogError(logTag,  "%s NACKStatus Failed %d ", __FUNCTION__,
+                  mPtrViEBase->LastError());
+      return kMediaConduitNACKStatusError;
+    }
+  }
+
   //Start Receive on the video engine
   if(mPtrViEBase->StartReceive(mChannel) == -1)
   {
     error = mPtrViEBase->LastError();
     CSFLogError(logTag, "%s Start Receive Error %d ", __FUNCTION__, error);
 
     return kMediaConduitUnknownError;
   }
@@ -784,18 +846,17 @@ WebrtcVideoConduit::CodecConfigToWebRTCC
   cinst.minBitrate = 200;
   cinst.startBitrate = 300;
   cinst.maxBitrate = 2000;
 }
 
 bool
 WebrtcVideoConduit::CopyCodecToDB(const VideoCodecConfig* codecInfo)
 {
-  VideoCodecConfig* cdcConfig = new VideoCodecConfig(codecInfo->mType,
-                                                     codecInfo->mName);
+  VideoCodecConfig* cdcConfig = new VideoCodecConfig(*codecInfo);
   mRecvCodecList.push_back(cdcConfig);
   return true;
 }
 
 /**
  * Checks if the codec is already in Conduit's database
  */
 
--- a/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp
+++ b/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp
@@ -1481,17 +1481,18 @@ static int vcmRxStartICE_m(cc_mcapid_t m
       return VCM_ERROR;
 
     mozilla::VideoCodecConfig *config_raw;
 
     for(int i=0; i <num_payloads; i++)
     {
       config_raw = new mozilla::VideoCodecConfig(
         payloads[i].remote_rtp_pt,
-        ccsdpCodecName(payloads[i].codec_type));
+        ccsdpCodecName(payloads[i].codec_type),
+        payloads[i].video.rtcp_fb_types);
       configs.push_back(config_raw);
     }
 
     if (conduit->ConfigureRecvMediaCodecs(configs))
       return VCM_ERROR;
 
     // Now we have all the pieces, create the pipeline
     mozilla::RefPtr<mozilla::MediaPipeline> pipeline =
@@ -2112,17 +2113,18 @@ static int vcmTxStartICE_m(cc_mcapid_t m
 
     // Now we have all the pieces, create the pipeline
     stream->StorePipeline(pc_track_id, pipeline);
 
   } else if (CC_IS_VIDEO(mcap_id)) {
     mozilla::VideoCodecConfig *config_raw;
     config_raw = new mozilla::VideoCodecConfig(
       payload->remote_rtp_pt,
-      ccsdpCodecName(payload->codec_type));
+      ccsdpCodecName(payload->codec_type),
+      payload->video.rtcp_fb_types);
 
     // Take possession of this pointer
     mozilla::ScopedDeletePtr<mozilla::VideoCodecConfig> config(config_raw);
 
     // Instantiate an appropriate conduit
     mozilla::RefPtr<mozilla::VideoSessionConduit> conduit =
       mozilla::VideoSessionConduit::Create();
 
--- a/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c
+++ b/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c
@@ -4502,31 +4502,31 @@ gsmsdp_add_rtcp_fb (int level, sdp_t *sd
     for (pt_index = 1; pt_index <= num_pts; pt_index++) {
         pt_codec = sdp_get_media_payload_type (sdp_p, level, pt_index,
                                                &indicator);
         if ((pt_codec & 0xFF) == codec) {
             int pt = GET_DYN_PAYLOAD_TYPE_VALUE(pt_codec);
 
             /* Add requested a=rtcp-fb:nack attributes */
             for (j = 0; j < SDP_MAX_RTCP_FB_NACK; j++) {
-                if (types & SDP_RTCP_FB_NACK_TO_BITMAP(j)) {
+                if (types & sdp_rtcp_fb_nack_to_bitmap(j)) {
                     gsmsdp_set_rtcp_fb_nack_attribute(level, sdp_p, pt, j);
                 }
             }
 
             /* Add requested a=rtcp-fb:ack attributes */
             for (j = 0; j < SDP_MAX_RTCP_FB_ACK; j++) {
-                if (types & SDP_RTCP_FB_ACK_TO_BITMAP(j)) {
+                if (types & sdp_rtcp_fb_ack_to_bitmap(j)) {
                     gsmsdp_set_rtcp_fb_nack_attribute(level, sdp_p, pt, j);
                 }
             }
 
             /* Add requested a=rtcp-fb:ccm attributes */
             for (j = 0; j < SDP_MAX_RTCP_FB_CCM; j++) {
-                if (types & SDP_RTCP_FB_CCM_TO_BITMAP(j)) {
+                if (types & sdp_rtcp_fb_ccm_to_bitmap(j)) {
                     gsmsdp_set_rtcp_fb_ccm_attribute(level, sdp_p, pt, j);
                 }
             }
 
         }
     }
     return CC_CAUSE_OK;
 }
@@ -4586,52 +4586,52 @@ gsmsdp_negotiate_rtcp_fb (cc_sdp_t *cc_s
         fb_types = 0;
 
         /* a=rtcp-fb:nack */
         i = 1;
         do {
             nack_type = sdp_attr_get_rtcp_fb_nack(cc_sdp_p->dest_sdp,
                                                   level, remote_pt, i);
             if (nack_type >= 0 && nack_type < SDP_MAX_RTCP_FB_NACK) {
-                fb_types |= SDP_RTCP_FB_NACK_TO_BITMAP(nack_type);
+                fb_types |= sdp_rtcp_fb_nack_to_bitmap(nack_type);
             }
             i++;
         } while (nack_type != SDP_RTCP_FB_NACK_NOT_FOUND);
 
         /* a=rtcp-fb:ack */
         i = 1;
         do {
             ack_type = sdp_attr_get_rtcp_fb_ack(cc_sdp_p->dest_sdp,
                                                 level, remote_pt, i);
             if (ack_type >= 0 && ack_type < SDP_MAX_RTCP_FB_ACK) {
-                fb_types |= SDP_RTCP_FB_ACK_TO_BITMAP(ack_type);
+                fb_types |= sdp_rtcp_fb_ack_to_bitmap(ack_type);
             }
             i++;
         } while (ack_type != SDP_RTCP_FB_ACK_NOT_FOUND);
 
         /* a=rtcp-fb:ccm */
         i = 1;
         do {
             ccm_type = sdp_attr_get_rtcp_fb_ccm(cc_sdp_p->dest_sdp,
                                                 level, remote_pt, i);
             if (ccm_type >= 0 && ccm_type < SDP_MAX_RTCP_FB_CCM) {
-                fb_types |= SDP_RTCP_FB_CCM_TO_BITMAP(ccm_type);
+                fb_types |= sdp_rtcp_fb_ccm_to_bitmap(ccm_type);
             }
             i++;
         } while (ccm_type != SDP_RTCP_FB_CCM_NOT_FOUND);
 
         /*
          * Mask out the types that we do not support
          */
         switch (codec) {
             case RTP_VP8:
                 fb_types &=
-                  SDP_RTCP_FB_NACK_TO_BITMAP(SDP_RTCP_FB_NACK_BASIC) |
-                  SDP_RTCP_FB_NACK_TO_BITMAP(SDP_RTCP_FB_NACK_PLI) |
-                  SDP_RTCP_FB_CCM_TO_BITMAP(SDP_RTCP_FB_CCM_FIR);
+                  sdp_rtcp_fb_nack_to_bitmap(SDP_RTCP_FB_NACK_BASIC) |
+                  sdp_rtcp_fb_nack_to_bitmap(SDP_RTCP_FB_NACK_PLI) |
+                  sdp_rtcp_fb_ccm_to_bitmap(SDP_RTCP_FB_CCM_FIR);
                 break;
             default:
                 fb_types = 0;
         }
 
         /*
          * Now, in our local SDP, set rtcp-fb types that both we and the
          * remote party support
@@ -5549,19 +5549,19 @@ gsmsdp_add_media_line (fsmdef_dcb_t *dcb
 
         if (media->support_direction != SDP_DIRECTION_INACTIVE) {
 
           gsmsdp_set_local_sdp_direction(dcb_p, media, media->direction);
 
           /* Add supported rtcp-fb types */
           if (media_cap->type == SDP_MEDIA_VIDEO) {
               gsmsdp_add_rtcp_fb (level, dcb_p->sdp->src_sdp, RTP_VP8,
-                  SDP_RTCP_FB_NACK_TO_BITMAP(SDP_RTCP_FB_NACK_BASIC) |
-                  SDP_RTCP_FB_NACK_TO_BITMAP(SDP_RTCP_FB_NACK_PLI) |
-                  SDP_RTCP_FB_CCM_TO_BITMAP(SDP_RTCP_FB_CCM_FIR));
+                  sdp_rtcp_fb_nack_to_bitmap(SDP_RTCP_FB_NACK_BASIC) |
+                  sdp_rtcp_fb_nack_to_bitmap(SDP_RTCP_FB_NACK_PLI) |
+                  sdp_rtcp_fb_ccm_to_bitmap(SDP_RTCP_FB_CCM_FIR));
           }
 
           /* setup and connection attributes */
           gsmsdp_set_setup_attribute(level, dcb_p->sdp->src_sdp, media->setup);
 
           /* This is a new media line so we should send connection:new */
           gsmsdp_set_connection_attribute(level, dcb_p->sdp->src_sdp,
             SDP_CONNECTION_NEW);
--- a/media/webrtc/signaling/src/sipcc/core/sdp/sdp.h
+++ b/media/webrtc/signaling/src/sipcc/core/sdp/sdp.h
@@ -469,74 +469,24 @@ typedef enum {
 
 typedef enum {
     SDP_RTCP_UNICAST_MODE_REFLECTION,
     SDP_RTCP_UNICAST_MODE_RSI,
     SDP_RTCP_MAX_UNICAST_MODE,
     SDP_RTCP_UNICAST_MODE_NOT_PRESENT
 } sdp_rtcp_unicast_mode_e;
 
-/* a=rtcp-fb enumerations */
-
-typedef enum {
-    SDP_RTCP_FB_ANY = -1,
-    SDP_RTCP_FB_ACK = 0,
-    SDP_RTCP_FB_CCM,
-    SDP_RTCP_FB_NACK,
-    SDP_RTCP_FB_TRR_INT,
-    SDP_MAX_RTCP_FB,
-    SDP_RTCP_FB_UNKNOWN
-} sdp_rtcp_fb_type_e;
-
-typedef enum {
-    SDP_RTCP_FB_NACK_NOT_FOUND = -1,
-    SDP_RTCP_FB_NACK_BASIC = 0,
-    SDP_RTCP_FB_NACK_SLI,
-    SDP_RTCP_FB_NACK_PLI,
-    SDP_RTCP_FB_NACK_RPSI,
-    SDP_RTCP_FB_NACK_APP,
-    SDP_RTCP_FB_NACK_RAI,
-    SDP_RTCP_FB_NACK_TLLEI,
-    SDP_RTCP_FB_NACK_PSLEI,
-    SDP_RTCP_FB_NACK_ECN,
-    SDP_MAX_RTCP_FB_NACK,
-    SDP_RTCP_FB_NACK_UNKNOWN
-} sdp_rtcp_fb_nack_type_e;
-
-typedef enum {
-    SDP_RTCP_FB_ACK_NOT_FOUND = -1,
-    SDP_RTCP_FB_ACK_RPSI = 0,
-    SDP_RTCP_FB_ACK_APP,
-    SDP_MAX_RTCP_FB_ACK,
-    SDP_RTCP_FB_ACK_UNKNOWN
-} sdp_rtcp_fb_ack_type_e;
-
-typedef enum {
-    SDP_RTCP_FB_CCM_NOT_FOUND = -1,
-    SDP_RTCP_FB_CCM_FIR = 0,
-    SDP_RTCP_FB_CCM_TMMBR,
-    SDP_RTCP_FB_CCM_TSTR,
-    SDP_RTCP_FB_CCM_VBCM,
-    SDP_MAX_RTCP_FB_CCM,
-    SDP_RTCP_FB_CCM_UNKNOWN
-} sdp_rtcp_fb_ccm_type_e;
-
 typedef enum {
     SDP_CONNECTION_NOT_FOUND = -1,
     SDP_CONNECTION_NEW = 0,
     SDP_CONNECTION_EXISTING,
     SDP_MAX_CONNECTION,
     SDP_CONNECTION_UNKNOWN
 } sdp_connection_type_e;
 
-#define SDP_RTCP_FB_NACK_TO_BITMAP(type) (1 << (type))
-#define SDP_RTCP_FB_ACK_TO_BITMAP(type)  (1 << (SDP_MAX_RTCP_FB_NACK + (type)))
-#define SDP_RTCP_FB_CCM_TO_BITMAP(type)  (1 << (SDP_MAX_RTCP_FB_NACK + \
-                                                SDP_MAX_RTCP_FB_ACK + (type)))
-
 /*
  * sdp_srtp_fec_order_t
  *  This type defines the order in which to perform FEC
  *  (Forward Error Correction) and SRTP Encryption/Authentication.
  */
 typedef enum sdp_srtp_fec_order_t_ {
     SDP_SRTP_THEN_FEC, /* upon sending perform SRTP then FEC */
     SDP_FEC_THEN_SRTP, /* upon sending perform FEC then SRTP */
--- a/media/webrtc/signaling/src/sipcc/include/ccsdp.h
+++ b/media/webrtc/signaling/src/sipcc/include/ccsdp.h
@@ -42,16 +42,17 @@
  *             attribute is specified for.
  * </pre>
  */
 
 #ifndef __CCSDP_H__
 #define __CCSDP_H__
 
 #include "cpr_types.h"
+#include "ccsdp_rtcp_fb.h"
 
 #define SIPSDP_ILBC_MODE20 20
 
 /**
  * Return codes for sdp helper APIs
  */
 typedef enum rtp_ptype_
 {
new file mode 100644
--- /dev/null
+++ b/media/webrtc/signaling/src/sipcc/include/ccsdp_rtcp_fb.h
@@ -0,0 +1,96 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef __CCSDP_RTCP_FB_H__
+#define __CCSDP_RTCP_FB_H__
+
+/* a=rtcp-fb enumerations */
+
+typedef enum {
+    SDP_RTCP_FB_ANY = -1,
+    SDP_RTCP_FB_ACK = 0,
+    SDP_RTCP_FB_CCM,
+    SDP_RTCP_FB_NACK,
+    SDP_RTCP_FB_TRR_INT,
+    SDP_MAX_RTCP_FB,
+    SDP_RTCP_FB_UNKNOWN
+} sdp_rtcp_fb_type_e;
+
+typedef enum {
+    SDP_RTCP_FB_NACK_NOT_FOUND = -1,
+    SDP_RTCP_FB_NACK_BASIC = 0,
+    SDP_RTCP_FB_NACK_SLI,
+    SDP_RTCP_FB_NACK_PLI,
+    SDP_RTCP_FB_NACK_RPSI,
+    SDP_RTCP_FB_NACK_APP,
+    SDP_RTCP_FB_NACK_RAI,
+    SDP_RTCP_FB_NACK_TLLEI,
+    SDP_RTCP_FB_NACK_PSLEI,
+    SDP_RTCP_FB_NACK_ECN,
+    SDP_MAX_RTCP_FB_NACK,
+    SDP_RTCP_FB_NACK_UNKNOWN
+} sdp_rtcp_fb_nack_type_e;
+
+typedef enum {
+    SDP_RTCP_FB_ACK_NOT_FOUND = -1,
+    SDP_RTCP_FB_ACK_RPSI = 0,
+    SDP_RTCP_FB_ACK_APP,
+    SDP_MAX_RTCP_FB_ACK,
+    SDP_RTCP_FB_ACK_UNKNOWN
+} sdp_rtcp_fb_ack_type_e;
+
+typedef enum {
+    SDP_RTCP_FB_CCM_NOT_FOUND = -1,
+    SDP_RTCP_FB_CCM_FIR = 0,
+    SDP_RTCP_FB_CCM_TMMBR,
+    SDP_RTCP_FB_CCM_TSTR,
+    SDP_RTCP_FB_CCM_VBCM,
+    SDP_MAX_RTCP_FB_CCM,
+    SDP_RTCP_FB_CCM_UNKNOWN
+} sdp_rtcp_fb_ccm_type_e;
+
+#if defined(__has_extension) && __has_extension(cxx_static_assert)
+static_assert(SDP_MAX_RTCP_FB_NACK +
+              SDP_MAX_RTCP_FB_ACK +
+              SDP_MAX_RTCP_FB_CCM < 32,
+              "rtcp-fb Bitmap is larger than 32 bits");
+#endif
+
+static inline int32_t
+sdp_rtcp_fb_nack_to_bitmap(sdp_rtcp_fb_nack_type_e type)
+{
+  int bitnumber = type;
+
+  if (type < 0 || type >= SDP_MAX_RTCP_FB_NACK) {
+    return 0;
+  }
+
+  return (1 << bitnumber);
+}
+
+static inline int32_t
+sdp_rtcp_fb_ack_to_bitmap(sdp_rtcp_fb_ack_type_e type)
+{
+  int bitnumber = type + SDP_MAX_RTCP_FB_NACK;
+
+  if (type < 0 || type >= SDP_MAX_RTCP_FB_ACK) {
+    return 0;
+  }
+
+  return (1 << bitnumber);
+}
+
+static inline int32_t
+sdp_rtcp_fb_ccm_to_bitmap(sdp_rtcp_fb_ccm_type_e type)
+{
+  int bitnumber = type + SDP_MAX_RTCP_FB_NACK + SDP_MAX_RTCP_FB_ACK;
+
+  if (type < 0 || type >= SDP_MAX_RTCP_FB_CCM) {
+    return 0;
+  }
+
+  return (1 << bitnumber);
+}
+
+#endif
--- a/media/webrtc/signaling/test/mediaconduit_unittests.cpp
+++ b/media/webrtc/signaling/test/mediaconduit_unittests.cpp
@@ -568,18 +568,18 @@ class TransportConduitTest : public ::te
     err = mVideoSession2->AttachRenderer(mVideoRenderer);
     ASSERT_EQ(mozilla::kMediaConduitNoError, err);
     err = mVideoSession->AttachTransport(mVideoTransport);
     ASSERT_EQ(mozilla::kMediaConduitNoError, err);
     err = mVideoSession2->AttachTransport(mVideoTransport);
     ASSERT_EQ(mozilla::kMediaConduitNoError, err);
 
     //configure send and recv codecs on theconduit
-    mozilla::VideoCodecConfig cinst1(120, "VP8");
-    mozilla::VideoCodecConfig cinst2(124, "I420");
+    mozilla::VideoCodecConfig cinst1(120, "VP8", 0);
+    mozilla::VideoCodecConfig cinst2(124, "I420", 0);
 
 
     std::vector<mozilla::VideoCodecConfig* > rcvCodecList;
     rcvCodecList.push_back(&cinst1);
     rcvCodecList.push_back(&cinst2);
 
     err = mVideoSession->ConfigureSendMediaCodec(&cinst1);
     ASSERT_EQ(mozilla::kMediaConduitNoError, err);
@@ -632,35 +632,35 @@ class TransportConduitTest : public ::te
 
     std::vector<mozilla::VideoCodecConfig* > rcvCodecList;
 
     //Same APIs
     cerr << "   *************************************************" << endl;
     cerr << "    1. Same Codec (VP8) Repeated Twice " << endl;
     cerr << "   *************************************************" << endl;
 
-    mozilla::VideoCodecConfig cinst1(120, "VP8");
-    mozilla::VideoCodecConfig cinst2(120, "VP8");
+    mozilla::VideoCodecConfig cinst1(120, "VP8", 0);
+    mozilla::VideoCodecConfig cinst2(120, "VP8", 0);
     rcvCodecList.push_back(&cinst1);
     rcvCodecList.push_back(&cinst2);
     err = mVideoSession->ConfigureRecvMediaCodecs(rcvCodecList);
     EXPECT_NE(err,mozilla::kMediaConduitNoError);
     rcvCodecList.pop_back();
     rcvCodecList.pop_back();
 
 
     PR_Sleep(PR_SecondsToInterval(2));
     cerr << "   *************************************************" << endl;
     cerr << "    2. Codec With Invalid Payload Names " << endl;
     cerr << "   *************************************************" << endl;
     cerr << "   Setting payload 1 with name: I4201234tttttthhhyyyy89087987y76t567r7756765rr6u6676" << endl;
     cerr << "   Setting payload 2 with name of zero length" << endl;
 
-    mozilla::VideoCodecConfig cinst3(124, "I4201234tttttthhhyyyy89087987y76t567r7756765rr6u6676");
-    mozilla::VideoCodecConfig cinst4(124, "");
+    mozilla::VideoCodecConfig cinst3(124, "I4201234tttttthhhyyyy89087987y76t567r7756765rr6u6676", 0);
+    mozilla::VideoCodecConfig cinst4(124, "", 0);
 
     rcvCodecList.push_back(&cinst3);
     rcvCodecList.push_back(&cinst4);
 
     err = mVideoSession->ConfigureRecvMediaCodecs(rcvCodecList);
     EXPECT_TRUE(err != mozilla::kMediaConduitNoError);
     rcvCodecList.pop_back();
     rcvCodecList.pop_back();