sdp: measure codec collisions in bundle
as described in https://www.rfc-editor.org/rfc/rfc8843#name-payload-type-pt-value-reuse ... all codecs associated with the payload type number MUST share an identical codec configuration See also https://github.com/w3c/webrtc-stats/issues/664 Measure how much this would break in UMA first BUG=webrtc:14420,webrtc:12716 Change-Id: Iafdc70248aa22bc37c15cc88a0c244398cb58176 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273881 Reviewed-by: Harald Alvestrand <hta@webrtc.org> Reviewed-by: Henrik Boström <hbos@webrtc.org> Commit-Queue: Philipp Hancke <phancke@microsoft.com> Cr-Commit-Position: refs/heads/main@{#38759}
This commit is contained in:
committed by
WebRTC LUCI CQ
parent
c06dc4df80
commit
f0ea56a0a2
@ -430,6 +430,73 @@ RTCError ValidateMids(const cricket::SessionDescription& description) {
|
||||
return RTCError::OK();
|
||||
}
|
||||
|
||||
RTCError FindDuplicateCodecParameters(
|
||||
const RtpCodecParameters codec_parameters,
|
||||
std::map<int, RtpCodecParameters>& payload_to_codec_parameters) {
|
||||
auto existing_codec_parameters =
|
||||
payload_to_codec_parameters.find(codec_parameters.payload_type);
|
||||
if (existing_codec_parameters != payload_to_codec_parameters.end() &&
|
||||
codec_parameters != existing_codec_parameters->second) {
|
||||
return RTCError(RTCErrorType::INVALID_PARAMETER,
|
||||
"A BUNDLE group contains a codec collision for "
|
||||
"payload_type='" +
|
||||
rtc::ToString(codec_parameters.payload_type) +
|
||||
". All codecs must share the same type, "
|
||||
"encoding name, clock rate and parameters.");
|
||||
}
|
||||
payload_to_codec_parameters.insert(
|
||||
std::make_pair(codec_parameters.payload_type, codec_parameters));
|
||||
return RTCError::OK();
|
||||
}
|
||||
|
||||
RTCError ValidateBundledPayloadTypes(
|
||||
const cricket::SessionDescription& description) {
|
||||
// https://www.rfc-editor.org/rfc/rfc8843#name-payload-type-pt-value-reuse
|
||||
// ... all codecs associated with the payload type number MUST share an
|
||||
// identical codec configuration. This means that the codecs MUST share
|
||||
// the same media type, encoding name, clock rate, and any parameter
|
||||
// that can affect the codec configuration and packetization.
|
||||
std::map<int, RtpCodecParameters> payload_to_codec_parameters;
|
||||
std::vector<const cricket::ContentGroup*> bundle_groups =
|
||||
description.GetGroupsByName(cricket::GROUP_TYPE_BUNDLE);
|
||||
for (const cricket::ContentGroup* bundle_group : bundle_groups) {
|
||||
std::map<int, RtpCodecParameters> payload_to_codec_parameters;
|
||||
for (const std::string& content_name : bundle_group->content_names()) {
|
||||
const cricket::MediaContentDescription* media_description =
|
||||
description.GetContentDescriptionByName(content_name);
|
||||
if (!media_description) {
|
||||
return RTCError(RTCErrorType::INVALID_PARAMETER,
|
||||
"A BUNDLE group contains a MID='" + content_name +
|
||||
"' matching no m= section.");
|
||||
}
|
||||
if (!media_description->has_codecs()) {
|
||||
continue;
|
||||
}
|
||||
const auto type = media_description->type();
|
||||
if (type == cricket::MEDIA_TYPE_AUDIO) {
|
||||
RTC_DCHECK(media_description->as_audio());
|
||||
for (const auto& c : media_description->as_audio()->codecs()) {
|
||||
auto error = FindDuplicateCodecParameters(
|
||||
c.ToCodecParameters(), payload_to_codec_parameters);
|
||||
if (!error.ok()) {
|
||||
return error;
|
||||
}
|
||||
}
|
||||
} else if (type == cricket::MEDIA_TYPE_VIDEO) {
|
||||
RTC_DCHECK(media_description->as_video());
|
||||
for (const auto& c : media_description->as_video()->codecs()) {
|
||||
auto error = FindDuplicateCodecParameters(
|
||||
c.ToCodecParameters(), payload_to_codec_parameters);
|
||||
if (!error.ok()) {
|
||||
return error;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return RTCError::OK();
|
||||
}
|
||||
|
||||
bool IsValidOfferToReceiveMedia(int value) {
|
||||
typedef PeerConnectionInterface::RTCOfferAnswerOptions Options;
|
||||
return (value >= Options::kUndefined) &&
|
||||
@ -3310,6 +3377,12 @@ RTCError SdpOfferAnswerHandler::ValidateSessionDescription(
|
||||
return RTCError(RTCErrorType::INVALID_PARAMETER, kSdpWithoutIceUfragPwd);
|
||||
}
|
||||
|
||||
// Validate bundle, payload types and that there are no collisions.
|
||||
error = ValidateBundledPayloadTypes(*sdesc->description());
|
||||
// TODO(bugs.webrtc.org/14420): actually reject.
|
||||
RTC_HISTOGRAM_BOOLEAN("WebRTC.PeerConnection.ValidBundledPayloadTypes",
|
||||
error.ok());
|
||||
|
||||
if (!pc_->ValidateBundleSettings(sdesc->description(),
|
||||
bundle_groups_by_mid)) {
|
||||
return RTCError(RTCErrorType::INVALID_PARAMETER, kBundleWithoutRtcpMux);
|
||||
|
||||
@ -114,4 +114,128 @@ TEST_F(SdpOfferAnswerTest, OnTrackReturnsProxiedObject) {
|
||||
transceiver->stopped();
|
||||
}
|
||||
|
||||
TEST_F(SdpOfferAnswerTest, BundleRejectsCodecCollisionsAudioVideo) {
|
||||
auto pc = CreatePeerConnection();
|
||||
std::string sdp =
|
||||
"v=0\r\n"
|
||||
"o=- 0 3 IN IP4 127.0.0.1\r\n"
|
||||
"s=-\r\n"
|
||||
"t=0 0\r\n"
|
||||
"a=group:BUNDLE 0 1\r\n"
|
||||
"a=fingerprint:sha-1 "
|
||||
"4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
|
||||
"a=setup:actpass\r\n"
|
||||
"a=ice-ufrag:ETEn\r\n"
|
||||
"a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n"
|
||||
"m=audio 9 UDP/TLS/RTP/SAVPF 111\r\n"
|
||||
"c=IN IP4 0.0.0.0\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
"a=sendonly\r\n"
|
||||
"a=mid:0\r\n"
|
||||
"a=rtpmap:111 opus/48000/2\r\n"
|
||||
"m=video 9 UDP/TLS/RTP/SAVPF 111\r\n"
|
||||
"c=IN IP4 0.0.0.0\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
"a=sendonly\r\n"
|
||||
"a=mid:1\r\n"
|
||||
"a=rtpmap:111 H264/90000\r\n"
|
||||
"a=fmtp:111 "
|
||||
"level-asymmetry-allowed=1;packetization-mode=0;profile-level-id="
|
||||
"42e01f\r\n";
|
||||
|
||||
auto desc = CreateSessionDescription(SdpType::kOffer, sdp);
|
||||
ASSERT_NE(desc, nullptr);
|
||||
RTCError error;
|
||||
pc->SetRemoteDescription(std::move(desc), &error);
|
||||
EXPECT_TRUE(error.ok());
|
||||
EXPECT_METRIC_EQ(
|
||||
1, webrtc::metrics::NumEvents(
|
||||
"WebRTC.PeerConnection.ValidBundledPayloadTypes", false));
|
||||
}
|
||||
|
||||
TEST_F(SdpOfferAnswerTest, BundleRejectsCodecCollisionsVideoFmtp) {
|
||||
auto pc = CreatePeerConnection();
|
||||
std::string sdp =
|
||||
"v=0\r\n"
|
||||
"o=- 0 3 IN IP4 127.0.0.1\r\n"
|
||||
"s=-\r\n"
|
||||
"t=0 0\r\n"
|
||||
"a=group:BUNDLE 0 1\r\n"
|
||||
"a=fingerprint:sha-1 "
|
||||
"4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
|
||||
"a=setup:actpass\r\n"
|
||||
"a=ice-ufrag:ETEn\r\n"
|
||||
"a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n"
|
||||
"m=video 9 UDP/TLS/RTP/SAVPF 111\r\n"
|
||||
"c=IN IP4 0.0.0.0\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
"a=sendonly\r\n"
|
||||
"a=mid:0\r\n"
|
||||
"a=rtpmap:111 H264/90000\r\n"
|
||||
"a=fmtp:111 "
|
||||
"level-asymmetry-allowed=1;packetization-mode=0;profile-level-id="
|
||||
"42e01f\r\n"
|
||||
"m=video 9 UDP/TLS/RTP/SAVPF 111\r\n"
|
||||
"c=IN IP4 0.0.0.0\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
"a=sendonly\r\n"
|
||||
"a=mid:1\r\n"
|
||||
"a=rtpmap:111 H264/90000\r\n"
|
||||
"a=fmtp:111 "
|
||||
"level-asymmetry-allowed=1;packetization-mode=1;profile-level-id="
|
||||
"42e01f\r\n";
|
||||
|
||||
auto desc = CreateSessionDescription(SdpType::kOffer, sdp);
|
||||
ASSERT_NE(desc, nullptr);
|
||||
RTCError error;
|
||||
pc->SetRemoteDescription(std::move(desc), &error);
|
||||
EXPECT_TRUE(error.ok());
|
||||
EXPECT_METRIC_EQ(
|
||||
1, webrtc::metrics::NumEvents(
|
||||
"WebRTC.PeerConnection.ValidBundledPayloadTypes", false));
|
||||
}
|
||||
|
||||
TEST_F(SdpOfferAnswerTest, BundleCodecCollisionInDifferentBundlesAllowed) {
|
||||
auto pc = CreatePeerConnection();
|
||||
std::string sdp =
|
||||
"v=0\r\n"
|
||||
"o=- 0 3 IN IP4 127.0.0.1\r\n"
|
||||
"s=-\r\n"
|
||||
"t=0 0\r\n"
|
||||
"a=group:BUNDLE 0\r\n"
|
||||
"a=group:BUNDLE 1\r\n"
|
||||
"a=fingerprint:sha-1 "
|
||||
"4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
|
||||
"a=setup:actpass\r\n"
|
||||
"a=ice-ufrag:ETEn\r\n"
|
||||
"a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n"
|
||||
"m=video 9 UDP/TLS/RTP/SAVPF 111\r\n"
|
||||
"c=IN IP4 0.0.0.0\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
"a=sendonly\r\n"
|
||||
"a=mid:0\r\n"
|
||||
"a=rtpmap:111 H264/90000\r\n"
|
||||
"a=fmtp:111 "
|
||||
"level-asymmetry-allowed=1;packetization-mode=0;profile-level-id="
|
||||
"42e01f\r\n"
|
||||
"m=video 9 UDP/TLS/RTP/SAVPF 111\r\n"
|
||||
"c=IN IP4 0.0.0.0\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
"a=sendonly\r\n"
|
||||
"a=mid:1\r\n"
|
||||
"a=rtpmap:111 H264/90000\r\n"
|
||||
"a=fmtp:111 "
|
||||
"level-asymmetry-allowed=1;packetization-mode=1;profile-level-id="
|
||||
"42e01f\r\n";
|
||||
|
||||
auto desc = CreateSessionDescription(SdpType::kOffer, sdp);
|
||||
ASSERT_NE(desc, nullptr);
|
||||
RTCError error;
|
||||
pc->SetRemoteDescription(std::move(desc), &error);
|
||||
EXPECT_TRUE(error.ok());
|
||||
EXPECT_METRIC_EQ(
|
||||
0, webrtc::metrics::NumEvents(
|
||||
"WebRTC.PeerConnection.ValidBundledPayloadTypes", false));
|
||||
}
|
||||
|
||||
} // namespace webrtc
|
||||
|
||||
Reference in New Issue
Block a user