Revert "sdp: reject duplicate codecs with the same id but different name or clockrate"
This reverts commit ad6807805d12e48f11c3a68b4befaf8d7c23e8b5.
Reason for revert: Speculative revert due to consistent Mac browser
test failures preventing WebRTC from rolling int Chromium:
https://ci.chromium.org/ui/p/chromium/builders/webrtc.fyi/WebRTC%20Chromium%20FYI%20Mac%20Tester/10410/overview
"Failed to parse SessionDescription. a=rtpmap:103 ISAC/16000 Duplicate payload type with conflicting codec name, clock rate or number of channels."
Original change's description:
> sdp: reject duplicate codecs with the same id but different name or clockrate
>
> since something like
> rtpmap:96 VP8/90000
> rtpmap:96 VP9/90000
> or
> rtpmap:97 ISAC/32000
> rtpmap:97 ISAC/16000
> is wrong. Note that fmtp or rtcp-fb are not taken into account.
> Also note that sending invalid static payload types now throws an error.
>
> Drive-by: replace "RtpMap" with "Rtpmap" for consistency.
>
> BUG=None
>
> Change-Id: I2574b82a6f1a0afe3edc866e514a5dbca0798e8c
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/263641
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Philipp Hancke <philipp.hancke@googlemail.com>
> Cr-Commit-Position: refs/heads/main@{#37028}
Bug: None
Change-Id: Ic9c06c9309bb68bd94bfdd2e30ffd6ff96f6812b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264540
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Christoffer Jansson <jansson@webrtc.org>
Reviewed-by: Philipp Hancke <philipp.hancke@googlemail.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37064}
diff --git a/pc/test/test_sdp_strings.h b/pc/test/test_sdp_strings.h
index e4ad325..6394ac5 100644
--- a/pc/test/test_sdp_strings.h
+++ b/pc/test/test_sdp_strings.h
@@ -133,10 +133,10 @@
"s=-\r\n"
"c=IN IP4 192.168.30.208\r\n"
"t=0 0\r\n"
- "m=audio 16000 RTP/SAVPF 0 8 109 110 126\r\n"
+ "m=audio 16000 RTP/SAVPF 0 8 18 110 126\r\n"
"a=rtpmap:0 PCMU/8000\r\n"
"a=rtpmap:8 PCMA/8000\r\n"
- "a=rtpmap:109 WeirdCodec1/8000\r\n"
+ "a=rtpmap:18 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 @@
"s=-\r\n"
"c=IN IP4 192.168.30.208\r\n"
"t=0 0\r\n"
- "m=audio 16000 RTP/SAVPF 0 8 109 110 126\r\n"
+ "m=audio 16000 RTP/SAVPF 0 8 18 110 126\r\n"
"a=rtpmap:0 PCMU/8000\r\n"
"a=rtpmap:8 PCMA/8000\r\n"
- "a=rtpmap:109 WeirdCodec1/8000\r\n"
+ "a=rtpmap:18 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 16ad68e..a62b6f7 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -280,7 +280,7 @@
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<Candidate>& candidates,
@@ -1682,7 +1682,7 @@
// RFC 4566
// a=rtpmap:<payload type> <encoding name>/<clock rate>
// [/<encodingparameters>]
- 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 @@
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 @@
return ret_val;
}
-// Updates or creates a new codec entry in the media description.
+// Updates or creates a new codec entry in the audio description.
template <class T, class U>
void AddOrReplaceCodec(MediaContentDescription* content_desc, const U& codec) {
T* desc = static_cast<T*>(content_desc);
@@ -3596,19 +3596,8 @@
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 (payload_type == existing_codec.id &&
- (encoding_name != existing_codec.name ||
- clock_rate != existing_codec.clockrate)) {
- return ParseFailed(
- line,
- "Duplicate payload type with conflicting codec name or clock rate.",
- error);
- }
- }
UpdateCodec(payload_type, encoding_name, video_desc);
} else if (media_type == cricket::MEDIA_TYPE_AUDIO) {
// RFC 4566
@@ -3627,17 +3616,6 @@
}
AudioContentDescription* audio_desc = media_desc->as_audio();
- for (const cricket::AudioCodec& existing_codec : audio_desc->codecs()) {
- if (payload_type == existing_codec.id &&
- (encoding_name != existing_codec.name ||
- clock_rate != existing_codec.clockrate ||
- channels != existing_codec.channels)) {
- return ParseFailed(line,
- "Duplicate payload type with conflicting codec "
- "name, clock rate or number of channels.",
- 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 8db214c..86e137e 100644
--- a/pc/webrtc_sdp_unittest.cc
+++ b/pc/webrtc_sdp_unittest.cc
@@ -2427,7 +2427,7 @@
// 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/8000\r\n"
+ "a=rtpmap:18 G729/16000\r\n"
"a=rtpmap:103 ISAC/16000\r\n";
JsepSessionDescription jdesc(kDummyType);
@@ -2438,7 +2438,7 @@
// The codecs in the AudioContentDescription should be in the same order as
// the payload types (<fmt>s) on the m= line.
ref_codecs.push_back(AudioCodec(0, "PCMU", 8000, 0, 1));
- ref_codecs.push_back(AudioCodec(18, "G729", 8000, 0, 1));
+ ref_codecs.push_back(AudioCodec(18, "G729", 16000, 0, 1));
ref_codecs.push_back(AudioCodec(103, "ISAC", 16000, 0, 1));
EXPECT_EQ(ref_codecs, audio->codecs());
}
@@ -4658,33 +4658,6 @@
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");
-}
-
// This tests parsing of SDP with unknown ssrc-specific attributes.
TEST_F(WebRtcSdpTest, ParseIgnoreUnknownSsrcSpecificAttribute) {
std::string sdp = kSdpString;
