diff --git a/pc/test/test_sdp_strings.h b/pc/test/test_sdp_strings.h index 6394ac5f5e..e4ad325d31 100644 --- a/pc/test/test_sdp_strings.h +++ b/pc/test/test_sdp_strings.h @@ -133,10 +133,10 @@ static const char kAudioSdpWithUnsupportedCodecsPlanB[] = "s=-\r\n" "c=IN IP4 192.168.30.208\r\n" "t=0 0\r\n" - "m=audio 16000 RTP/SAVPF 0 8 18 110 126\r\n" + "m=audio 16000 RTP/SAVPF 0 8 109 110 126\r\n" "a=rtpmap:0 PCMU/8000\r\n" "a=rtpmap:8 PCMA/8000\r\n" - "a=rtpmap:18 WeirdCodec1/8000\r\n" + "a=rtpmap:109 WeirdCodec1/8000\r\n" "a=rtpmap:110 WeirdCodec2/8000\r\n" "a=rtpmap:126 telephone-event/8000\r\n" "a=sendonly\r\n" @@ -160,10 +160,10 @@ static const char kAudioSdpWithUnsupportedCodecsUnifiedPlan[] = "s=-\r\n" "c=IN IP4 192.168.30.208\r\n" "t=0 0\r\n" - "m=audio 16000 RTP/SAVPF 0 8 18 110 126\r\n" + "m=audio 16000 RTP/SAVPF 0 8 109 110 126\r\n" "a=rtpmap:0 PCMU/8000\r\n" "a=rtpmap:8 PCMA/8000\r\n" - "a=rtpmap:18 WeirdCodec1/8000\r\n" + "a=rtpmap:109 WeirdCodec1/8000\r\n" "a=rtpmap:110 WeirdCodec2/8000\r\n" "a=rtpmap:126 telephone-event/8000\r\n" "a=sendonly\r\n" diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index a62b6f7ffa..5094db49f3 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -280,7 +280,7 @@ static void BuildRtpContentAttributes(const MediaContentDescription* media_desc, const cricket::MediaType media_type, int msid_signaling, std::string* message); -static void BuildRtpMap(const MediaContentDescription* media_desc, +static void BuildRtpmap(const MediaContentDescription* media_desc, const cricket::MediaType media_type, std::string* message); static void BuildCandidate(const std::vector& candidates, @@ -1682,7 +1682,7 @@ void BuildRtpContentAttributes(const MediaContentDescription* media_desc, // RFC 4566 // a=rtpmap: / // [/] - BuildRtpMap(media_desc, media_type, message); + BuildRtpmap(media_desc, media_type, message); for (const StreamParams& track : media_desc->streams()) { // Build the ssrc-group lines. @@ -1882,7 +1882,7 @@ bool GetParameter(const std::string& name, return true; } -void BuildRtpMap(const MediaContentDescription* media_desc, +void BuildRtpmap(const MediaContentDescription* media_desc, const cricket::MediaType media_type, std::string* message) { RTC_DCHECK(message != NULL); @@ -2852,7 +2852,7 @@ T GetCodecWithPayloadType(const std::vector& codecs, int payload_type) { return ret_val; } -// Updates or creates a new codec entry in the audio description. +// Updates or creates a new codec entry in the media description. template void AddOrReplaceCodec(MediaContentDescription* content_desc, const U& codec) { T* desc = static_cast(content_desc); @@ -3556,6 +3556,7 @@ bool ParseRtpmapAttribute(absl::string_view line, const std::vector& payload_types, MediaContentDescription* media_desc, SdpParseError* error) { + static const int kFirstDynamicPayloadTypeLowerRange = 35; std::vector fields; rtc::split(line.substr(kLinePrefixLength), kSdpDelimiterSpaceChar, &fields); // RFC 4566 @@ -3596,8 +3597,23 @@ bool ParseRtpmapAttribute(absl::string_view line, if (!GetValueFromString(line, codec_params[1], &clock_rate, error)) { return false; } + if (media_type == cricket::MEDIA_TYPE_VIDEO) { VideoContentDescription* video_desc = media_desc->as_video(); + for (const cricket::VideoCodec& existing_codec : video_desc->codecs()) { + if (!existing_codec.name.empty() && payload_type == existing_codec.id && + (encoding_name != existing_codec.name || + clock_rate != existing_codec.clockrate)) { + rtc::StringBuilder description; + description + << "Duplicate " + << (payload_type < kFirstDynamicPayloadTypeLowerRange + ? "statically assigned" + : "") + << " payload type with conflicting codec name or clock rate."; + return ParseFailed(line, description.Release(), error); + } + } UpdateCodec(payload_type, encoding_name, video_desc); } else if (media_type == cricket::MEDIA_TYPE_AUDIO) { // RFC 4566 @@ -3616,6 +3632,21 @@ bool ParseRtpmapAttribute(absl::string_view line, } AudioContentDescription* audio_desc = media_desc->as_audio(); + for (const cricket::AudioCodec& existing_codec : audio_desc->codecs()) { + if (!existing_codec.name.empty() && payload_type == existing_codec.id && + (encoding_name != existing_codec.name || + clock_rate != existing_codec.clockrate || + channels != existing_codec.channels)) { + rtc::StringBuilder description; + description + << "Duplicate " + << (payload_type < kFirstDynamicPayloadTypeLowerRange + ? "statically assigned" + : "") + << " payload type with conflicting codec name or clock rate."; + return ParseFailed(line, description.Release(), error); + } + } UpdateCodec(payload_type, encoding_name, clock_rate, 0, channels, audio_desc); } diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc index 86e137ee1a..306b16281c 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -2427,7 +2427,7 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutRtpmap) { // Codec that doesn't appear in the m= line will be ignored. "a=rtpmap:104 ISAC/32000\r\n" // The rtpmap line for static payload codec is optional. - "a=rtpmap:18 G729/16000\r\n" + "a=rtpmap:18 G729/8000\r\n" "a=rtpmap:103 ISAC/16000\r\n"; JsepSessionDescription jdesc(kDummyType); @@ -2438,7 +2438,7 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutRtpmap) { // The codecs in the AudioContentDescription should be in the same order as // the payload types (s) on the m= line. ref_codecs.push_back(AudioCodec(0, "PCMU", 8000, 0, 1)); - ref_codecs.push_back(AudioCodec(18, "G729", 16000, 0, 1)); + ref_codecs.push_back(AudioCodec(18, "G729", 8000, 0, 1)); ref_codecs.push_back(AudioCodec(103, "ISAC", 16000, 0, 1)); EXPECT_EQ(ref_codecs, audio->codecs()); } @@ -4658,6 +4658,47 @@ TEST_F(WebRtcSdpTest, MaxChannels) { ExpectParseFailure(sdp, "a=rtpmap:108 ISAC/16000/512"); } +TEST_F(WebRtcSdpTest, DuplicateAudioRtpmapWithConflict) { + std::string sdp = + "v=0\r\n" + "o=- 11 22 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "m=audio 49232 RTP/AVP 108\r\n" + // Same name but different payload type. + "a=rtpmap:108 ISAC/16000\r\n" + "a=rtpmap:108 ISAC/32000\r\n"; + + ExpectParseFailure(sdp, "a=rtpmap:108 ISAC/32000"); +} + +TEST_F(WebRtcSdpTest, DuplicateVideoRtpmapWithConflict) { + std::string sdp = + "v=0\r\n" + "o=- 11 22 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "m=video 49232 RTP/AVP 108\r\n" + "a=rtpmap:108 VP8/90000\r\n" + "a=rtpmap:108 VP9/90000\r\n"; + + ExpectParseFailure(sdp, "a=rtpmap:108 VP9/90000"); +} + +TEST_F(WebRtcSdpTest, FmtpBeforeRtpMap) { + std::string sdp = + "v=0\r\n" + "o=- 11 22 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "m=video 49232 RTP/AVP 108\r\n" + "a=fmtp:108 profile-level=1\r\n" + "a=rtpmap:108 VP9/90000\r\n"; + + JsepSessionDescription jdesc_output(kDummyType); + EXPECT_TRUE(SdpDeserialize(sdp, &jdesc_output)); +} + // This tests parsing of SDP with unknown ssrc-specific attributes. TEST_F(WebRtcSdpTest, ParseIgnoreUnknownSsrcSpecificAttribute) { std::string sdp = kSdpString;