From aa4b0775aa9877410fb45fe406e786aafef6f934 Mon Sep 17 00:00:00 2001
From: ossu <ossu@webrtc.org>
Date: Mon, 30 Jan 2017 07:41:18 -0800
Subject: [PATCH] Simplify IsFmtpParam according to RFC 4855.

This should help pave the way for injectable audio codecs, since
external implementations need to be able to signal arbitrary fmtp
parameters.

BUG=webrtc:5806

Review-Url: https://codereview.webrtc.org/2661453003
Cr-Commit-Position: refs/heads/master@{#16360}
---
 webrtc/pc/webrtcsdp.cc          | 32 +++---------------
 webrtc/pc/webrtcsdp_unittest.cc | 59 +++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 27 deletions(-)

diff --git a/webrtc/pc/webrtcsdp.cc b/webrtc/pc/webrtcsdp.cc
index f1b9052b9f..11eefe5c3d 100644
--- a/webrtc/pc/webrtcsdp.cc
+++ b/webrtc/pc/webrtcsdp.cc
@@ -1597,33 +1597,11 @@ void WriteFmtpParameters(const cricket::CodecParameterMap& parameters,
 }
 
 bool IsFmtpParam(const std::string& name) {
-  const char* kFmtpParams[] = {
-      // TODO(hta): Split FMTP parameters apart from parameters in general.
-      // FMTP parameters are codec specific, not generic.
-      kCodecParamMinPTime,
-      kCodecParamSPropStereo,
-      kCodecParamStereo,
-      kCodecParamUseInbandFec,
-      kCodecParamUseDtx,
-      kCodecParamStartBitrate,
-      kCodecParamMaxBitrate,
-      kCodecParamMinBitrate,
-      kCodecParamMaxQuantization,
-      kCodecParamSctpProtocol,
-      kCodecParamSctpStreams,
-      kCodecParamMaxAverageBitrate,
-      kCodecParamMaxPlaybackRate,
-      kCodecParamAssociatedPayloadType,
-      cricket::kH264FmtpPacketizationMode,
-      cricket::kH264FmtpLevelAsymmetryAllowed,
-      cricket::kH264FmtpProfileLevelId,
-      cricket::kFlexfecFmtpRepairWindow};
-  for (size_t i = 0; i < arraysize(kFmtpParams); ++i) {
-    if (name.compare(kFmtpParams[i]) == 0) {
-      return true;
-    }
-  }
-  return false;
+  // RFC 4855, section 3 specifies the mapping of media format parameters to SDP
+  // parameters. Only ptime, maxptime, channels and rate are placed outside of
+  // the fmtp line. In WebRTC, channels and rate are already handled separately
+  // and thus not included in the CodecParameterMap.
+  return name != kCodecParamPTime && name != kCodecParamMaxPTime;
 }
 
 // Retreives fmtp parameters from |params|, which may contain other parameters
diff --git a/webrtc/pc/webrtcsdp_unittest.cc b/webrtc/pc/webrtcsdp_unittest.cc
index 43774abf7d..81a5aaf4fe 100644
--- a/webrtc/pc/webrtcsdp_unittest.cc
+++ b/webrtc/pc/webrtcsdp_unittest.cc
@@ -3074,6 +3074,65 @@ TEST_F(WebRtcSdpTest, DeserializeVideoFmtpWithSpace) {
   EXPECT_EQ(found->second, "40");
 }
 
+TEST_F(WebRtcSdpTest, SerializeAudioFmtpWithUnknownParameter) {
+  AudioContentDescription* acd = static_cast<AudioContentDescription*>(
+      GetFirstAudioContent(&desc_)->description);
+
+  cricket::AudioCodecs codecs = acd->codecs();
+  codecs[0].params["unknown-future-parameter"] = "SomeFutureValue";
+  acd->set_codecs(codecs);
+
+  ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(),
+                                jdesc_.session_id(),
+                                jdesc_.session_version()));
+  std::string message = webrtc::SdpSerialize(jdesc_, false);
+  std::string sdp_with_fmtp = kSdpFullString;
+  InjectAfter("a=rtpmap:111 opus/48000/2\r\n",
+              "a=fmtp:111 unknown-future-parameter=SomeFutureValue\r\n",
+              &sdp_with_fmtp);
+  EXPECT_EQ(sdp_with_fmtp, message);
+}
+
+TEST_F(WebRtcSdpTest, SerializeAudioFmtpWithKnownFmtpParameter) {
+  AudioContentDescription* acd = static_cast<AudioContentDescription*>(
+      GetFirstAudioContent(&desc_)->description);
+
+  cricket::AudioCodecs codecs = acd->codecs();
+  codecs[0].params["stereo"] = "1";
+  acd->set_codecs(codecs);
+
+  ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(),
+                                jdesc_.session_id(),
+                                jdesc_.session_version()));
+  std::string message = webrtc::SdpSerialize(jdesc_, false);
+  std::string sdp_with_fmtp = kSdpFullString;
+  InjectAfter("a=rtpmap:111 opus/48000/2\r\n",
+              "a=fmtp:111 stereo=1\r\n",
+              &sdp_with_fmtp);
+  EXPECT_EQ(sdp_with_fmtp, message);
+}
+
+TEST_F(WebRtcSdpTest, SerializeAudioFmtpWithPTimeAndMaxPTime) {
+  AudioContentDescription* acd = static_cast<AudioContentDescription*>(
+      GetFirstAudioContent(&desc_)->description);
+
+  cricket::AudioCodecs codecs = acd->codecs();
+  codecs[0].params["ptime"] = "20";
+  codecs[0].params["maxptime"] = "120";
+  acd->set_codecs(codecs);
+
+  ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(),
+                                jdesc_.session_id(),
+                                jdesc_.session_version()));
+  std::string message = webrtc::SdpSerialize(jdesc_, false);
+  std::string sdp_with_fmtp = kSdpFullString;
+  InjectAfter("a=rtpmap:104 ISAC/32000\r\n",
+              "a=maxptime:120\r\n"  // No comma here. String merging!
+              "a=ptime:20\r\n",
+              &sdp_with_fmtp);
+  EXPECT_EQ(sdp_with_fmtp, message);
+}
+
 TEST_F(WebRtcSdpTest, SerializeVideoFmtp) {
   VideoContentDescription* vcd = static_cast<VideoContentDescription*>(
       GetFirstVideoContent(&desc_)->description);