Bug 880067 - Part 3: SDP negotiation of rtcp-fb r=ehugg
authorAdam Roach [:abr] <adam@nostrum.com>
Thu, 22 Aug 2013 13:18:38 -0500
changeset 144114 610f5f410e3bedb02a4798a4aba1f8daa417b298
parent 144113 1916191a311c99a476a4695f136ecc2ac805c804
child 144115 497a75a9a15f3a43462737105d4d8c3c96bb5593
push id25150
push userryanvm@gmail.com
push dateFri, 23 Aug 2013 21:49:29 +0000
treeherdermozilla-central@17143a9a0d83 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehugg
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 3: SDP negotiation of rtcp-fb r=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/include/vcm.h
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
@@ -1,9 +1,11 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+ * vim: set ts=8 sts=4 et sw=4 tw=99:
+ * 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 "cpr_in.h"
 #include "cpr_rand.h"
 #include "cpr_stdlib.h"
 #include "lsm.h"
 #include "fsm.h"
@@ -4397,16 +4399,199 @@ fsmdef_media_t* gsmsdp_find_media_by_med
             /* found a match */
             return (media);
         }
     }
     return (NULL);
 }
 
 /*
+ * gsmsdp_add_rtcp_fb
+ *
+ * Description:
+ *  Adds a=rtcp-fb attributes to local SDP for supported video codecs
+ *
+ * Parameters:
+ *   level - SDP media level (between 1 and the # of m-lines)
+ *   sdp_p - pointer to local SDP
+ *   codec - the codec type that the attributes should be added for
+ *   types - a bitmask of rtcp-fb types to add, taken from
+ *           sdp_rtcp_fb_bitmask_e
+ *
+ * returns
+ *  CC_CAUSE_OK - success
+ *  any other code - failure
+ */
+cc_causes_t
+gsmsdp_add_rtcp_fb (int level, sdp_t *sdp_p,
+                    rtp_ptype codec, unsigned int types)
+{
+    int num_pts;
+    int pt_codec;
+    sdp_payload_ind_e indicator;
+
+    int pt_index;
+    unsigned int j;
+    num_pts = sdp_get_media_num_payload_types(sdp_p, level);
+    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)) {
+                    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)) {
+                    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)) {
+                    gsmsdp_set_rtcp_fb_ccm_attribute(level, sdp_p, pt, j);
+                }
+            }
+
+        }
+    }
+    return CC_CAUSE_OK;
+}
+
+
+/*
+ * gsmsdp_negotiate_rtcp_fb
+ *
+ * Description:
+ *  Negotiates a=rtcp-fb attributes to local SDP for supported video codecs
+ *
+ * Parameters:
+ *   cc_sdp_p - local and remote SDP
+ *   media    - The media structure for the current level to be negotiated
+ *   offer    - True if the remote SDP is an offer
+ *
+ * returns
+ *  CC_CAUSE_OK - success
+ *  any other code - failure
+ */
+cc_causes_t
+gsmsdp_negotiate_rtcp_fb (cc_sdp_t *cc_sdp_p,
+                          fsmdef_media_t *media,
+                          boolean offer)
+{
+    int level = media->level;
+    int pt_codec;
+    int remote_pt;
+    sdp_payload_ind_e indicator;
+    int pt_index, i;
+    sdp_rtcp_fb_nack_type_e nack_type;
+    sdp_rtcp_fb_ack_type_e ack_type;
+    sdp_rtcp_fb_ccm_type_e ccm_type;
+    uint32_t fb_types = 0;
+
+    int num_pts = sdp_get_media_num_payload_types(cc_sdp_p->dest_sdp, level);
+
+    /*
+     * Remove any previously negotiated rtcp-fb attributes from the
+     * local SDP
+     */
+    sdp_result_e result = SDP_SUCCESS;
+    while (result == SDP_SUCCESS) {
+        result = sdp_delete_attr (cc_sdp_p->src_sdp, level, 0,
+                                  SDP_ATTR_RTCP_FB, 1);
+    }
+
+    /*
+     * For each remote payload type, determine what feedback types are
+     * requested.
+     */
+    for (pt_index = 1; pt_index <= num_pts; pt_index++) {
+        int pt_codec = sdp_get_media_payload_type (cc_sdp_p->dest_sdp,
+                                                   level, pt_index, &indicator);
+        int codec = pt_codec & 0xFF;
+        remote_pt = GET_DYN_PAYLOAD_TYPE_VALUE(pt_codec);
+        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);
+            }
+            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);
+            }
+            i++;
+        } while (ack_type != SDP_RTCP_FB_CCM_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);
+            }
+            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);
+                break;
+            default:
+                fb_types = 0;
+        }
+
+        /*
+         * Now, in our local SDP, set rtcp-fb types that both we and the
+         * remote party support
+         */
+        if (fb_types) {
+            gsmsdp_add_rtcp_fb (level, cc_sdp_p->src_sdp, codec, fb_types);
+        }
+
+        /*
+         * Finally, update the media record for this payload type to
+         * reflect the expected feedback types
+         */
+        for (i = 0; i < media->num_payloads; i++) {
+            if (media->payloads[i].remote_rtp_pt == remote_pt) {
+                media->payloads[i].video.rtcp_fb_types = fb_types;
+            }
+        }
+    }
+
+    return CC_CAUSE_OK;
+}
+
+/*
  * gsmsdp_negotiate_media_lines
  *
  * Description:
  *
  * Walk down the media lines provided in the remote sdp. Compare each
  * media line to the corresponding media line in the local sdp. If
  * the media line does not exist in the local sdp, add it. If the media
  * line exists in the local sdp but is different from the remote sdp,
@@ -4726,16 +4911,21 @@ gsmsdp_negotiate_media_lines (fsm_fcb_t 
             } else {
                 /*
                  * Rejecting multicast because direction is not RECVONLY
                  */
                 unsupported_line = TRUE;
                 update_local_ret_value = TRUE;
             }
 
