Bug 880067 - Part 1: SDP rtcp-fb parsing/serializing r=ekr,ehugg
authorAdam Roach [:abr] <adam@nostrum.com>
Thu, 20 Jun 2013 14:34:13 -0500
changeset 135850 2ffbbe96954c1937a76f9505f7783387a5a7582a
parent 135849 f7c30b1d5c3593904a7f89bb19c12a696124cb1b
child 135851 d3e29136b2c6562b9e55f71f915e4dfc435aefb3
push id29846
push useradam@nostrum.com
push dateThu, 20 Jun 2013 19:34:24 +0000
treeherdermozilla-inbound@2ffbbe96954c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersekr, ehugg
bugs880067
milestone24.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 1: SDP rtcp-fb parsing/serializing r=ekr,ehugg
media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c
media/webrtc/signaling/src/sipcc/core/sdp/sdp.h
media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr.c
media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr_access.c
media/webrtc/signaling/src/sipcc/core/sdp/sdp_main.c
media/webrtc/signaling/src/sipcc/core/sdp/sdp_private.h
media/webrtc/signaling/src/sipcc/include/ccsdp.h
media/webrtc/signaling/test/Makefile.in
media/webrtc/signaling/test/sdp_unittests.cpp
media/webrtc/signaling/test/signaling_unittests.cpp
--- a/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c
+++ b/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c
@@ -1638,16 +1638,156 @@ gsmsdp_set_ice_attribute (sdp_attr_e sdp
 
     result = sdp_attr_set_ice_attribute(sdp_p, level, 0, sdp_attr, a_instance, ice_attrib);
     if (result != SDP_SUCCESS) {
         GSM_ERR_MSG("Failed to set attribute");
     }
 }
 
 /*
+ * gsmsdp_set_rtcp_fb_ack_attribute
+ *
+ * Description:
+ *
+ * Adds an rtcp-fb:...ack attribute attributes to the specified SDP.
+ *
+ * Parameters:
+ *
+ * level        - The media level of the SDP where the media attribute exists.
+ * sdp_p        - Pointer to the SDP to set the ice candidate attribute against.
+ * ack_type     - Type of ack feedback mechanism in use
+ */
+void
+gsmsdp_set_rtcp_fb_ack_attribute (uint16_t level,
+                                  void *sdp_p,
+                                  u16 payload_type,
+                                  sdp_rtcp_fb_ack_type_e ack_type)
+{
+    uint16_t      a_instance = 0;
+    sdp_result_e  result;
+
+    result = sdp_add_new_attr(sdp_p, level, 0, SDP_ATTR_RTCP_FB, &a_instance);
+    if (result != SDP_SUCCESS) {
+        GSM_ERR_MSG("Failed to add attribute");
+        return;
+    }
+
+    result = sdp_attr_set_rtcp_fb_ack(sdp_p, level, payload_type,
+                                      a_instance, ack_type);
+    if (result != SDP_SUCCESS) {
+        GSM_ERR_MSG("Failed to set attribute");
+    }
+}
+
+/*
+ * gsmsdp_set_rtcp_fb_nack_attribute
+ *
+ * Description:
+ *
+ * Adds an rtcp-fb:...nack attribute attributes to the specified SDP.
+ *
+ * Parameters:
+ *
+ * level        - The media level of the SDP where the media attribute exists.
+ * sdp_p        - Pointer to the SDP to set the ice candidate attribute against.
+ * nack_type    - Type of nack feedback mechanism in use
+ */
+void
+gsmsdp_set_rtcp_fb_nack_attribute (uint16_t level,
+                                   void *sdp_p,
+                                   u16 payload_type,
+                                   sdp_rtcp_fb_nack_type_e nack_type)
+{
+    uint16_t      a_instance = 0;
+    sdp_result_e  result;
+
+    result = sdp_add_new_attr(sdp_p, level, 0, SDP_ATTR_RTCP_FB, &a_instance);
+    if (result != SDP_SUCCESS) {
+        GSM_ERR_MSG("Failed to add attribute");
+        return;
+    }
+
+    result = sdp_attr_set_rtcp_fb_nack(sdp_p, level, payload_type,
+                                       a_instance, nack_type);
+    if (result != SDP_SUCCESS) {
+        GSM_ERR_MSG("Failed to set attribute");
+    }
+}
+
+/*
+ * gsmsdp_set_rtcp_fb_trr_int_attribute
+ *
+ * Description:
+ *
+ * Adds an rtcp-fb:...trr-int attribute attributes to the specified SDP.
+ *
+ * Parameters:
+ *
+ * level        - The media level of the SDP where the media attribute exists.
+ * sdp_p        - Pointer to the SDP to set the ice candidate attribute against.
+ * trr_interval - Interval to set trr-int value to
+ */
+void
+gsmsdp_set_rtcp_fb_trr_int_attribute (uint16_t level,
+                                      void *sdp_p,
+                                      u16 payload_type,
+                                      u32 trr_interval)
+{
+    uint16_t      a_instance = 0;
+    sdp_result_e  result;
+
+    result = sdp_add_new_attr(sdp_p, level, 0, SDP_ATTR_RTCP_FB, &a_instance);
+    if (result != SDP_SUCCESS) {
+        GSM_ERR_MSG("Failed to add attribute");
+        return;
+    }
+
+    result = sdp_attr_set_rtcp_fb_trr_int(sdp_p, level, payload_type,
+                                          a_instance, trr_interval);
+    if (result != SDP_SUCCESS) {
+        GSM_ERR_MSG("Failed to set attribute");
+    }
+}
+
+/*
+ * gsmsdp_set_rtcp_fb_ccm_attribute
+ *
+ * Description:
+ *
+ * Adds an rtcp-fb:...ccm attribute attributes to the specified SDP.
+ *
+ * Parameters:
+ *
+ * level        - The media level of the SDP where the media attribute exists.
+ * sdp_p        - Pointer to the SDP to set the ice candidate attribute against.
+ * ccm_type     - Type of ccm feedback mechanism in use
+ */
+void
+gsmsdp_set_rtcp_fb_ccm_attribute (uint16_t level,
+                                  void *sdp_p,
+                                  u16 payload_type,
+                                  sdp_rtcp_fb_ccm_type_e ccm_type)
+{
+    uint16_t      a_instance = 0;
+    sdp_result_e  result;
+
+    result = sdp_add_new_attr(sdp_p, level, 0, SDP_ATTR_RTCP_FB, &a_instance);
+    if (result != SDP_SUCCESS) {
+        GSM_ERR_MSG("Failed to add attribute");
+        return;
+    }
+
+    result = sdp_attr_set_rtcp_fb_ccm(sdp_p, level, payload_type,
+                                      a_instance, ccm_type);
+    if (result != SDP_SUCCESS) {
+        GSM_ERR_MSG("Failed to set attribute");
+    }
+}
+
+/*
  * gsmsdp_set_rtcp_mux_attribute
  *
  * Description:
  *
  * Adds an ice attribute attributes to the specified SDP.
  *
  * Parameters:
  *
@@ -4661,16 +4801,29 @@ gsmsdp_negotiate_media_lines (fsm_fcb_t 
             break;
 
         default:
             /* Not a support media type stream */
             unsupported_line = TRUE;
             break;
         }
 
