From 7d1aff6eed05fdb9daabb06259c55d2049e41742 Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Mon, 25 Sep 2023 14:42:51 +0200 Subject: [PATCH] Unify RTP payload type validity checking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit making the UsedId generator the source of truth. BUG=webrtc:12197 Change-Id: I4318a1366f8b2e20ea5ae264232437a9006c5103 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/321120 Commit-Queue: Philipp Hancke Reviewed-by: Henrik Boström Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#40802} --- pc/BUILD.gn | 1 + pc/sdp_offer_answer.cc | 19 +++++-------------- pc/used_ids.h | 10 ++++++++++ 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 2da703e869..899b89a0fe 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -1108,6 +1108,7 @@ rtc_source_set("sdp_offer_answer") { ":stream_collection", ":transceiver_list", ":usage_pattern", + ":used_ids", ":webrtc_session_description_factory", "../api:array_view", "../api:audio_options_api", diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index fe7093d604..1ed5c80c29 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -53,6 +53,7 @@ #include "pc/rtp_sender_proxy.h" #include "pc/simulcast_description.h" #include "pc/usage_pattern.h" +#include "pc/used_ids.h" #include "pc/webrtc_session_description_factory.h" #include "rtc_base/helpers.h" #include "rtc_base/logging.h" @@ -120,12 +121,6 @@ const char kDefaultStreamId[] = "default"; static const char kDefaultAudioSenderId[] = "defaulta0"; static const char kDefaultVideoSenderId[] = "defaultv0"; -// NOTE: Duplicated from pc/used_ids.h -static const int kLastDynamicPayloadTypeLowerRange = 63; - -static const int kFirstDynamicPayloadTypeUpperRange = 96; -static const int kLastDynamicPayloadTypeUpperRange = 127; - void NoteAddIceCandidateResult(int result) { RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.AddIceCandidate", result, kAddIceCandidateMax); @@ -574,10 +569,8 @@ RTCError ValidatePayloadTypes(const cricket::SessionDescription& description) { if (type == cricket::MEDIA_TYPE_AUDIO) { RTC_DCHECK(media_description->as_audio()); for (const auto& codec : media_description->as_audio()->codecs()) { - if (codec.id < 0 || codec.id > kLastDynamicPayloadTypeUpperRange || - (media_description->rtcp_mux() && - (codec.id > kLastDynamicPayloadTypeLowerRange && - codec.id < kFirstDynamicPayloadTypeUpperRange))) { + if (!cricket::UsedPayloadTypes::IsIdValid( + codec, media_description->rtcp_mux())) { LOG_AND_RETURN_ERROR( RTCErrorType::INVALID_PARAMETER, "The media section with MID='" + content.mid() + @@ -589,10 +582,8 @@ RTCError ValidatePayloadTypes(const cricket::SessionDescription& description) { } else if (type == cricket::MEDIA_TYPE_VIDEO) { RTC_DCHECK(media_description->as_video()); for (const auto& codec : media_description->as_video()->codecs()) { - if (codec.id < 0 || codec.id > kLastDynamicPayloadTypeUpperRange || - (media_description->rtcp_mux() && - (codec.id > kLastDynamicPayloadTypeLowerRange && - codec.id < kFirstDynamicPayloadTypeUpperRange))) { + if (!cricket::UsedPayloadTypes::IsIdValid( + codec, media_description->rtcp_mux())) { LOG_AND_RETURN_ERROR( RTCErrorType::INVALID_PARAMETER, "The media section with MID='" + content.mid() + diff --git a/pc/used_ids.h b/pc/used_ids.h index 1236a786df..6b342cbea8 100644 --- a/pc/used_ids.h +++ b/pc/used_ids.h @@ -96,6 +96,16 @@ class UsedPayloadTypes : public UsedIds { : UsedIds(kFirstDynamicPayloadTypeLowerRange, kLastDynamicPayloadTypeUpperRange) {} + // Check if a payload type is valid. The range [64-95] is forbidden + // when rtcp-mux is used. + static bool IsIdValid(Codec codec, bool rtcp_mux) { + if (rtcp_mux && (codec.id > kLastDynamicPayloadTypeLowerRange && + codec.id < kFirstDynamicPayloadTypeUpperRange)) { + return false; + } + return codec.id >= 0 && codec.id <= kLastDynamicPayloadTypeUpperRange; + } + protected: bool IsIdUsed(int new_id) override { // Range marked for RTCP avoidance is "used".