Bug 1043515 - Ignore unknown fmtp values; partially fix unittests to handle H264_P0 disabled. r=ehugg, a=lmandel
authorRandell Jesup <rjesup@jesup.org>
Sun, 27 Jul 2014 20:00:06 -0400
changeset 217361 f7a26b0d0de524c8e80a5b6f90d58369bfaa1230
parent 217360 fafb35ae78924b1102ead13ef8f575244e9a7361
child 217362 1f0ef1b4edd786d206e285c39f7928ccd65da92f
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehugg, lmandel
bugs1043515
milestone33.0a2
Bug 1043515 - Ignore unknown fmtp values; partially fix unittests to handle H264_P0 disabled. r=ehugg, a=lmandel
media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr.c
media/webrtc/signaling/test/signaling_unittests.cpp
--- a/media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr.c
+++ b/media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr.c
@@ -1741,16 +1741,31 @@ sdp_result_e sdp_parse_attr_fmtp (sdp_t 
                     }
                     if (strchr(temp, 'T') !=NULL) {
                         attr_p->attr.fmtp.annex_t = TRUE;
                     }
                     temp=PL_strtok_r(NULL, ";", &strtok_state);
                 }
             } /* if (temp) */
             done = TRUE;
+        } else {
+          // XXX Note that DTMF fmtp will fall into here:
+          // a=fmtp:101 0-15 (or 0-15,NN,NN etc)
+
+          // unknown parameter - eat chars until ';'
+          CSFLogDebug(logTag, "%s Unknown fmtp type (%s) - ignoring", __FUNCTION__,
+                      tmp);
+          fmtp_ptr = sdp_getnextstrtok(fmtp_ptr, tmp, sizeof(tmp), "; \t",
+                                       &result1);
+          if (result1 != SDP_SUCCESS) {
+            fmtp_ptr = sdp_getnextstrtok(fmtp_ptr, tmp, sizeof(tmp), " \t", &result1);
+            if (result1 != SDP_SUCCESS) {
+              // hmmm, no ; or spaces or tabs; continue on
+            }
+          }
         }
         fmtp_ptr++;
       } else {
           done = TRUE;
       }
     } /* while  - done loop*/
 
     if (codec_info_found) {
--- a/media/webrtc/signaling/test/signaling_unittests.cpp
+++ b/media/webrtc/signaling/test/signaling_unittests.cpp
@@ -3931,47 +3931,55 @@ TEST_F(SignalingTest, MaxFsFrCallerCodec
 TEST_F(SignalingTest, ValidateMultipleVideoCodecsInOffer)
 {
   EnsureInit();
   sipcc::OfferOptions options;
 
   a1_->CreateOffer(options, OFFER_AV, SHOULD_SENDRECV_AV);
   std::string offer = a1_->offer();
 
+#ifdef H264_P0_SUPPORTED
   ASSERT_NE(offer.find("RTP/SAVPF 120 126 97"), std::string::npos);
+#else
+  ASSERT_NE(offer.find("RTP/SAVPF 120 126"), std::string::npos);
+#endif
   ASSERT_NE(offer.find("a=rtpmap:120 VP8/90000"), std::string::npos);
   ASSERT_NE(offer.find("a=rtpmap:126 H264/90000"), std::string::npos);
-  ASSERT_NE(offer.find("a=rtpmap:97 H264/90000"), std::string::npos);
   ASSERT_NE(offer.find("a=fmtp:126 profile-level-id="), std::string::npos);
-  ASSERT_NE(offer.find("a=fmtp:97 profile-level-id="), std::string::npos);
   ASSERT_NE(offer.find("a=rtcp-fb:120 nack"), std::string::npos);
   ASSERT_NE(offer.find("a=rtcp-fb:120 nack pli"), std::string::npos);
   ASSERT_NE(offer.find("a=rtcp-fb:120 ccm fir"), std::string::npos);
   ASSERT_NE(offer.find("a=rtcp-fb:126 nack"), std::string::npos);
   ASSERT_NE(offer.find("a=rtcp-fb:126 nack pli"), std::string::npos);
   ASSERT_NE(offer.find("a=rtcp-fb:126 ccm fir"), std::string::npos);
+#ifdef H264_P0_SUPPORTED
+  ASSERT_NE(offer.find("a=rtpmap:97 H264/90000"), std::string::npos);
+  ASSERT_NE(offer.find("a=fmtp:97 profile-level-id="), std::string::npos);
   ASSERT_NE(offer.find("a=rtcp-fb:97 nack"), std::string::npos);
   ASSERT_NE(offer.find("a=rtcp-fb:97 nack pli"), std::string::npos);
   ASSERT_NE(offer.find("a=rtcp-fb:97 ccm fir"), std::string::npos);
+#endif
 }
 
-// Remove VP8 from offer and check that answer negotiates H264 P1 correctly
+// Remove VP8 from offer and check that answer negotiates H264 P1 correctly and ignores unknown params
 TEST_F(SignalingTest, RemoveVP8FromOfferWithP1First)
 {
   EnsureInit();
 
   sipcc::OfferOptions options;
   size_t match;
 
   a1_->CreateOffer(options, OFFER_AV, SHOULD_SENDRECV_AV);
 
   // Remove VP8 from offer
   std::string offer = a1_->offer();
   offer.replace(match = offer.find("RTP/SAVPF 120"),
-    strlen("RTP/SAVPF 126"), "RTP/SAVPF");
+    strlen("RTP/SAVPF 120"), "RTP/SAVPF");
+  offer.replace(match = offer.find("profile-level-id"),
+    strlen("profile-level-id"), "max-foo=1234;profile-level-id");
   ParsedSDP sdpWrapper(offer);
   sdpWrapper.DeleteAllLines("a=rtcp-fb:120");
   sdpWrapper.DeleteLine("a=rtpmap:120");
 
   std::cout << "Modified SDP " << std::endl
             << indent(sdpWrapper.getSdp()) << std::endl;
 
   // P1 should be offered first
@@ -4004,43 +4012,38 @@ TEST_F(SignalingTest, OfferWithH264Befor
 
   sipcc::OfferOptions options;
   size_t match;
 
   a1_->CreateOffer(options, OFFER_AV, SHOULD_SENDRECV_AV);
 
   // Swap VP8 and P1 in offer
   std::string offer = a1_->offer();
+#ifdef H264_P0_SUPPORTED
   offer.replace(match = offer.find("RTP/SAVPF 120 126 97"),
     strlen("RTP/SAVPF 126 120 97"), "RTP/SAVPF 126 120 97");
+#else
+  offer.replace(match = offer.find("RTP/SAVPF 120 126"),
+    strlen("RTP/SAVPF 126 120"), "RTP/SAVPF 126 120");
+#endif
 
   offer.replace(match = offer.find("a=rtpmap:126 H264/90000"),
     strlen("a=rtpmap:120 VP8/90000"), "a=rtpmap:120 VP8/90000");
   offer.replace(match = offer.find("a=rtpmap:120 VP8/90000"),
     strlen("a=rtpmap:126 H264/90000"), "a=rtpmap:126 H264/90000");
 
-  offer.replace(match = offer.find("a=rtcp-fb:126 nack"),
-    strlen("a=rtcp-fb:120 nack"), "a=rtcp-fb:120 nack");
-  offer.replace(match = offer.find("a=rtcp-fb:126 nack pli"),
-    strlen("a=rtcp-fb:120 nack pli"), "a=rtcp-fb:120 nack pli");
-  offer.replace(match = offer.find("a=rtcp-fb:126 ccm fir"),
-    strlen("a=rtcp-fb:120 ccm fir"), "a=rtcp-fb:120 ccm fir");
-
-  offer.replace(match = offer.find("a=rtcp-fb:120 nack"),
-    strlen("a=rtcp-fb:126 nack"), "a=rtcp-fb:126 nack");
-  offer.replace(match = offer.find("a=rtcp-fb:120 nack pli"),
-    strlen("a=rtcp-fb:126 nack pli"), "a=rtcp-fb:126 nack pli");
-  offer.replace(match = offer.find("a=rtcp-fb:120 ccm fir"),
-    strlen("a=rtcp-fb:126 ccm fir"), "a=rtcp-fb:126 ccm fir");
-
   std::cout << "Modified SDP " << std::endl
             << indent(offer) << std::endl;
 
   // P1 should be offered first
+#ifdef H264_P0_SUPPORTED
   ASSERT_NE(offer.find("RTP/SAVPF 126 120 97"), std::string::npos);
+#else
+  ASSERT_NE(offer.find("RTP/SAVPF 126 120"), std::string::npos);
+#endif
 
   a1_->SetLocal(TestObserver::OFFER, offer);
   a2_->SetRemote(TestObserver::OFFER, offer, false);
   a2_->CreateAnswer(offer, OFFER_AV|ANSWER_AV, SHOULD_SENDRECV_AV);
 
   std::string answer(a2_->answer());
 
   // Validate answer SDP
@@ -4051,16 +4054,17 @@ TEST_F(SignalingTest, OfferWithH264Befor
   ASSERT_NE(answer.find("a=rtcp-fb:126 ccm fir"), std::string::npos);
   // VP8 and P0 removed
   ASSERT_EQ(answer.find("a=rtpmap:97 H264/90000"), std::string::npos);
   ASSERT_EQ(answer.find("a=rtpmap:120 VP8/90000"), std::string::npos);
   ASSERT_EQ(answer.find("a=rtcp-fb:120"), std::string::npos);
   ASSERT_EQ(answer.find("a=rtcp-fb:97"), std::string::npos);
 }
 
+#ifdef H264_P0_SUPPORTED
 // Remove H.264 P1 and VP8 from offer, check answer negotiates H.264 P0
 TEST_F(SignalingTest, OfferWithOnlyH264P0)
 {
   EnsureInit();
 
   sipcc::OfferOptions options;
   size_t match;
 
@@ -4101,16 +4105,17 @@ TEST_F(SignalingTest, OfferWithOnlyH264P
   ASSERT_NE(answer.find("a=rtcp-fb:97 nack pli"), std::string::npos);
   ASSERT_NE(answer.find("a=rtcp-fb:97 ccm fir"), std::string::npos);
   // Ensure VP8 and P1 removed
   ASSERT_EQ(answer.find("a=rtpmap:126 H264/90000"), std::string::npos);
   ASSERT_EQ(answer.find("a=rtpmap:120 VP8/90000"), std::string::npos);
   ASSERT_EQ(answer.find("a=rtcp-fb:120"), std::string::npos);
   ASSERT_EQ(answer.find("a=rtcp-fb:126"), std::string::npos);
 }
+#endif
 
 // Test negotiating an answer which has only H.264 P1
 // Which means replace VP8 with H.264 P1 in answer
 TEST_F(SignalingTest, AnswerWithoutVP8)
 {
   EnsureInit();
 
   sipcc::OfferOptions options;