+        /* TODO (abr) -- temporarily hardcode rtcb-fb attributes to match our
+           actual behavior. This really needs to be a negotiation, with the
+           results of the negotiation propagating into the codec configuration.
+           See Bug 880067. */
+        if (media_type == SDP_MEDIA_VIDEO) {
+            gsmsdp_set_rtcp_fb_nack_attribute(media->level, sdp_p->src_sdp,
+                                              SDP_ALL_PAYLOADS,
+                                              SDP_RTCP_FB_NACK_UNSPECIFIED);
+            gsmsdp_set_rtcp_fb_ccm_attribute(media->level, sdp_p->src_sdp,
+                                             SDP_ALL_PAYLOADS,
+                                             SDP_RTCP_FB_CCM_FIR);
+        }
+
         if (unsupported_line) {
             /* add this line to unsupported line */
             gsmsdp_add_unsupported_stream_to_local_sdp(sdp_p, i);
             gsmsdp_set_mid_attr(sdp_p->src_sdp, i);
             /* Remove the media if one to be removed */
             if (media != NULL) {
                 /* remove this media off the list */
                 gsmsdp_remove_media(dcb_p, media);
@@ -5225,16 +5378,30 @@ gsmsdp_create_local_sdp (fsmdef_dcb_t *d
                 }
             } else {
                 if (gsmsdp_add_media_line(dcb_p, media_cap, cap_index, level,
                                           CPR_IP_ADDR_IPV4, offer) == NULL) {
                     /* fail to add a media line, go back one level */
                     level = level - 1;
                 }
             }
+
+            /* TODO (abr) -- temporarily hardcode rtcb-fb attributes to match
+               our actual behavior. This really needs to be a negotiation, with
+               the results of the negotiation propagating into the codec
+               configuration.  See Bug 880067. */
+            if (media_cap->type == SDP_MEDIA_VIDEO) {
+                gsmsdp_set_rtcp_fb_nack_attribute(level, dcb_p->sdp->src_sdp,
+                                                  SDP_ALL_PAYLOADS,
+                                                  SDP_RTCP_FB_NACK_UNSPECIFIED);
+                gsmsdp_set_rtcp_fb_ccm_attribute(level, dcb_p->sdp->src_sdp,
+                                                 SDP_ALL_PAYLOADS,
+                                                 SDP_RTCP_FB_CCM_FIR);
+            }
+
         }
         /* next capability */
         media_cap++;
     }
 
     if (level == 0) {
         /*
          * Did not find media line for the SDP and we do not
--- a/media/webrtc/signaling/src/sipcc/core/sdp/sdp.h
+++ b/media/webrtc/signaling/src/sipcc/core/sdp/sdp.h
@@ -469,16 +469,61 @@ 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_UNSPECIFIED = 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;
+
 /*
  * 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 */
@@ -823,16 +868,35 @@ typedef struct sdp_source_filter {
    sdp_nettype_e     nettype;
    sdp_addrtype_e    addrtype;
    char              dest_addr[SDP_MAX_STRING_LEN+1];
    u16               num_src_addr;
    char              src_list[SDP_MAX_SRC_ADDR_LIST+1][SDP_MAX_STRING_LEN+1];
 } sdp_source_filter_t;
 
 /*
+ * a=rtcp-fb:<payload-type> <feedback-type> [<feedback-parameters>]
+ * Defines RTCP feedback parameters
+ */
+#define SDP_ALL_PAYLOADS         0xFFFF
+typedef struct sdp_fmtp_fb {
+    u16                          payload_num;    /* can be SDP_ALL_PAYLOADS */
+    sdp_rtcp_fb_type_e           feedback_type;
+    union {
+        sdp_rtcp_fb_ack_type_e   ack;
+        sdp_rtcp_fb_ccm_type_e   ccm;
+        sdp_rtcp_fb_nack_type_e  nack;
+        u32                      trr_int;
+    } param;
+    char extra[SDP_MAX_STRING_LEN + 1]; /* Holds any trailing information that
+                                           cannot be represented by preceding
+                                           fields. */
+} sdp_fmtp_fb_t;
+
+/*
  * b=<bw-modifier>:<val>
  *
 */
 typedef struct sdp_bw_data {
     struct sdp_bw_data       *next_p;
     sdp_bw_modifier_e        bw_modifier;
     int                      bw_val;
 } sdp_bw_data_t;
@@ -934,22 +998,23 @@ typedef struct sdp_attr {
         sdp_transport_map_t   transport_map;    /* A rtpmap or sprtmap */
         sdp_subnet_t          subnet;
         sdp_t38_ratemgmt_e    t38ratemgmt;
         sdp_t38_udpec_e       t38udpec;
         sdp_pccodec_t         pccodec;
         sdp_silencesupp_t     silencesupp;
         sdp_mca_t            *cap_p;    /* A X-CAP or CDSC attribute */
         sdp_rtr_t             rtr;
-    sdp_comediadir_t      comediadir;
-    sdp_srtp_crypto_context_t srtp_context;
+        sdp_comediadir_t      comediadir;
+        sdp_srtp_crypto_context_t srtp_context;
         sdp_mptime_t          mptime;
         sdp_stream_data_t     stream_data;
         char                  unknown[SDP_MAX_STRING_LEN+1];
         sdp_source_filter_t   source_filter;
+        sdp_fmtp_fb_t         rtcp_fb;
     } attr;
     struct sdp_attr          *next_p;
 } sdp_attr_t;
 
 typedef struct sdp_srtp_crypto_suite_list_ {
     sdp_srtp_crypto_suite_t crypto_suite_val;
     char * crypto_suite_str;
     unsigned char key_size_bytes;
@@ -1023,17 +1088,16 @@ typedef struct {
 
 /* Token processing table. */
 typedef struct {
     char *name;
     sdp_result_e (*parse_func)(sdp_t *sdp_p, u16 level, const char *ptr);
     sdp_result_e (*build_func)(sdp_t *sdp_p, u16 level, flex_string *fs);
 } sdp_tokenarray_t;
 
-
 /* Attribute processing table. */
 typedef struct {
     char *name;
     u16 strlen;
     sdp_result_e (*parse_func)(sdp_t *sdp_p, sdp_attr_t *attr_p,
                                const char *ptr);
     sdp_result_e (*build_func)(sdp_t *sdp_p, sdp_attr_t *attr_p,
                                flex_string *fs);
@@ -2003,9 +2067,38 @@ sdp_result_e
 sdp_attr_get_dtls_fingerprint_attribute (void *sdp_ptr, u16 level,
                                   u8 cap_num, sdp_attr_e sdp_attr, u16 inst_num,
                                   char **out);
 
 sdp_result_e
 sdp_attr_set_dtls_fingerprint_attribute(void *sdp_ptr, u16 level,
                               u8 cap_num, sdp_attr_e sdp_attr, u16 inst_num, const char *dtls_fingerprint);
 
+sdp_rtcp_fb_ack_type_e
+sdp_attr_get_rtcp_fb_ack(void *sdp_ptr, u16 level, u16 payload_type, u16 inst);
+
+sdp_rtcp_fb_nack_type_e
+sdp_attr_get_rtcp_fb_nack(void *sdp_ptr, u16 level, u16 payload_type, u16 inst);
+
+u32
+sdp_attr_get_rtcp_fb_trr_int(void *sdp_ptr, u16 level, u16 payload_type,
+                             u16 inst);
+
+sdp_rtcp_fb_ccm_type_e
+sdp_attr_get_rtcp_fb_ccm(void *sdp_ptr, u16 level, u16 payload_type, u16 inst);
+
+sdp_result_e
+sdp_attr_set_rtcp_fb_ack(void *sdp_ptr, u16 level, u16 payload_type, u16 inst,
+                         sdp_rtcp_fb_ack_type_e type);
+
+sdp_result_e
+sdp_attr_set_rtcp_fb_nack(void *sdp_ptr, u16 level, u16 payload_type, u16 inst,
+                          sdp_rtcp_fb_nack_type_e);
+
+sdp_result_e
+sdp_attr_set_rtcp_fb_trr_int(void *sdp_ptr, u16 level, u16 payload_type,
+                             u16 inst, u32 interval);
+
+sdp_result_e
+sdp_attr_set_rtcp_fb_ccm(void *sdp_ptr, u16 level, u16 payload_type, u16 inst,
+                         sdp_rtcp_fb_ccm_type_e);
+
 #endif /* _SDP_H_ */
--- a/media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr.c
+++ b/media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr.c
@@ -4750,8 +4750,247 @@ sdp_result_e sdp_build_attr_rtcp_mux_att
     return SDP_SUCCESS;
 }
 
 sdp_result_e sdp_parse_attr_rtcp_mux_attr (sdp_t *sdp_p, sdp_attr_t *attr_p, const char *ptr) {
     attr_p->attr.boolean_val = TRUE;
 
     return (SDP_SUCCESS);
 }
+
+sdp_result_e sdp_build_attr_rtcp_fb(sdp_t *sdp_p,
+                                    sdp_attr_t *attr_p,
+                                    flex_string *fs)
+{
+    flex_string_sprintf(fs, "a=%s:", sdp_attr[attr_p->type].name);
+
+    /* Payload Type */
+    if (attr_p->attr.rtcp_fb.payload_num == SDP_ALL_PAYLOADS) {
+      flex_string_sprintf(fs, "* ");
+    } else {
+      flex_string_sprintf(fs, "%d ",attr_p->attr.rtcp_fb.payload_num);
+    }
+
+    /* Feedback Type */
+    if (attr_p->attr.rtcp_fb.feedback_type < SDP_RTCP_FB_UNKNOWN) {
+        flex_string_sprintf(fs, "%s",
+            sdp_rtcp_fb_type_val[attr_p->attr.rtcp_fb.feedback_type].name);
+    }
+
+    /* Feedback Type Parameters */
+    switch (attr_p->attr.rtcp_fb.feedback_type) {
+        case SDP_RTCP_FB_ACK:
+            if (attr_p->attr.rtcp_fb.param.ack < SDP_MAX_RTCP_FB_ACK) {
+                flex_string_sprintf(fs, " %s",
+                    sdp_rtcp_fb_ack_type_val[attr_p->attr.rtcp_fb.param.ack]
+                        .name);
+            }
+            break;
+        case SDP_RTCP_FB_CCM: /* RFC 5104 */
+            if (attr_p->attr.rtcp_fb.param.ccm < SDP_MAX_RTCP_FB_CCM) {
+                flex_string_sprintf(fs, " %s",
+                    sdp_rtcp_fb_ccm_type_val[attr_p->attr.rtcp_fb.param.ccm]
+                        .name);
+            }
+            break;
+        case SDP_RTCP_FB_NACK:
+            if (attr_p->attr.rtcp_fb.param.nack > SDP_RTCP_FB_NACK_UNSPECIFIED
+                && attr_p->attr.rtcp_fb.param.nack < SDP_MAX_RTCP_FB_NACK) {
+                flex_string_sprintf(fs, " %s",
+                    sdp_rtcp_fb_nack_type_val[attr_p->attr.rtcp_fb.param.nack]
+                        .name);
+            }
+            break;
+        case SDP_RTCP_FB_TRR_INT:
+            flex_string_sprintf(fs, " %u", attr_p->attr.rtcp_fb.param.trr_int);
+            break;
+
+        case SDP_RTCP_FB_UNKNOWN:
+            /* Contents are in the "extra" field */
+            break;
+
+        default:
+            CSFLogError(logTag, "%s Error: Invalid rtcp-fb enum (%d)",
+                        sdp_p->debug_str, attr_p->attr.rtcp_fb.feedback_type);
+            return SDP_FAILURE;
+    }
+
+    /* Tack on any information that cannot otherwise be represented by
+     * the sdp_fmtp_fb_t structure. */
+    if (attr_p->attr.rtcp_fb.extra[0]) {
+        flex_string_sprintf(fs, " %s", attr_p->attr.rtcp_fb.extra);
+    }
+
+    /* Line ending */
+    flex_string_sprintf(fs, "\r\n");
+
+    return SDP_SUCCESS;
+}
+
+static int find_token_enum(const char *attr_name,
+                           sdp_t *sdp_p,
+                           const char **ptr,
+                           const sdp_namearray_t *types,
+                           int type_count,
+                           int unknown_value)
+{
+    sdp_result_e  result = SDP_SUCCESS;
+    char          tmp[SDP_MAX_STRING_LEN+1];
+    int           i;
+
+    *ptr = sdp_getnextstrtok(*ptr, tmp, sizeof(tmp), " \t", &result);
+    if (result != SDP_SUCCESS) {
+        sdp_parse_error(sdp_p->peerconnection,
+            "%s Warning: problem parsing %s", sdp_p->debug_str, attr_name);
+        sdp_p->conf_p->num_invalid_param++;
+        return -1;
+    }
+
+    for (i=0; i < type_count; i++) {
+        if (!cpr_strncasecmp(tmp, types[i].name, types[i].strlen)) {
+            return i;
+        }
+    }
+    return unknown_value;
+}
+
+sdp_result_e sdp_parse_attr_rtcp_fb (sdp_t *sdp_p,
+                                     sdp_attr_t *attr_p,
+                                     const char *ptr)
+{
+    sdp_result_e     result = SDP_SUCCESS;
+    sdp_fmtp_fb_t   *rtcp_fb_p = &(attr_p->attr.rtcp_fb);
+    char             tmp[SDP_MAX_STRING_LEN+1];
+    int              i;
+
+    /* Set up attribute fields */
+    rtcp_fb_p->payload_num = 0;
+    rtcp_fb_p->feedback_type = SDP_RTCP_FB_UNKNOWN;
+    rtcp_fb_p->extra[0] = '\0';
+
+    /* Skip WS (just in case) */
+    while (*ptr == ' ' || *ptr == '\t') {
+        ptr++;
+    }
+
+    /* Look for the special "*" payload type */
+    if (*ptr == '*') {
+        rtcp_fb_p->payload_num = SDP_ALL_PAYLOADS;
+        ptr++;
+    } else {
+        /* If the pt is not '*', parse it out as an integer */
+        rtcp_fb_p->payload_num = (u16)sdp_getnextnumtok(ptr, &ptr,
+                                                        " \t", &result);
+        if (result != SDP_SUCCESS) {
+            sdp_parse_error(sdp_p->peerconnection,
+              "%s Warning: could not parse payload type for rtcp-fb attribute",
+              sdp_p->debug_str);
+            sdp_p->conf_p->num_invalid_param++;
+
+            return SDP_INVALID_PARAMETER;
+        }
+    }
+
+    /* Read feedback type */
+    i = find_token_enum("rtcp-fb attribute", sdp_p, &ptr, sdp_rtcp_fb_type_val,
+                        SDP_MAX_RTCP_FB, SDP_RTCP_FB_UNKNOWN);
+    if (i < 0) {
+        sdp_parse_error(sdp_p->peerconnection,
+          "%s Warning: could not parse feedback type for rtcp-fb attribute",
+          sdp_p->debug_str);
+        sdp_p->conf_p->num_invalid_param++;
+        return SDP_INVALID_PARAMETER;
+    }
+    rtcp_fb_p->feedback_type = (sdp_rtcp_fb_type_e) i;
+
+    switch(rtcp_fb_p->feedback_type) {
+        case SDP_RTCP_FB_ACK:
+            i = find_token_enum("rtcp-fb ack type", sdp_p, &ptr,
+                                sdp_rtcp_fb_ack_type_val,
+                                SDP_MAX_RTCP_FB_ACK, SDP_RTCP_FB_ACK_UNKNOWN);
+            if (i < 0) {
+                sdp_parse_error(sdp_p->peerconnection,
+                  "%s Warning: could not parse ack type for rtcp-fb attribute",
+                  sdp_p->debug_str);
+                sdp_p->conf_p->num_invalid_param++;
+                return SDP_INVALID_PARAMETER;
+            }
+            rtcp_fb_p->param.ack = (sdp_rtcp_fb_ack_type_e) i;
+            break;
+
+        case SDP_RTCP_FB_CCM:
+            i = find_token_enum("rtcp-fb ccm type", sdp_p, &ptr,
+                                sdp_rtcp_fb_ccm_type_val,
+                                SDP_MAX_RTCP_FB_CCM, SDP_RTCP_FB_CCM_UNKNOWN);
+            if (i < 0) {
+                sdp_parse_error(sdp_p->peerconnection,
+                  "%s Warning: could not parse ccm type for rtcp-fb attribute",
+                  sdp_p->debug_str);
+                sdp_p->conf_p->num_invalid_param++;
+                return SDP_INVALID_PARAMETER;
+            }
+            rtcp_fb_p->param.ccm = (sdp_rtcp_fb_ccm_type_e) i;
+
+            /* TODO -- We don't currently parse tmmbr parameters or vbcm
+               submessage types. If we decide to support these modes of
+               operation, we probably want to add parsing code for them.
+               For the time being, they'll just end up parsed into "extra". */
+            break;
+
+        case SDP_RTCP_FB_NACK:
+            /* Skip any remaining WS -- see
+               http://code.google.com/p/webrtc/issues/detail?id=1922 */
+            while (*ptr == ' ' || *ptr == '\t') {
+                ptr++;
+            }
+            /* Check for empty string */
+            if (*ptr == '\r') {
+                rtcp_fb_p->param.nack = SDP_RTCP_FB_NACK_UNSPECIFIED;
+                break;
+            }
+            i = find_token_enum("rtcp-fb nack type", sdp_p, &ptr,
+                                sdp_rtcp_fb_nack_type_val,
+                                SDP_MAX_RTCP_FB_NACK, SDP_RTCP_FB_NACK_UNKNOWN);
+            if (i < 0) {
+                sdp_parse_error(sdp_p->peerconnection,
+                  "%s Warning: could not parse nack type for rtcp-fb attribute",
+                  sdp_p->debug_str);
+                sdp_p->conf_p->num_invalid_param++;
+                return SDP_INVALID_PARAMETER;
+            }
+            rtcp_fb_p->param.nack = (sdp_rtcp_fb_nack_type_e) i;
+            break;
+
+        case SDP_RTCP_FB_TRR_INT:
+            rtcp_fb_p->param.trr_int = sdp_getnextnumtok(ptr, &ptr,
+                                                         " \t", &result);
+            if (result != SDP_SUCCESS) {
+                sdp_parse_error(sdp_p->peerconnection,
+                  "%s Warning: could not parse trr-int value for rtcp-fb "
+                  "attribute", sdp_p->debug_str);
+                sdp_p->conf_p->num_invalid_param++;
+                return SDP_INVALID_PARAMETER;
+            }
+            break;
+
+        case SDP_RTCP_FB_UNKNOWN:
+            /* Handled by "extra", below */
+            break;
+
+        default:
+            /* This is an internal error, not a parsing error */
+            CSFLogError(logTag, "%s Error: Invalid rtcp-fb enum (%d)",
+                        sdp_p->debug_str, attr_p->attr.rtcp_fb.feedback_type);
+            return SDP_FAILURE;
+    }
+
+    /* Skip any remaining WS  */
+    while (*ptr == ' ' || *ptr == '\t') {
+        ptr++;
+    }
+
+    /* Just store the rest of the line in "extra" -- this will return
+       a failure result if there is no more text, but that's fine. */
+    ptr = sdp_getnextstrtok(ptr, rtcp_fb_p->extra,
+                            sizeof(rtcp_fb_p->extra), "\r\n", &result);
+
+    return SDP_SUCCESS;
+}
--- a/media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr_access.c
+++ b/media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr_access.c
@@ -11867,8 +11867,338 @@ sdp_attr_set_sdescriptions_salt_size (vo
 	}
     }
 
     attr_p->attr.srtp_context.master_salt_size_bytes = salt_size;
     return SDP_SUCCESS;
 
 }
 
