diff --git a/pc/media_session.cc b/pc/media_session.cc index 4f297bac15..6e85c60bf1 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -508,6 +508,7 @@ webrtc::RTCError AssignCodecIdsAndLinkRed( char buffer[100]; rtc::SimpleStringBuilder param(buffer); param << opus_codec << "/" << opus_codec; + RTC_LOG(LS_ERROR) << "DEBUG: Setting RED param to " << param.str(); codec.SetParam(kCodecParamNotInNameValueFormat, param.str()); } } @@ -655,10 +656,9 @@ const Codec* GetAssociatedCodecForRed(const CodecList& codec_list, // Adds all codecs from `reference_codecs` to `offered_codecs` that don't // already exist in `offered_codecs` and ensure the payload types don't // collide. -webrtc::RTCError MergeCodecs(const CodecList& reference_codecs, - CodecList& offered_codecs, - std::string mid, - webrtc::PayloadTypeSuggester& pt_suggester) { +void MergeCodecs(const CodecList& reference_codecs, + CodecList& offered_codecs, + UsedPayloadTypes* used_pltypes) { // Add all new codecs that are not RTX/RED codecs. // The two-pass splitting of the loops means preferring payload types // of actual codecs with respect to collisions. @@ -667,11 +667,7 @@ webrtc::RTCError MergeCodecs(const CodecList& reference_codecs, reference_codec.GetResiliencyType() != Codec::ResiliencyType::kRed && !FindMatchingCodec(reference_codecs, offered_codecs, reference_codec)) { Codec codec = reference_codec; - auto id_or_error = pt_suggester.SuggestPayloadType(mid, codec); - if (!id_or_error.ok()) { - return id_or_error.MoveError(); - } - codec.id = id_or_error.value(); + used_pltypes->FindAndSetIdUsed(&codec); offered_codecs.push_back(codec); } } @@ -698,35 +694,15 @@ webrtc::RTCError MergeCodecs(const CodecList& reference_codecs, rtx_codec.params[kCodecParamAssociatedPayloadType] = rtc::ToString(matching_codec->id); - auto id_or_error = pt_suggester.SuggestPayloadType(mid, rtx_codec); - if (!id_or_error.ok()) { - return id_or_error.MoveError(); - } - rtx_codec.id = id_or_error.value(); + used_pltypes->FindAndSetIdUsed(&rtx_codec); offered_codecs.push_back(rtx_codec); } else if (reference_codec.GetResiliencyType() == Codec::ResiliencyType::kRed && !FindMatchingCodec(reference_codecs, offered_codecs, reference_codec)) { Codec red_codec = reference_codec; - const Codec* associated_codec = nullptr; - // Special case: For voice RED, if parameter is not set, look for - // OPUS as the matching codec. - // This is because we add the RED codec in audio engine init, but - // can't set the parameter until PTs are assigned. - if (red_codec.type == Codec::Type::kAudio && - red_codec.params.find(kCodecParamNotInNameValueFormat) == - red_codec.params.end()) { - for (const Codec& codec : reference_codecs) { - if (absl::EqualsIgnoreCase(codec.name, kOpusCodecName)) { - associated_codec = &codec; - break; - } - } - } else { - associated_codec = - GetAssociatedCodecForRed(reference_codecs, red_codec); - } + const Codec* associated_codec = + GetAssociatedCodecForRed(reference_codecs, red_codec); if (associated_codec) { std::optional matching_codec = FindMatchingCodec( reference_codecs, offered_codecs, *associated_codec); @@ -740,15 +716,11 @@ webrtc::RTCError MergeCodecs(const CodecList& reference_codecs, rtc::ToString(matching_codec->id) + "/" + rtc::ToString(matching_codec->id); } - auto id_or_error = pt_suggester.SuggestPayloadType(mid, red_codec); - if (!id_or_error.ok()) { - return id_or_error.MoveError(); - } - red_codec.id = id_or_error.value(); + used_pltypes->FindAndSetIdUsed(&red_codec); offered_codecs.push_back(red_codec); } } - return webrtc::RTCError::OK(); + offered_codecs.CheckConsistency(); } // `codecs` is a full list of codecs with correct payload type mappings, which @@ -835,28 +807,19 @@ CodecList MatchCodecPreference( } // Compute the union of `codecs1` and `codecs2`. -webrtc::RTCErrorOr ComputeCodecsUnion( - const CodecList& codecs1, - const CodecList& codecs2, - std::string mid, - webrtc::PayloadTypeSuggester& pt_suggester) { +CodecList ComputeCodecsUnion(const CodecList codecs1, const CodecList codecs2) { CodecList all_codecs; + UsedPayloadTypes used_payload_types; for (const Codec& codec : codecs1) { Codec codec_mutable = codec; - auto id_or_error = pt_suggester.SuggestPayloadType(mid, codec); - if (!id_or_error.ok()) { - return id_or_error.MoveError(); - } - codec_mutable.id = id_or_error.value(); + used_payload_types.FindAndSetIdUsed(&codec_mutable); all_codecs.push_back(codec_mutable); } // Use MergeCodecs to merge the second half of our list as it already checks // and fixes problems with duplicate payload types. - webrtc::RTCError error = MergeCodecs(codecs2, all_codecs, mid, pt_suggester); - if (!error.ok()) { - return error; - } + MergeCodecs(codecs2, all_codecs, &used_payload_types); + return all_codecs; } @@ -1250,8 +1213,7 @@ webrtc::RTCErrorOr GetNegotiatedCodecsForAnswer( const MediaSessionOptions& session_options, const ContentInfo* current_content, const CodecList& codecs, - const CodecList& supported_codecs, - webrtc::PayloadTypeSuggester& pt_suggester) { + const CodecList& supported_codecs) { CodecList filtered_codecs; if (!media_description_options.codec_preferences.empty()) { @@ -1290,19 +1252,7 @@ webrtc::RTCErrorOr GetNegotiatedCodecsForAnswer( // Use ComputeCodecsUnion to avoid having duplicate payload IDs. // This is a no-op for audio until RTX is added. - // TODO(hta): figure out why current_content is not always there. - std::string mid; - if (current_content) { - mid = current_content->name; - } else { - mid = ""; - } - auto codecs_or_error = - ComputeCodecsUnion(filtered_codecs, other_codecs, mid, pt_suggester); - if (!codecs_or_error.ok()) { - return codecs_or_error.MoveError(); - } - filtered_codecs = codecs_or_error.MoveValue(); + filtered_codecs = ComputeCodecsUnion(filtered_codecs, other_codecs); } if (media_description_options.type == MEDIA_TYPE_AUDIO && @@ -1379,7 +1329,6 @@ MediaSessionDescriptionFactory::MediaSessionDescriptionFactory( transport_desc_factory_->trials().IsEnabled( "WebRTC-PayloadTypesInTransport")) { RTC_CHECK(transport_desc_factory_); - RTC_CHECK(pt_suggester_); if (media_engine) { audio_send_codecs_ = CodecList(media_engine->voice().send_codecs()); audio_recv_codecs_ = CodecList(media_engine->voice().recv_codecs()); @@ -1868,29 +1817,20 @@ const CodecList& MediaSessionDescriptionFactory::GetVideoCodecsForAnswer( RTC_CHECK_NOTREACHED(); } -webrtc::RTCError MergeCodecsFromDescription( +void MergeCodecsFromDescription( const std::vector& current_active_contents, CodecList& audio_codecs, CodecList& video_codecs, - webrtc::PayloadTypeSuggester& pt_suggester) { + UsedPayloadTypes* used_pltypes) { for (const ContentInfo* content : current_active_contents) { if (IsMediaContentOfType(content, MEDIA_TYPE_AUDIO)) { - webrtc::RTCError error = - MergeCodecs(CodecList{content->media_description()->codecs()}, - audio_codecs, content->name, pt_suggester); - if (!error.ok()) { - return error; - } + MergeCodecs(CodecList{content->media_description()->codecs()}, + audio_codecs, used_pltypes); } else if (IsMediaContentOfType(content, MEDIA_TYPE_VIDEO)) { - webrtc::RTCError error = - MergeCodecs(CodecList{content->media_description()->codecs()}, - video_codecs, content->name, pt_suggester); - if (!error.ok()) { - return error; - } + MergeCodecs(CodecList{content->media_description()->codecs()}, + video_codecs, used_pltypes); } } - return webrtc::RTCError::OK(); } // Getting codecs for an offer involves these steps: @@ -1906,20 +1846,15 @@ void MediaSessionDescriptionFactory::GetCodecsForOffer( // First - get all codecs from the current description if the media type // is used. Add them to `used_pltypes` so the payload type is not reused if a // new media type is added. + UsedPayloadTypes used_pltypes; CodecList video_codec_list(*video_codecs); CodecList audio_codec_list(*audio_codecs); - webrtc::RTCError error = - MergeCodecsFromDescription(current_active_contents, audio_codec_list, - video_codec_list, *pt_suggester_); - RTC_CHECK(error.ok()); + MergeCodecsFromDescription(current_active_contents, audio_codec_list, + video_codec_list, &used_pltypes); // Add our codecs that are not in the current description. - error = MergeCodecs(CodecList{all_audio_codecs_}, audio_codec_list, "", - *pt_suggester_); - RTC_CHECK(error.ok()); - error = MergeCodecs(CodecList{all_video_codecs_}, video_codec_list, "", - *pt_suggester_); - RTC_CHECK(error.ok()); + MergeCodecs(CodecList{all_audio_codecs_}, audio_codec_list, &used_pltypes); + MergeCodecs(CodecList{all_video_codecs_}, video_codec_list, &used_pltypes); *audio_codecs = audio_codec_list.codecs(); *video_codecs = video_codec_list.codecs(); } @@ -1939,12 +1874,11 @@ void MediaSessionDescriptionFactory::GetCodecsForAnswer( // First - get all codecs from the current description if the media type // is used. Add them to `used_pltypes` so the payload type is not reused if a // new media type is added. + UsedPayloadTypes used_pltypes; CodecList video_codec_list(*video_codecs); CodecList audio_codec_list(*audio_codecs); - webrtc::RTCError error = - MergeCodecsFromDescription(current_active_contents, audio_codec_list, - video_codec_list, *pt_suggester_); - RTC_CHECK(error.ok()); + MergeCodecsFromDescription(current_active_contents, audio_codec_list, + video_codec_list, &used_pltypes); // Second - filter out codecs that we don't support at all and should ignore. CodecList filtered_offered_audio_codecs; @@ -1975,12 +1909,8 @@ void MediaSessionDescriptionFactory::GetCodecsForAnswer( // Add codecs that are not in the current description but were in // `remote_offer`. - error = MergeCodecs(filtered_offered_audio_codecs, audio_codec_list, "", - *pt_suggester_); - RTC_CHECK(error.ok()); - error = MergeCodecs(filtered_offered_video_codecs, video_codec_list, "", - *pt_suggester_); - RTC_CHECK(error.ok()); + MergeCodecs(filtered_offered_audio_codecs, audio_codec_list, &used_pltypes); + MergeCodecs(filtered_offered_video_codecs, video_codec_list, &used_pltypes); *audio_codecs = audio_codec_list.codecs(); *video_codecs = video_codec_list.codecs(); } @@ -2293,9 +2223,9 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForAnswer( ? GetAudioCodecsForAnswer(offer_rtd, answer_rtd) : GetVideoCodecsForAnswer(offer_rtd, answer_rtd); webrtc::RTCErrorOr> error_or_filtered_codecs = - GetNegotiatedCodecsForAnswer( - media_description_options, session_options, current_content, - CodecList(codecs), CodecList(supported_codecs), *pt_suggester_); + GetNegotiatedCodecsForAnswer(media_description_options, session_options, + current_content, CodecList(codecs), + CodecList(supported_codecs)); if (!error_or_filtered_codecs.ok()) { return error_or_filtered_codecs.MoveError(); } @@ -2527,10 +2457,9 @@ void MediaSessionDescriptionFactory::ComputeVideoCodecsIntersectionAndUnion() { video_sendrecv_codecs_.clear(); // Use ComputeCodecsUnion to avoid having duplicate payload IDs - auto error_or_codecs = ComputeCodecsUnion( - video_recv_codecs_, video_send_codecs_, "", *pt_suggester_); - RTC_CHECK(error_or_codecs.ok()); - all_video_codecs_ = error_or_codecs.MoveValue(); + all_video_codecs_ = ComputeCodecsUnion(CodecList(video_recv_codecs_), + CodecList(video_send_codecs_)); + // Use NegotiateCodecs to merge our codec lists, since the operation is // essentially the same. Put send_codecs as the offered_codecs, which is the // order we'd like to follow. The reasoning is that encoding is usually more diff --git a/pc/test/integration_test_helpers.cc b/pc/test/integration_test_helpers.cc index 885d654d37..39588d2317 100644 --- a/pc/test/integration_test_helpers.cc +++ b/pc/test/integration_test_helpers.cc @@ -20,7 +20,6 @@ #include "absl/functional/any_invocable.h" #include "api/audio/builtin_audio_processing_builder.h" #include "api/enable_media_with_defaults.h" -#include "api/field_trials_view.h" #include "api/jsep.h" #include "api/peer_connection_interface.h" #include "api/rtc_event_log/rtc_event_log_factory.h" diff --git a/pc/used_ids.h b/pc/used_ids.h index d55eead98e..42ef00a7c0 100644 --- a/pc/used_ids.h +++ b/pc/used_ids.h @@ -14,7 +14,9 @@ #include #include "api/rtp_parameters.h" +#include "media/base/codec.h" #include "rtc_base/checks.h" +#include "rtc_base/logging.h" namespace cricket { template @@ -86,6 +88,41 @@ class UsedIds { std::set id_set_; }; +// Helper class used for finding duplicate RTP payload types among audio, video +// and data codecs. When bundle is used the payload types may not collide. +class UsedPayloadTypes : public UsedIds { + public: + UsedPayloadTypes() + : 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". + if (new_id > kLastDynamicPayloadTypeLowerRange && + new_id < kFirstDynamicPayloadTypeUpperRange) + return true; + return UsedIds::IsIdUsed(new_id); + } + + private: + static const int kFirstDynamicPayloadTypeLowerRange = 35; + static const int kLastDynamicPayloadTypeLowerRange = 63; + + static const int kFirstDynamicPayloadTypeUpperRange = 96; + static const int kLastDynamicPayloadTypeUpperRange = 127; +}; + // Helper class used for finding duplicate RTP Header extension ids among // audio and video extensions. class UsedRtpHeaderExtensionIds : public UsedIds {