+            /* Negotiate rtcp feedback mechanisms */
+            if (media && media_type == SDP_MEDIA_VIDEO) {
+                gsmsdp_negotiate_rtcp_fb (dcb_p->sdp, media, offer);
+            }
+
             /*
              * Negotiate rtcp-mux
              */
             if(SDP_MEDIA_APPLICATION != media_type) {
               sdp_res = sdp_attr_get_rtcp_mux_attribute(sdp_p->dest_sdp, i,
                                                         0, SDP_ATTR_RTCP_MUX,
                                                         1, &rtcp_mux);
 
@@ -4810,29 +5000,16 @@ 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 && 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);
@@ -5266,16 +5443,24 @@ gsmsdp_add_media_line (fsmdef_dcb_t *dcb
 
         gsmsdp_update_local_sdp_media(dcb_p, dcb_p->sdp, TRUE, media,
                                           media->transport);
 
         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));
+          }
+
           /*
            * wait until here to set ICE candidates as SDP is now initialized
            */
           for (i=0; i<media->candidate_ct; i++) {
             gsmsdp_set_ice_attribute (SDP_ATTR_ICE_CANDIDATE, level, dcb_p->sdp->src_sdp, media->candidatesp[i]);
           }
 
           config_get_value(CFGID_RTCPMUX, &rtcpmux, sizeof(rtcpmux));