+
+/* Function:    sdp_find_rtcp_fb_attr
+ * Description: Helper to find the nth instance of a rtcp-fb attribute of
+ *              the specified feedback type.
+ * Parameters:  sdp_p        The SDP handle returned by sdp_init_description.
+ *              level        The level to check for the attribute.
+ *              payload_type The payload to get the attribute for
+ *              fb_type      The feedback type to look for.
+ *              inst_num     The attribute instance number to check.
+ * Returns:     Pointer to the attribute, or NULL if not found.
+ */
+
+sdp_attr_t *
+sdp_find_rtcp_fb_attr (sdp_t *sdp_p,
+                       u16 level,
+                       u16 payload_type,
+                       sdp_rtcp_fb_type_e fb_type,
+                       u16 inst_num)
+{
+    u16          attr_count=0;
+    sdp_mca_t   *mca_p;
+    sdp_attr_t  *attr_p;
+
+    mca_p = sdp_find_media_level(sdp_p, level);
+    if (!mca_p) {
+        return (NULL);
+    }
+    for (attr_p = mca_p->media_attrs_p; attr_p; attr_p = attr_p->next_p) {
+        if (attr_p->type == SDP_ATTR_RTCP_FB &&
+            (attr_p->attr.rtcp_fb.payload_num == payload_type ||
+             attr_p->attr.rtcp_fb.payload_num == SDP_ALL_PAYLOADS) &&
+            attr_p->attr.rtcp_fb.feedback_type == fb_type) {
+            attr_count++;
+            if (attr_count == inst_num) {
+                return (attr_p);
+            }
+        }
+    }
+    return NULL;
+}
+
+/* Function:    sdp_attr_get_rtcp_fb_ack
+ * Description: Returns the value of the rtcp-fb:...ack attribute
+ * Parameters:  sdp_ptr      The SDP handle returned by sdp_init_description.
+ *              level        The level to check for the attribute.
+ *              payload_type The payload to get the attribute for
+ *              inst_num    The attribute instance number to check.
+ * Returns:     ACK type (SDP_RTCP_FB_ACK_NOT_FOUND if not present)
+ */
+sdp_rtcp_fb_ack_type_e
+sdp_attr_get_rtcp_fb_ack(void *sdp_ptr, u16 level, u16 payload_type, u16 inst)
+{
+    sdp_t       *sdp_p = (sdp_t *)sdp_ptr;
+    sdp_attr_t  *attr_p;
+
+    if (!sdp_verify_sdp_ptr(sdp_p)) {
+        return SDP_RTCP_FB_ACK_NOT_FOUND;
+    }
+
+    attr_p = sdp_find_rtcp_fb_attr(sdp_p, level, payload_type,
+                                   SDP_RTCP_FB_ACK, inst);
+    if (!attr_p) {
+        if (sdp_p->debug_flag[SDP_DEBUG_ERRORS]) {
+            CSFLogError(logTag, "%s rtcp-fb attribute, level %u, pt %u, "
+                      "instance %u not found.", sdp_p->debug_str, level,
+                      payload_type, inst);
+        }
+        sdp_p->conf_p->num_invalid_param++;
+        return SDP_RTCP_FB_ACK_NOT_FOUND;
+    }
+    return (attr_p->attr.rtcp_fb.param.ack);
+}
+
+/* Function:    sdp_attr_get_rtcp_fb_nack
+ * Description: Returns the value of the rtcp-fb:...nack attribute
+ * Parameters:  sdp_ptr      The SDP handle returned by sdp_init_description.
+ *              level        The level to check for the attribute.
+ *              payload_type The payload to get the attribute for
+ *              inst_num    The attribute instance number to check.
+ * Returns:     NACK type (SDP_RTCP_FB_NACK_NOT_FOUND if not present)
+ */
+sdp_rtcp_fb_nack_type_e
+sdp_attr_get_rtcp_fb_nack(void *sdp_ptr, u16 level, u16 payload_type, u16 inst)
+{
+    sdp_t       *sdp_p = (sdp_t *)sdp_ptr;
+    sdp_attr_t  *attr_p;
+
+    if (!sdp_verify_sdp_ptr(sdp_p)) {
+        return SDP_RTCP_FB_NACK_NOT_FOUND;
+    }
+
+    attr_p = sdp_find_rtcp_fb_attr(sdp_p, level, payload_type,
+                                   SDP_RTCP_FB_NACK, inst);
+    if (!attr_p) {
+        if (sdp_p->debug_flag[SDP_DEBUG_ERRORS]) {
+            CSFLogError(logTag, "%s rtcp-fb attribute, level %u, pt %u, "
+                      "instance %u not found.", sdp_p->debug_str, level,
+                      payload_type, inst);
+        }
+        sdp_p->conf_p->num_invalid_param++;
+        return SDP_RTCP_FB_NACK_NOT_FOUND;
+    }
+    return (attr_p->attr.rtcp_fb.param.nack);
+}
+
+/* Function:    sdp_attr_get_rtcp_fb_trr_int
+ * Description: Returns the value of the rtcp-fb:...trr-int attribute
+ * Parameters:  sdp_ptr      The SDP handle returned by sdp_init_description.
+ *              level        The level to check for the attribute.
+ *              payload_type The payload to get the attribute for
+ *              inst_num    The attribute instance number to check.
+ * Returns:     trr-int interval (0xFFFFFFFF if not found)
+ */
+u32
+sdp_attr_get_rtcp_fb_trr_int(void *sdp_ptr, u16 level,
+                             u16 payload_type, u16 inst)
+{
+    sdp_t       *sdp_p = (sdp_t *)sdp_ptr;
+    sdp_attr_t  *attr_p;
+
+    if (!sdp_verify_sdp_ptr(sdp_p)) {
+        return 0xFFFFFFFF;
+    }
+
+    attr_p = sdp_find_rtcp_fb_attr(sdp_p, level, payload_type,
+                                   SDP_RTCP_FB_TRR_INT, inst);
+    if (!attr_p) {
+        if (sdp_p->debug_flag[SDP_DEBUG_ERRORS]) {
+            CSFLogError(logTag, "%s rtcp-fb attribute, level %u, pt %u, "
+                      "instance %u not found.", sdp_p->debug_str, level,
+                      payload_type, inst);
+        }
+        sdp_p->conf_p->num_invalid_param++;
+        return 0xFFFFFFFF;
+    }
+    return (attr_p->attr.rtcp_fb.param.trr_int);
+}
+
+/* Function:    sdp_attr_get_rtcp_fb_ccm
+ * Description: Returns the value of the rtcp-fb:...ccm attribute
+ * Parameters:  sdp_ptr      The SDP handle returned by sdp_init_description.
+ *              level        The level to check for the attribute.
+ *              payload_type The payload to get the attribute for
+ *              inst_num    The attribute instance number to check.
+ * Returns:     CCM type (SDP_RTCP_FB_CCM_NOT_FOUND if not present)
+ */
+sdp_rtcp_fb_ccm_type_e
+sdp_attr_get_rtcp_fb_ccm(void *sdp_ptr, u16 level, u16 payload_type, u16 inst)
+{
+    sdp_t       *sdp_p = (sdp_t *)sdp_ptr;
+    sdp_attr_t  *attr_p;
+
+    if (!sdp_verify_sdp_ptr(sdp_p)) {
+        return SDP_RTCP_FB_CCM_NOT_FOUND;
+    }
+
+    attr_p = sdp_find_rtcp_fb_attr(sdp_p, level, payload_type,
+                                   SDP_RTCP_FB_CCM, inst);
+    if (!attr_p) {
+        if (sdp_p->debug_flag[SDP_DEBUG_ERRORS]) {
+            CSFLogError(logTag, "%s rtcp-fb attribute, level %u, pt %u, "
+                      "instance %u not found.", sdp_p->debug_str, level,
+                      payload_type, inst);
+        }
+        sdp_p->conf_p->num_invalid_param++;
+        return SDP_RTCP_FB_CCM_NOT_FOUND;
+    }
+    return (attr_p->attr.rtcp_fb.param.ccm);
+}
+
+/* Function:    sdp_attr_set_rtcp_fb_ack
+ * Description: Sets the value of an rtcp-fb:...ack attribute
+ * Parameters:  sdp_ptr        The SDP handle returned by sdp_init_description.
+ *              level          The level to set the attribute.
+ *              payload_type   The value to set the payload type to for
+ *                             this attribute. Can be SDP_ALL_PAYLOADS.
+ *              inst_num       The attribute instance number to check.
+ *              type           The ack type to indicate
+ * Returns:     SDP_SUCCESS            Attribute param was set successfully.
+ *              SDP_INVALID_PARAMETER  Specified attribute is not defined.
+ */
+sdp_result_e
+sdp_attr_set_rtcp_fb_ack(void *sdp_ptr, u16 level, u16 payload_type, u16 inst,
+                         sdp_rtcp_fb_ack_type_e type)
+{
+    sdp_t       *sdp_p = (sdp_t *)sdp_ptr;
+    sdp_attr_t  *attr_p;
+
+    if (!sdp_verify_sdp_ptr(sdp_p)) {
+        return (SDP_INVALID_SDP_PTR);
+    }
+
+    attr_p = sdp_find_attr(sdp_p, level, 0, SDP_ATTR_RTCP_FB, inst);
+    if (!attr_p) {
+        if (sdp_p->debug_flag[SDP_DEBUG_ERRORS]) {
+            CSFLogError(logTag, "%s rtcp_fb ack attribute, level %u "
+                      "instance %u not found.", sdp_p->debug_str, level,
+                      inst);
+        }
+        sdp_p->conf_p->num_invalid_param++;
+        return (SDP_INVALID_PARAMETER);
+    }
+
+    attr_p->attr.rtcp_fb.payload_num = payload_type;
+    attr_p->attr.rtcp_fb.feedback_type = SDP_RTCP_FB_ACK;
+    attr_p->attr.rtcp_fb.param.ack = type;
+    attr_p->attr.rtcp_fb.extra[0] = '\0';
+    return (SDP_SUCCESS);
+}
+
+
+/* Function:    sdp_attr_set_rtcp_fb_nack
+ * Description: Sets the value of an rtcp-fb:...nack attribute
+ * Parameters:  sdp_ptr        The SDP handle returned by sdp_init_description.
+ *              level          The level to set the attribute.
+ *              payload_type   The value to set the payload type to for
+ *                             this attribute. Can be SDP_ALL_PAYLOADS.
+ *              inst_num       The attribute instance number to check.
+ *              type           The nack type to indicate
+ * Returns:     SDP_SUCCESS            Attribute param was set successfully.
+ *              SDP_INVALID_PARAMETER  Specified attribute is not defined.
+ */
+sdp_result_e
+sdp_attr_set_rtcp_fb_nack(void *sdp_ptr, u16 level, u16 payload_type, u16 inst,
+                          sdp_rtcp_fb_nack_type_e type)
+{
+    sdp_t       *sdp_p = (sdp_t *)sdp_ptr;
+    sdp_attr_t  *attr_p;
+
+    if (!sdp_verify_sdp_ptr(sdp_p)) {
+        return (SDP_INVALID_SDP_PTR);
+    }
+
+    attr_p = sdp_find_attr(sdp_p, level, 0, SDP_ATTR_RTCP_FB, inst);
+    if (!attr_p) {
+        if (sdp_p->debug_flag[SDP_DEBUG_ERRORS]) {
+            CSFLogError(logTag, "%s rtcp_fb nack attribute, level %u "
+                      "instance %u not found.", sdp_p->debug_str, level,
+                      inst);
+        }
+        sdp_p->conf_p->num_invalid_param++;
+        return (SDP_INVALID_PARAMETER);
+    }
+
+    attr_p->attr.rtcp_fb.payload_num = payload_type;
+    attr_p->attr.rtcp_fb.feedback_type = SDP_RTCP_FB_NACK;
+    attr_p->attr.rtcp_fb.param.nack = type;
+    attr_p->attr.rtcp_fb.extra[0] = '\0';
+    return (SDP_SUCCESS);
+}
+
+/* Function:    sdp_attr_set_rtcp_fb_trr_int
+ * Description: Sets the value of an rtcp-fb:...trr-int attribute
+ * Parameters:  sdp_ptr        The SDP handle returned by sdp_init_description.
+ *              level          The level to set the attribute.
+ *              payload_type   The value to set the payload type to for
+ *                             this attribute. Can be SDP_ALL_PAYLOADS.
+ *              inst_num       The attribute instance number to check.
+ *              interval       The interval time to indicate
+ * Returns:     SDP_SUCCESS            Attribute param was set successfully.
+ *              SDP_INVALID_PARAMETER  Specified attribute is not defined.
+ */
+sdp_result_e
+sdp_attr_set_rtcp_fb_trr_int(void *sdp_ptr, u16 level, u16 payload_type,
+                             u16 inst, u32 interval)
+{
+    sdp_t       *sdp_p = (sdp_t *)sdp_ptr;
+    sdp_attr_t  *attr_p;
+
+    if (!sdp_verify_sdp_ptr(sdp_p)) {
+        return (SDP_INVALID_SDP_PTR);
+    }
+
+    attr_p = sdp_find_attr(sdp_p, level, 0, SDP_ATTR_RTCP_FB, inst);
+    if (!attr_p) {
+        if (sdp_p->debug_flag[SDP_DEBUG_ERRORS]) {
+            CSFLogError(logTag, "%s rtcp_fb trr-int attribute, level %u "
+                      "instance %u not found.", sdp_p->debug_str, level,
+                      inst);
+        }
+        sdp_p->conf_p->num_invalid_param++;
+        return (SDP_INVALID_PARAMETER);
+    }
+
+    attr_p->attr.rtcp_fb.payload_num = payload_type;
+    attr_p->attr.rtcp_fb.feedback_type = SDP_RTCP_FB_TRR_INT;
+    attr_p->attr.rtcp_fb.param.trr_int = interval;
+    attr_p->attr.rtcp_fb.extra[0] = '\0';
+    return (SDP_SUCCESS);
+}
+
+/* Function:    sdp_attr_set_rtcp_fb_ccm
+ * Description: Sets the value of an rtcp-fb:...ccm attribute
+ * Parameters:  sdp_ptr        The SDP handle returned by sdp_init_description.
+ *              level          The level to set the attribute.
+ *              payload_type   The value to set the payload type to for
+ *                             this attribute. Can be SDP_ALL_PAYLOADS.
+ *              inst_num       The attribute instance number to check.
+ *              type           The ccm type to indicate
+ * Returns:     SDP_SUCCESS            Attribute param was set successfully.
+ *              SDP_INVALID_PARAMETER  Specified attribute is not defined.
+ */
+sdp_result_e
+sdp_attr_set_rtcp_fb_ccm(void *sdp_ptr, u16 level, u16 payload_type, u16 inst,
+                         sdp_rtcp_fb_ccm_type_e type)
+{
+    sdp_t       *sdp_p = (sdp_t *)sdp_ptr;
+    sdp_attr_t  *attr_p;
+
+    if (!sdp_verify_sdp_ptr(sdp_p)) {
+        return (SDP_INVALID_SDP_PTR);
+    }
+
+    attr_p = sdp_find_attr(sdp_p, level, 0, SDP_ATTR_RTCP_FB, inst);
+    if (!attr_p) {
+        if (sdp_p->debug_flag[SDP_DEBUG_ERRORS]) {
+            CSFLogError(logTag, "%s rtcp_fb ccm attribute, level %u "
+                      "instance %u not found.", sdp_p->debug_str, level,
+                      inst);
+        }
+        sdp_p->conf_p->num_invalid_param++;
+        return (SDP_INVALID_PARAMETER);
+    }
+
+    attr_p->attr.rtcp_fb.payload_num = payload_type;
+    attr_p->attr.rtcp_fb.feedback_type = SDP_RTCP_FB_CCM;
+    attr_p->attr.rtcp_fb.param.ccm = type;
+    attr_p->attr.rtcp_fb.extra[0] = '\0';
+    return (SDP_SUCCESS);
+}
--- a/media/webrtc/signaling/src/sipcc/core/sdp/sdp_main.c
+++ b/media/webrtc/signaling/src/sipcc/core/sdp/sdp_main.c
@@ -162,17 +162,19 @@ const sdp_attrarray_t sdp_attr[SDP_MAX_A
       sdp_parse_attr_ice_attr, sdp_build_attr_ice_attr },
     {"ice-pwd", sizeof("ice-pwd"),
       sdp_parse_attr_ice_attr, sdp_build_attr_ice_attr},
     {"rtcp-mux", sizeof("rtcp-mux"),
       sdp_parse_attr_rtcp_mux_attr, sdp_build_attr_rtcp_mux_attr},
     {"fingerprint", sizeof("fingerprint"),
       sdp_parse_attr_fingerprint_attr, sdp_build_attr_simple_string},
     {"maxptime", sizeof("maxptime"),
-      sdp_parse_attr_simple_u32, sdp_build_attr_simple_u32}
+      sdp_parse_attr_simple_u32, sdp_build_attr_simple_u32},
+    {"rtcp-fb", sizeof("rtcp-fb"),
+      sdp_parse_attr_rtcp_fb, sdp_build_attr_rtcp_fb}
 };
 
 /* Note: These *must* be in the same order as the enum types. */
 const sdp_namearray_t sdp_media[SDP_MAX_MEDIA_TYPES] =
 {
     {"audio",        sizeof("audio")},
     {"video",        sizeof("video")},
     {"application",  sizeof("application")},
@@ -436,16 +438,57 @@ const sdp_namearray_t sdp_src_filter_mod
 
 /* Maintain the same order as defined in typdef sdp_rtcp_unicast_mode_e */
 const sdp_namearray_t sdp_rtcp_unicast_mode_val[SDP_RTCP_MAX_UNICAST_MODE] =
 {
     {"reflection", sizeof("reflection")},
     {"rsi",        sizeof("rsi")}
 };
 
+#define SDP_NAME(x) {x, sizeof(x)}
+/* Maintain the same order as defined in typdef sdp_rtcp_fb_type_e */
+const sdp_namearray_t sdp_rtcp_fb_type_val[SDP_MAX_RTCP_FB] =
+{
+    SDP_NAME("ack"),
+    SDP_NAME("ccm"),
+    SDP_NAME("nack"),
+    SDP_NAME("trr-int")
+};
+
+/* Maintain the same order as defined in typdef sdp_rtcp_fb_nack_type_e */
+const sdp_namearray_t sdp_rtcp_fb_nack_type_val[SDP_MAX_RTCP_FB_NACK] =
+{
+    SDP_NAME(""),
+    SDP_NAME("sli"),
+    SDP_NAME("pli"),
+    SDP_NAME("rpsi"),
+    SDP_NAME("app"),
+    SDP_NAME("rai"),
+    SDP_NAME("tllei"),
+    SDP_NAME("pslei"),
+    SDP_NAME("ecn")
+};
+
+/* Maintain the same order as defined in typdef sdp_rtcp_fb_ack_type_e */
+const sdp_namearray_t sdp_rtcp_fb_ack_type_val[SDP_MAX_RTCP_FB_ACK] =
+{
+    SDP_NAME("rpsi"),
+    SDP_NAME("app")
+};
+
+/* Maintain the same order as defined in typdef sdp_rtcp_fb_ccm_type_e */
+const sdp_namearray_t sdp_rtcp_fb_ccm_type_val[SDP_MAX_RTCP_FB_CCM] =
+{
+    SDP_NAME("fir"),
+    SDP_NAME("tmmbr"),
+    SDP_NAME("tstr"),
+    SDP_NAME("vbcm")
+};
+
+
 /*  Maintain same order as defined in typedef sdp_srtp_crypto_suite_t */
 const sdp_srtp_crypto_suite_list sdp_srtp_crypto_suite_array[SDP_SRTP_MAX_NUM_CRYPTO_SUITES] =
 {
   {SDP_SRTP_UNKNOWN_CRYPTO_SUITE, UNKNOWN_CRYPTO_SUITE, 0, 0},
   {SDP_SRTP_AES_CM_128_HMAC_SHA1_32, AES_CM_128_HMAC_SHA1_32,
       SDP_SRTP_AES_CM_128_HMAC_SHA1_32_KEY_BYTES,
       SDP_SRTP_AES_CM_128_HMAC_SHA1_32_SALT_BYTES},
   {SDP_SRTP_AES_CM_128_HMAC_SHA1_80, AES_CM_128_HMAC_SHA1_80,
--- a/media/webrtc/signaling/src/sipcc/core/sdp/sdp_private.h
+++ b/media/webrtc/signaling/src/sipcc/core/sdp/sdp_private.h
@@ -28,16 +28,21 @@ extern const sdp_namearray_t sdp_fmtp_co
 extern const sdp_namearray_t sdp_fmtp_codec_param_val[];
 extern const sdp_namearray_t sdp_silencesupp_pref[];
 extern const sdp_namearray_t sdp_silencesupp_siduse[];
 extern const sdp_namearray_t sdp_srtp_context_crypto_suite[];
 extern const sdp_namearray_t sdp_bw_modifier_val[];
 extern const sdp_namearray_t sdp_group_attr_val[];
 extern const sdp_namearray_t sdp_src_filter_mode_val[];
 extern const sdp_namearray_t sdp_rtcp_unicast_mode_val[];
+extern const sdp_namearray_t sdp_rtcp_fb_type_val[];
+extern const sdp_namearray_t sdp_rtcp_fb_nack_type_val[];
+extern const sdp_namearray_t sdp_rtcp_fb_ack_type_val[];
+extern const sdp_namearray_t sdp_rtcp_fb_ccm_type_val[];
+
 
 extern const  sdp_srtp_crypto_suite_list sdp_srtp_crypto_suite_array[];
 /* Function Prototypes */
 
 /* sdp_access.c */
 extern sdp_mca_t *sdp_find_media_level(sdp_t *sdp_p, u16 level);
 extern sdp_bw_data_t* sdp_find_bw_line (void *sdp_ptr, u16 level, u16 inst_num);
 
@@ -133,16 +138,22 @@ extern sdp_result_e sdp_build_attr_silen
                                                sdp_attr_t *attr_p,
                                                flex_string *fs);
 extern sdp_result_e sdp_parse_attr_srtpcontext(sdp_t *sdp_p,
                                                sdp_attr_t *attr_p,
                                                const char *ptr);
 extern sdp_result_e sdp_build_attr_srtpcontext(sdp_t *sdp_p,
                                                sdp_attr_t *attr_p,
                                                flex_string *fs);
+extern sdp_result_e sdp_parse_attr_rtcp_fb(sdp_t *sdp_p,
+                                           sdp_attr_t *attr_p,
+                                           const char *ptr);
+extern sdp_result_e sdp_build_attr_rtcp_fb(sdp_t *sdp_p,
+                                           sdp_attr_t *attr_p,
+                                           flex_string *fs);
 extern sdp_result_e sdp_parse_attr_mptime(
     sdp_t *sdp_p, sdp_attr_t *attr_p, const char *ptr);
 extern sdp_result_e sdp_build_attr_mptime(
     sdp_t *sdp_p, sdp_attr_t *attr_p, flex_string *fs);
 
 extern sdp_result_e sdp_parse_attr_x_sidin(
     sdp_t *sdp_p, sdp_attr_t *attr_p, const char *ptr);
 extern sdp_result_e sdp_build_attr_x_sidin(
--- a/media/webrtc/signaling/src/sipcc/include/ccsdp.h
+++ b/media/webrtc/signaling/src/sipcc/include/ccsdp.h
@@ -236,16 +236,17 @@ typedef enum {
     SDP_ATTR_LABEL,
     SDP_ATTR_FRAMERATE,
     SDP_ATTR_ICE_CANDIDATE,
     SDP_ATTR_ICE_UFRAG,
     SDP_ATTR_ICE_PWD,
     SDP_ATTR_RTCP_MUX,
     SDP_ATTR_DTLS_FINGERPRINT,
     SDP_ATTR_MAXPTIME,
+    SDP_ATTR_RTCP_FB,  /* RFC 4585 */
     SDP_MAX_ATTR_TYPES,
     SDP_ATTR_INVALID
 } sdp_attr_e;
 
 /**
  * Gets the value of the fmtp attribute- parameter-sets parameter for H.264 codec
  *
  * @param[in]  sdp_handle     The SDP handle
--- a/media/webrtc/signaling/test/Makefile.in
+++ b/media/webrtc/signaling/test/Makefile.in
@@ -124,32 +124,38 @@ endif
 
 LOCAL_INCLUDES += \
  -I. \
  -I$(topsrcdir)/media/webrtc/trunk/testing/gtest/include \
  -I$(topsrcdir)/ipc/chromium/src \
  -I$(topsrcdir)/media/mtransport \
  -I$(topsrcdir)/media/mtransport/test \
  -I$(topsrcdir)/media/webrtc/signaling/include \
+ -I$(topsrcdir)/media/webrtc/signaling/src/sipcc/core/sdp \
+ -I$(topsrcdir)/media/webrtc/signaling/src/sipcc/cpr/include \
+ -I$(topsrcdir)/media/webrtc/signaling/src/sipcc/core/includes \
+ -I$(topsrcdir)/media/webrtc/signaling/src/common/browser_logging \
+ -I$(topsrcdir)/media/webrtc/signaling/src/media \
  -I$(topsrcdir)/media/webrtc/signaling/src/media-conduit \
  -I$(topsrcdir)/media/webrtc/signaling/src/mediapipeline \
  -I$(topsrcdir)/media/webrtc/signaling/src/sipcc/include \
  -I$(topsrcdir)/media/webrtc/signaling/src/peerconnection \
  -I$(topsrcdir)/media/webrtc/signaling/media-conduit\
  -I$(topsrcdir)/media/webrtc/trunk/third_party/libjingle/source/ \
  -I$(topsrcdir)/xpcom/base/ \
  $(NULL)
 
 ifneq ($(OS_TARGET),WINNT)
 ifneq (gonk,$(MOZ_WIDGET_TOOLKIT))
 ifdef JS_SHARED_LIBRARY
 LIBS += $(MOZ_ZLIB_LIBS)
 endif
 
 CPP_UNIT_TESTS = \
+  sdp_unittests.cpp \
   signaling_unittests.cpp \
   mediapipeline_unittest.cpp \
   mediaconduit_unittests.cpp \
   $(NULL)
 endif
 endif
 
 include $(topsrcdir)/config/config.mk
new file mode 100644
--- /dev/null
+++ b/media/webrtc/signaling/test/sdp_unittests.cpp
@@ -0,0 +1,479 @@
+/* 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 <string>
+
+#define GTEST_HAS_RTTI 0
+#include "gtest/gtest.h"
+#include "gtest_utils.h"
+
+#include "nspr.h"
+#include "nss.h"
+
+#include "FakeMediaStreams.h"
+#include "FakeMediaStreamsImpl.h"
+#include "PeerConnectionImpl.h"
+#include "PeerConnectionCtx.h"
+
+#include "mtransport_test_utils.h"
+MtransportTestUtils *test_utils;
+nsCOMPtr<nsIThread> gThread;
+
+extern "C" {
+#include "sdp.h"
+#include "sdp_private.h"
+}
+
+namespace test {
+
+static bool SetupGlobalThread() {
+  if (!gThread) {
+    nsIThread *thread;
+
+    nsresult rv = NS_NewNamedThread("pseudo-main",&thread);
+    if (NS_FAILED(rv))
+      return false;
+
+    gThread = thread;
+    sipcc::PeerConnectionCtx::InitializeGlobal(gThread);
+  }
+  return true;
+}
+
+class SdpTest : public ::testing::Test {
+  public:
+    SdpTest() : sdp_ptr_(nullptr) {
+      sdp_media_e supported_media[] = {
+        SDP_MEDIA_AUDIO,
+        SDP_MEDIA_VIDEO,
+        SDP_MEDIA_APPLICATION,
+        SDP_MEDIA_DATA,
+        SDP_MEDIA_CONTROL,
+        SDP_MEDIA_NAS_RADIUS,
+        SDP_MEDIA_NAS_TACACS,
+        SDP_MEDIA_NAS_DIAMETER,
+        SDP_MEDIA_NAS_L2TP,
+        SDP_MEDIA_NAS_LOGIN,
+        SDP_MEDIA_NAS_NONE,
+        SDP_MEDIA_IMAGE,
+      };
+
+      config_p_ = sdp_init_config();
+      unsigned int i;
+      for (i = 0; i < sizeof(supported_media) / sizeof(sdp_media_e); i++) {
+        sdp_media_supported(config_p_, supported_media[i], true);
+      }
+      sdp_nettype_supported(config_p_, SDP_NT_INTERNET, true);
+      sdp_addrtype_supported(config_p_, SDP_AT_IP4, true);
+      sdp_addrtype_supported(config_p_, SDP_AT_IP6, true);
+      sdp_transport_supported(config_p_, SDP_TRANSPORT_RTPSAVPF, true);
+      sdp_transport_supported(config_p_, SDP_TRANSPORT_UDPTL, true);
+      sdp_require_session_name(config_p_, false);
+    }
+
+    static void SetUpTestCase() {
+      ASSERT_TRUE(SetupGlobalThread());
+    }
+
+    void SetUp() {
+      final_level_ = 0;
+      sdp_ptr_ = nullptr;
+    }
+
+    static void TearDownTestCase() {
+      gThread = nullptr;
+    }
+
+    void ResetSdp() {
+      if (!sdp_ptr_) {
+        sdp_free_description(sdp_ptr_);
+      }
+      sdp_ptr_ = sdp_init_description("BogusPeerConnectionId", config_p_);
+    }
+
+    void ParseSdp(const std::string &sdp_str) {
+      char *bufp = const_cast<char *>(sdp_str.data());
+      ResetSdp();
+      ASSERT_EQ(sdp_parse(sdp_ptr_, &bufp, sdp_str.size()), SDP_SUCCESS);
+    }
+
+    void InitLocalSdp() {
+      ResetSdp();
+      ASSERT_EQ(sdp_set_version(sdp_ptr_, 0), SDP_SUCCESS);
+      ASSERT_EQ(sdp_set_owner_username(sdp_ptr_, "-"), SDP_SUCCESS);
+      ASSERT_EQ(sdp_set_owner_sessionid(sdp_ptr_, "132954853"), SDP_SUCCESS);
+      ASSERT_EQ(sdp_set_owner_version(sdp_ptr_, "0"), SDP_SUCCESS);
+      ASSERT_EQ(sdp_set_owner_network_type(sdp_ptr_, SDP_NT_INTERNET),
+                SDP_SUCCESS);
+      ASSERT_EQ(sdp_set_owner_address_type(sdp_ptr_, SDP_AT_IP4), SDP_SUCCESS);
+      ASSERT_EQ(sdp_set_owner_address(sdp_ptr_, "198.51.100.7"), SDP_SUCCESS);
+      ASSERT_EQ(sdp_set_session_name(sdp_ptr_, "SDP Unit Test"), SDP_SUCCESS);
+      ASSERT_EQ(sdp_set_time_start(sdp_ptr_, "0"), SDP_SUCCESS);
+      ASSERT_EQ(sdp_set_time_stop(sdp_ptr_, "0"), SDP_SUCCESS);
+    }
+
+    std::string SerializeSdp() {
+      flex_string fs;
+      flex_string_init(&fs);
+      EXPECT_EQ(sdp_build(sdp_ptr_, &fs), SDP_SUCCESS);
+      std::string body(fs.buffer);
+      flex_string_free(&fs);
+      return body;
+    }
+
+    // Returns "level" for new media section
+    int AddNewMedia(sdp_media_e type) {
+      final_level_++;
+      EXPECT_EQ(sdp_insert_media_line(sdp_ptr_, final_level_), SDP_SUCCESS);
+      EXPECT_EQ(sdp_set_conn_nettype(sdp_ptr_, final_level_, SDP_NT_INTERNET),
+                SDP_SUCCESS);
+      EXPECT_EQ(sdp_set_conn_addrtype(sdp_ptr_, final_level_, SDP_AT_IP4),
+                SDP_SUCCESS);
+      EXPECT_EQ(sdp_set_conn_address(sdp_ptr_, final_level_, "198.51.100.7"),
+                SDP_SUCCESS);
+      EXPECT_EQ(sdp_set_media_type(sdp_ptr_, final_level_, SDP_MEDIA_VIDEO),
+                SDP_SUCCESS);
+      EXPECT_EQ(sdp_set_media_transport(sdp_ptr_, final_level_,
+                                        SDP_TRANSPORT_RTPAVP),
+                SDP_SUCCESS);
+      EXPECT_EQ(sdp_set_media_portnum(sdp_ptr_, final_level_, 12345, 0),
+                SDP_SUCCESS);
+      EXPECT_EQ(sdp_add_media_payload_type(sdp_ptr_, final_level_, 120,
+                                           SDP_PAYLOAD_NUMERIC),
+                SDP_SUCCESS);
+      return final_level_;
+    }
+
+    u16 AddNewRtcpFbAck(int level, sdp_rtcp_fb_ack_type_e type,
+                         u16 payload = SDP_ALL_PAYLOADS) {
+      u16 inst_num = 0;
+      EXPECT_EQ(sdp_add_new_attr(sdp_ptr_, level, 0, SDP_ATTR_RTCP_FB,
+                                 &inst_num), SDP_SUCCESS);
+      EXPECT_EQ(sdp_attr_set_rtcp_fb_ack(sdp_ptr_, level, payload, inst_num,
+                                         type), SDP_SUCCESS);
+      return inst_num;
+    }
+
+    u16 AddNewRtcpFbNack(int level, sdp_rtcp_fb_nack_type_e type,
+                         u16 payload = SDP_ALL_PAYLOADS) {
+      u16 inst_num = 0;
+      EXPECT_EQ(sdp_add_new_attr(sdp_ptr_, level, 0, SDP_ATTR_RTCP_FB,
+                                 &inst_num), SDP_SUCCESS);
+      EXPECT_EQ(sdp_attr_set_rtcp_fb_nack(sdp_ptr_, level, payload, inst_num,
+                                          type), SDP_SUCCESS);
+      return inst_num;
+    }
+
+    u16 AddNewRtcpTrrInt(int level, u32 interval,
+                         u16 payload = SDP_ALL_PAYLOADS) {
+      u16 inst_num = 0;
+      EXPECT_EQ(sdp_add_new_attr(sdp_ptr_, level, 0, SDP_ATTR_RTCP_FB,
+                                 &inst_num), SDP_SUCCESS);
+      EXPECT_EQ(sdp_attr_set_rtcp_fb_trr_int(sdp_ptr_, level, payload, inst_num,
+                                             interval), SDP_SUCCESS);
+      return inst_num;
+    }
+
+    u16 AddNewRtcpFbCcm(int level, sdp_rtcp_fb_ccm_type_e type,
+                         u16 payload = SDP_ALL_PAYLOADS) {
+      u16 inst_num = 0;
+      EXPECT_EQ(sdp_add_new_attr(sdp_ptr_, level, 0, SDP_ATTR_RTCP_FB,
+                                 &inst_num), SDP_SUCCESS);
+      EXPECT_EQ(sdp_attr_set_rtcp_fb_ccm(sdp_ptr_, level, payload, inst_num,
+                                         type), SDP_SUCCESS);
+      return inst_num;
+    }
+
+  protected:
+    int final_level_;
+    void *config_p_;
+    sdp_t *sdp_ptr_;
+};
+
+static const std::string kVideoSdp =
+  "v=0\r\n"
+  "o=- 137331303 2 IN IP4 127.0.0.1\r\n"
+  "s=SIP Call\r\n"
+  "t=0 0\r\n"
+  "m=video 56436 RTP/SAVPF 120\r\n"
+  "c=IN IP4 198.51.100.7\r\n"
+  "a=rtpmap:120 VP8/90000\r\n";
+
+TEST_F(SdpTest, parseRtcpFbAckRpsi) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 ack rpsi\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ack(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_ACK_RPSI);
+}
+
+TEST_F(SdpTest, parseRtcpFbAckApp) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 ack app\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ack(sdp_ptr_, 1, 120, 1), SDP_RTCP_FB_ACK_APP);
+}
+
+TEST_F(SdpTest, parseRtcpFbAckAppFoo) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 ack app foo\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ack(sdp_ptr_, 1, 120, 1), SDP_RTCP_FB_ACK_APP);
+}
+
+TEST_F(SdpTest, parseRtcpFbAckFooBar) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 ack foo bar\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ack(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_ACK_UNKNOWN);
+}
+
+TEST_F(SdpTest, parseRtcpFbAckFooBarBaz) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 ack foo bar baz\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ack(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_ACK_UNKNOWN);
+}
+
+TEST_F(SdpTest, parseRtcpFbNack) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 nack\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_NACK_UNSPECIFIED);
+}
+
+TEST_F(SdpTest, parseRtcpFbNackPli) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 nack pli\r\n");
+}
+
+TEST_F(SdpTest, parseRtcpFbNackSli) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 nack sli\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_NACK_SLI);
+}
+
+TEST_F(SdpTest, parseRtcpFbNackRpsi) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 nack rpsi\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_NACK_RPSI);
+}
+
+TEST_F(SdpTest, parseRtcpFbNackApp) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 nack app\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_NACK_APP);
+}
+
+TEST_F(SdpTest, parseRtcpFbNackAppFoo) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 nack app foo\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_NACK_APP);
+}
+
+TEST_F(SdpTest, parseRtcpFbNackAppFooBar) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 nack app foo bar\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_NACK_APP);
+}
+
+TEST_F(SdpTest, parseRtcpFbNackFooBarBaz) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 nack foo bar baz\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_NACK_UNKNOWN);
+}
+
+TEST_F(SdpTest, parseRtcpFbTrrInt0) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 trr-int 0\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_trr_int(sdp_ptr_, 1, 120, 1), 0U);
+}
+
+TEST_F(SdpTest, parseRtcpFbTrrInt123) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 trr-int 123\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_trr_int(sdp_ptr_, 1, 120, 1), 123U);
+}
+
+TEST_F(SdpTest, parseRtcpFbCcmFir) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 ccm fir\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 1), SDP_RTCP_FB_CCM_FIR);
+}
+
+TEST_F(SdpTest, parseRtcpFbCcmTmmbr) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 ccm tmmbr\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_CCM_TMMBR);
+}
+
+TEST_F(SdpTest, parseRtcpFbCcmTmmbrSmaxpr) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 ccm tmmbr smaxpr=456\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_CCM_TMMBR);
+}
+
+TEST_F(SdpTest, parseRtcpFbCcmTstr) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 ccm tstr\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_CCM_TSTR);
+}
+
+TEST_F(SdpTest, parseRtcpFbCcmVbcm) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 ccm vbcm 123 456 789\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 1),
+                                     SDP_RTCP_FB_CCM_VBCM);
+  // We don't currently parse out VBCM submessage types, since we don't have
+  // any use for them.
+}
+
+TEST_F(SdpTest, parseRtcpFbCcmFoo) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 ccm foo\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_CCM_UNKNOWN);
+}
+
+TEST_F(SdpTest, parseRtcpFbCcmFooBarBaz) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 ccm foo bar baz\r\n");
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_CCM_UNKNOWN);
+}
+
+TEST_F(SdpTest, parseRtcpFbFoo) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 foo\r\n");
+}
+
+TEST_F(SdpTest, parseRtcpFbFooBar) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 foo bar\r\n");
+}
+
+TEST_F(SdpTest, parseRtcpFbFooBarBaz) {
+  ParseSdp(kVideoSdp + "a=rtcp-fb:120 foo bar baz\r\n");
+}
+
+
+TEST_F(SdpTest, parseRtcpFbKitchenSink) {
+  ParseSdp(kVideoSdp +
+    "a=rtcp-fb:120 ack rpsi\r\n"
+    "a=rtcp-fb:120 ack app\r\n"
+    "a=rtcp-fb:120 ack app foo\r\n"
+    "a=rtcp-fb:120 ack foo bar\r\n"
+    "a=rtcp-fb:120 ack foo bar baz\r\n"
+    "a=rtcp-fb:120 nack\r\n"
+    "a=rtcp-fb:120 nack pli\r\n"
+    "a=rtcp-fb:120 nack sli\r\n"
+    "a=rtcp-fb:120 nack rpsi\r\n"
+    "a=rtcp-fb:120 nack app\r\n"
+    "a=rtcp-fb:120 nack app foo\r\n"
+    "a=rtcp-fb:120 nack app foo bar\r\n"
+    "a=rtcp-fb:120 nack foo bar baz\r\n"
+    "a=rtcp-fb:120 trr-int 0\r\n"
+    "a=rtcp-fb:120 trr-int 123\r\n"
+    "a=rtcp-fb:120 ccm fir\r\n"
+    "a=rtcp-fb:120 ccm tmmbr\r\n"
+    "a=rtcp-fb:120 ccm tmmbr smaxpr=456\r\n"
+    "a=rtcp-fb:120 ccm tstr\r\n"
+    "a=rtcp-fb:120 ccm vbcm 123 456 789\r\n"
+    "a=rtcp-fb:120 ccm foo\r\n"
+    "a=rtcp-fb:120 ccm foo bar baz\r\n"
+    "a=rtcp-fb:120 foo\r\n"
+    "a=rtcp-fb:120 foo bar\r\n"
+    "a=rtcp-fb:120 foo bar baz\r\n");
+
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ack(sdp_ptr_, 1, 120, 1), SDP_RTCP_FB_ACK_RPSI);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ack(sdp_ptr_, 1, 120, 2), SDP_RTCP_FB_ACK_APP);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ack(sdp_ptr_, 1, 120, 3), SDP_RTCP_FB_ACK_APP);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ack(sdp_ptr_, 1, 120, 4),
+            SDP_RTCP_FB_ACK_UNKNOWN);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ack(sdp_ptr_, 1, 120, 5),
+            SDP_RTCP_FB_ACK_UNKNOWN);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ack(sdp_ptr_, 1, 120, 6),
+            SDP_RTCP_FB_ACK_NOT_FOUND);
+
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 1),
+            SDP_RTCP_FB_NACK_UNSPECIFIED);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 2),
+            SDP_RTCP_FB_NACK_PLI);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 3),
+            SDP_RTCP_FB_NACK_SLI);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 4),
+            SDP_RTCP_FB_NACK_RPSI);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 5),
+            SDP_RTCP_FB_NACK_APP);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 6),
+            SDP_RTCP_FB_NACK_APP);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 7),
+            SDP_RTCP_FB_NACK_APP);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 8),
+            SDP_RTCP_FB_NACK_UNKNOWN);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_nack(sdp_ptr_, 1, 120, 9),
+            SDP_RTCP_FB_NACK_NOT_FOUND);
+
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_trr_int(sdp_ptr_, 1, 120, 1), 0U);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_trr_int(sdp_ptr_, 1, 120, 2), 123U);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_trr_int(sdp_ptr_, 1, 120, 3), 0xFFFFFFFF);
+
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 1), SDP_RTCP_FB_CCM_FIR);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 2),
+            SDP_RTCP_FB_CCM_TMMBR);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 3),
+            SDP_RTCP_FB_CCM_TMMBR);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 4),
+            SDP_RTCP_FB_CCM_TSTR);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 5),
+            SDP_RTCP_FB_CCM_VBCM);
+  // We don't currently parse out VBCM submessage types, since we don't have
+  // any use for them.
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 6),
+            SDP_RTCP_FB_CCM_UNKNOWN);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 7),
+            SDP_RTCP_FB_CCM_UNKNOWN);
+  ASSERT_EQ(sdp_attr_get_rtcp_fb_ccm(sdp_ptr_, 1, 120, 8),
+            SDP_RTCP_FB_CCM_NOT_FOUND);
+}
+
+
+/* TODO (abr@mozilla.com) These attribute adding test cases definitely need
+   beefing up; for now, I'm testing the two use cases that we know
+   we need right now.  An exhaustive check of the various permutations
+   will look similar to the parsing tests, above */
+
+TEST_F(SdpTest, addRtcpFbNack) {
+  InitLocalSdp();
+  int level = AddNewMedia(SDP_MEDIA_VIDEO);
+  AddNewRtcpFbNack(level, SDP_RTCP_FB_NACK_UNSPECIFIED, 120);
+  std::string body = SerializeSdp();
+  ASSERT_NE(body.find("a=rtcp-fb:120 nack\r\n"), std::string::npos);
+}
+
+TEST_F(SdpTest, addRtcpFbNackAllPt) {
+  InitLocalSdp();
+  int level = AddNewMedia(SDP_MEDIA_VIDEO);
+  AddNewRtcpFbNack(level, SDP_RTCP_FB_NACK_UNSPECIFIED);
+  std::string body = SerializeSdp();
+  ASSERT_NE(body.find("a=rtcp-fb:* nack\r\n"), std::string::npos);
+}
+
+
+TEST_F(SdpTest, addRtcpFbCcmFir) {
+  InitLocalSdp();
+  int level = AddNewMedia(SDP_MEDIA_VIDEO);
+  AddNewRtcpFbCcm(level, SDP_RTCP_FB_CCM_FIR, 120);
+  std::string body = SerializeSdp();
+  ASSERT_NE(body.find("a=rtcp-fb:120 ccm fir\r\n"), std::string::npos);
+}
+
+TEST_F(SdpTest, addRtcpFbCcmFirAllPt) {
+  InitLocalSdp();
+  int level = AddNewMedia(SDP_MEDIA_VIDEO);
+  AddNewRtcpFbCcm(level, SDP_RTCP_FB_CCM_FIR);
+  std::string body = SerializeSdp();
+  ASSERT_NE(body.find("a=rtcp-fb:* ccm fir\r\n"), std::string::npos);
+}
+
+/* TODO We need to test the pt=* use cases. */
+
+} // End namespace test.
+
+int main(int argc, char **argv) {
+  test_utils = new MtransportTestUtils();
+  NSS_NoDB_Init(NULL);
+  NSS_SetDomesticPolicy();
+
+  ::testing::InitGoogleTest(&argc, argv);
+  int result = RUN_ALL_TESTS();
+
+  delete test_utils;
+
+  return result;
+}
--- a/media/webrtc/signaling/test/signaling_unittests.cpp
+++ b/media/webrtc/signaling/test/signaling_unittests.cpp
@@ -1999,18 +1999,26 @@ TEST_F(SignalingTest, CheckTrickleSdpCha
   ASSERT_NE(a1_.getLocalDescription().find("\r\na=candidate"),
             std::string::npos);
   ASSERT_NE(a1_.getRemoteDescription().find("\r\na=candidate"),
             std::string::npos);
   ASSERT_NE(a2_.getLocalDescription().find("\r\na=candidate"),
             std::string::npos);
   ASSERT_NE(a2_.getRemoteDescription().find("\r\na=candidate"),
             std::string::npos);
+  /* TODO (abr): These checks aren't quite right, since trickle ICE
+   * can easily result in SDP that is semantically identical but
+   * varies syntactically (in particularly, the ordering of attributes
+   * withing an m-line section can be different). This needs to be updated
+   * to be a semantic comparision between the SDP. Currently, these checks
+   * will fail whenever we add any other attributes to the SDP, such as
+   * RTCP MUX or RTCP feedback.
   ASSERT_EQ(a1_.getLocalDescription(),a2_.getRemoteDescription());
   ASSERT_EQ(a2_.getLocalDescription(),a1_.getRemoteDescription());
+  */
 }
 
 TEST_F(SignalingTest, ipAddrAnyOffer)
 {
   sipcc::MediaConstraints constraints;
   std::string offer =
     "v=0\r\n"
     "o=- 1 1 IN IP4 127.0.0.1\r\n"