diff --git a/media/base/codec.h b/media/base/codec.h index fd8a97c5e4..c3be2334ce 100644 --- a/media/base/codec.h +++ b/media/base/codec.h @@ -67,6 +67,8 @@ struct RTC_EXPORT Codec { 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 3188e3a956..af584791be 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -1762,8 +1762,13 @@ void WriteRtcpFbHeader(int payload_type, rtc::StringBuilder* os) { 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 @@ bool ParseFmtpParam(const std::string& line, 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: =; =; ... return true; @@ -3622,7 +3629,7 @@ bool ParseFmtpAttributes(const std::string& line, std::string line_payload; std::string line_params; - // RFC 5576 + // https://tools.ietf.org/html/rfc4566#section-6 // a=fmtp: // At least two fields, whereas the second one is any of the optional // parameters. @@ -3651,17 +3658,15 @@ bool ParseFmtpAttributes(const std::string& line, 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 49fc0063cd..7b83c86ab1 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -1293,8 +1293,7 @@ class WebRtcSdpTest : public ::testing::Test { "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 @@ class WebRtcSdpTest : public ::testing::Test { // 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 @@ class WebRtcSdpTest : public ::testing::Test { 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 @@ TEST_F(WebRtcSdpTest, SerializeAudioFmtpWithPTimeAndMaxPTime) { 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_);