sdp: parse and serialize non-key=value fmtp lines
some codecs like RED and telephone-event have fmtp lines which
do not conform to the list-of-key=value convention. Add support
for parsing and serializing this by setting the name to the empty
string.
BUG=webrtc:11640
Change-Id: Ie3ef7c98f756940f97d27a39af0574aa37949f74
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178120
Commit-Queue: Taylor <deadbeef@webrtc.org>
Reviewed-by: Justin Uberti <juberti@webrtc.org>
Reviewed-by: Taylor <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31609}
diff --git a/media/base/codec.h b/media/base/codec.h
index fd8a97c..c3be233 100644
--- a/media/base/codec.h
+++ b/media/base/codec.h
@@ -67,6 +67,8 @@
int id;
std::string name;
int clockrate;
+ // Non key-value parameters such as the telephone-event "0‐15" are
+ // represented using an empty string as key, i.e. {"": "0-15"}.
CodecParameterMap params;
FeedbackParams feedback_params;
diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc
index 3188e3a..af58479 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -1762,8 +1762,13 @@
void WriteFmtpParameter(const std::string& parameter_name,
const std::string& parameter_value,
rtc::StringBuilder* os) {
- // fmtp parameters: |parameter_name|=|parameter_value|
- *os << parameter_name << kSdpDelimiterEqual << parameter_value;
+ if (parameter_name == "") {
+ // RFC 2198 and RFC 4733 don't use key-value pairs.
+ *os << parameter_value;
+ } else {
+ // fmtp parameters: |parameter_name|=|parameter_value|
+ *os << parameter_name << kSdpDelimiterEqual << parameter_value;
+ }
}
bool IsFmtpParam(const std::string& name) {
@@ -3603,8 +3608,10 @@
std::string* value,
SdpParseError* error) {
if (!rtc::tokenize_first(line, kSdpDelimiterEqualChar, parameter, value)) {
- ParseFailed(line, "Unable to parse fmtp parameter. \'=\' missing.", error);
- return false;
+ // Support for non-key-value lines like RFC 2198 or RFC 4733.
+ *parameter = "";
+ *value = line;
+ return true;
}
// a=fmtp:<payload_type> <param1>=<value1>; <param2>=<value2>; ...
return true;
@@ -3622,7 +3629,7 @@
std::string line_payload;
std::string line_params;
- // RFC 5576
+ // https://tools.ietf.org/html/rfc4566#section-6
// a=fmtp:<format> <format specific parameters>
// At least two fields, whereas the second one is any of the optional
// parameters.
@@ -3651,17 +3658,15 @@
cricket::CodecParameterMap codec_params;
for (auto& iter : fields) {
- if (iter.find(kSdpDelimiterEqual) == std::string::npos) {
- // Only fmtps with equals are currently supported. Other fmtp types
- // should be ignored. Unknown fmtps do not constitute an error.
- continue;
- }
-
std::string name;
std::string value;
if (!ParseFmtpParam(rtc::string_trim(iter), &name, &value, error)) {
return false;
}
+ if (codec_params.find(name) != codec_params.end()) {
+ RTC_LOG(LS_INFO) << "Overwriting duplicate fmtp parameter with key \""
+ << name << "\".";
+ }
codec_params[name] = value;
}
diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc
index 49fc006..7b83c86 100644
--- a/pc/webrtc_sdp_unittest.cc
+++ b/pc/webrtc_sdp_unittest.cc
@@ -1293,8 +1293,7 @@
"inline:NzB4d1BINUAvLEw6UzF3WSJ+PSdFcGdUJShpX1Zj|2^20|1:32",
"dummy_session_params"));
audio->set_protocol(cricket::kMediaProtocolSavpf);
- AudioCodec opus(111, "opus", 48000, 0, 2);
- audio->AddCodec(opus);
+ audio->AddCodec(AudioCodec(111, "opus", 48000, 0, 2));
audio->AddCodec(AudioCodec(103, "ISAC", 16000, 0, 1));
audio->AddCodec(AudioCodec(104, "ISAC", 32000, 0, 1));
return audio;
@@ -1934,13 +1933,14 @@
// description.
"a=msid-semantic: WMS\r\n"
// Pl type 111 preferred.
- "m=audio 9 RTP/SAVPF 111 104 103\r\n"
+ "m=audio 9 RTP/SAVPF 111 104 103 105\r\n"
// Pltype 111 listed before 103 and 104 in the map.
"a=rtpmap:111 opus/48000/2\r\n"
// Pltype 103 listed before 104.
"a=rtpmap:103 ISAC/16000\r\n"
"a=rtpmap:104 ISAC/32000\r\n"
- "a=fmtp:111 0-15,66,70\r\n"
+ "a=rtpmap:105 telephone-event/8000\r\n"
+ "a=fmtp:105 0-15,66,70\r\n"
"a=fmtp:111 ";
std::ostringstream os;
os << "minptime=" << params.min_ptime << "; stereo=" << params.stereo
@@ -1987,6 +1987,14 @@
VerifyCodecParameter(codec.params, "maxptime", params.max_ptime);
}
+ cricket::AudioCodec dtmf = acd->codecs()[3];
+ EXPECT_EQ("telephone-event", dtmf.name);
+ EXPECT_EQ(105, dtmf.id);
+ EXPECT_EQ(3u,
+ dtmf.params.size()); // ptime and max_ptime count as parameters.
+ EXPECT_EQ(dtmf.params.begin()->first, "");
+ EXPECT_EQ(dtmf.params.begin()->second, "0-15,66,70");
+
const VideoContentDescription* vcd =
GetFirstVideoContentDescription(jdesc_output->description());
ASSERT_TRUE(vcd);
@@ -3592,6 +3600,28 @@
EXPECT_EQ(sdp_with_fmtp, message);
}
+TEST_F(WebRtcSdpTest, SerializeAudioFmtpWithTelephoneEvent) {
+ AudioContentDescription* acd = GetFirstAudioContentDescription(&desc_);
+
+ cricket::AudioCodecs codecs = acd->codecs();
+ cricket::AudioCodec dtmf(105, "telephone-event", 8000, 0, 1);
+ dtmf.params[""] = "0-15";
+ codecs.push_back(dtmf);
+ acd->set_codecs(codecs);
+
+ ASSERT_TRUE(jdesc_.Initialize(desc_.Clone(), jdesc_.session_id(),
+ jdesc_.session_version()));
+ std::string message = webrtc::SdpSerialize(jdesc_);
+ std::string sdp_with_fmtp = kSdpFullString;
+ InjectAfter("m=audio 2345 RTP/SAVPF 111 103 104", " 105", &sdp_with_fmtp);
+ InjectAfter(
+ "a=rtpmap:104 ISAC/32000\r\n",
+ "a=rtpmap:105 telephone-event/8000\r\n" // No comma here. String merging!
+ "a=fmtp:105 0-15\r\n",
+ &sdp_with_fmtp);
+ EXPECT_EQ(sdp_with_fmtp, message);
+}
+
TEST_F(WebRtcSdpTest, SerializeVideoFmtp) {
VideoContentDescription* vcd = GetFirstVideoContentDescription(&desc_);