@@ -5392,30 +5577,16 @@ 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
@@ -483,17 +483,17 @@ typedef enum {
     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_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,
@@ -514,16 +514,21 @@ typedef enum {
     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;
 
+#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/core/sdp/sdp_attr.c
+++ b/media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr.c
@@ -4779,17 +4779,17 @@ sdp_result_e sdp_build_attr_rtcp_fb(sdp_
         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
+            if (attr_p->attr.rtcp_fb.param.nack > SDP_RTCP_FB_NACK_BASIC
                 && 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);
@@ -4930,17 +4930,17 @@ sdp_result_e sdp_parse_attr_rtcp_fb (sdp
         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;
+                rtcp_fb_p->param.nack = SDP_RTCP_FB_NACK_BASIC;
                 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",
--- a/media/webrtc/signaling/src/sipcc/include/vcm.h
+++ b/media/webrtc/signaling/src/sipcc/include/vcm.h
@@ -180,16 +180,17 @@ typedef struct
       int channels;
       int bitrate;     /* Wire bitrate of RTP packet payloads */
     } audio;
 
     struct
     {
       int width;
       int height;
+      uint32_t rtcp_fb_types;
     } video;
   };
 
   /* Codec-specific parameters */
   union
   {
     struct {
         uint16_t mode;
--- a/media/webrtc/signaling/test/sdp_unittests.cpp
+++ b/media/webrtc/signaling/test/sdp_unittests.cpp
@@ -228,17 +228,17 @@ 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);
+            SDP_RTCP_FB_NACK_BASIC);
 }
 
 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");
@@ -376,17 +376,17 @@ TEST_F(SdpTest, parseRtcpFbKitchenSink) 
   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);
+            SDP_RTCP_FB_NACK_BASIC);
   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);
@@ -452,25 +452,25 @@ TEST_F(SdpTest, addRtcpFbAckAppAllPt) {
   AddNewRtcpFbAck(level, SDP_RTCP_FB_ACK_APP);
   std::string body = SerializeSdp();
   ASSERT_NE(body.find("a=rtcp-fb:* ack app\r\n"), std::string::npos);
 }
 
 TEST_F(SdpTest, addRtcpFbNack) {
   InitLocalSdp();
   int level = AddNewMedia(SDP_MEDIA_VIDEO);
-  AddNewRtcpFbNack(level, SDP_RTCP_FB_NACK_UNSPECIFIED, 120);
+  AddNewRtcpFbNack(level, SDP_RTCP_FB_NACK_BASIC, 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);
+  AddNewRtcpFbNack(level, SDP_RTCP_FB_NACK_BASIC);
   std::string body = SerializeSdp();
   ASSERT_NE(body.find("a=rtcp-fb:* nack\r\n"), std::string::npos);
 }
 
 TEST_F(SdpTest, addRtcpFbNackSli) {
   InitLocalSdp();
   int level = AddNewMedia(SDP_MEDIA_VIDEO);
   AddNewRtcpFbNack(level, SDP_RTCP_FB_NACK_SLI, 120);
--- a/media/webrtc/signaling/test/signaling_unittests.cpp
+++ b/media/webrtc/signaling/test/signaling_unittests.cpp
@@ -1880,16 +1880,18 @@ TEST_F(SignalingTest, ChromeOfferAnswer)
     "a=sendrecv\r\n"
     "a=mid:video\r\n"
     "a=rtcp-mux\r\n"
     "a=crypto:1 AES_CM_128_HMAC_SHA1_80 inline:"
       "RzrYlzpkTsvgYFD1hQqNCzQ7y4emNLKI1tODsjim\r\n"
     "a=rtpmap:100 VP8/90000\r\n"
     "a=rtpmap:101 red/90000\r\n"
     "a=rtpmap:102 ulpfec/90000\r\n"
+    "a=rtcp-fb:100 nack\r\n"
+    "a=rtcp-fb:100 ccm fir\r\n"
     "a=ssrc:3012607008 cname:KIXaNxUlU5DP3fVS\r\n"
     "a=ssrc:3012607008 msid:A5UL339RyGxT7zwgyF12BFqesxkmbUsaycp5 v0\r\n"
     "a=ssrc:3012607008 mslabel:A5UL339RyGxT7zwgyF12BFqesxkmbUsaycp5\r\n"
     "a=ssrc:3012607008 label:A5UL339RyGxT7zwgyF12BFqesxkmbUsaycp5v0\r\n";
 
 
   std::cout << "Setting offer to:" << std::endl << indent(offer) << std::endl;
   a2_.SetRemote(TestObserver::OFFER, offer);