From b7abaee819771ca297bac4c51ec0daf62bd9a3fc Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Wed, 23 Oct 2024 10:23:35 +0000 Subject: [PATCH] Revert "Use Payload Type suggester for all codec merging" This reverts commit 0bac2aae596771db020f01a57fee4828081fbc38. Reason for revert: Suspected breakages downstream Original change's description: > Use Payload Type suggester for all codec merging > > Bug: webrtc:360058654 > Change-Id: Id475762253c427c1800c2352a60fc0121c2dc388 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/364783 > Reviewed-by: Florent Castelli > Commit-Queue: Harald Alvestrand > Cr-Commit-Position: refs/heads/main@{#43267} Bug: webrtc:360058654, b/375132036 Change-Id: Ieda626270193e7e6c93903b3c03a691b2bf0c1e2 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366540 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Commit-Queue: Harald Alvestrand Reviewed-by: Florent Castelli Cr-Commit-Position: refs/heads/main@{#43290} --- call/BUILD.gn | 11 --- call/DEPS | 3 - call/fake_payload_type_suggester.h | 45 --------- call/payload_type.h | 13 --- call/payload_type_picker.cc | 15 ++- media/BUILD.gn | 1 - media/DEPS | 3 - media/base/codec_comparators.cc | 5 - media/base/codec_comparators.h | 9 +- media/engine/fake_webrtc_call.h | 26 ++++- media/engine/webrtc_video_engine.cc | 39 ++------ pc/BUILD.gn | 7 -- pc/media_session.cc | 142 ++++++++-------------------- pc/media_session_unittest.cc | 90 +++++------------- pc/sdp_offer_answer.cc | 4 +- pc/test/integration_test_helpers.cc | 30 ------ pc/test/integration_test_helpers.h | 46 +++++++-- pc/used_ids.h | 37 ++++++++ 18 files changed, 183 insertions(+), 343 deletions(-) delete mode 100644 call/fake_payload_type_suggester.h diff --git a/call/BUILD.gn b/call/BUILD.gn index 5b21ddaaf9..0c4255a593 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -801,17 +801,6 @@ if (rtc_include_tests) { ] } - rtc_source_set("fake_payload_type_suggester") { - testonly = true - sources = [ "fake_payload_type_suggester.h" ] - deps = [ - ":payload_type", - ":payload_type_picker", - "../api:rtc_error", - "../media:codec", - ] - } - rtc_library("fake_network_pipe_unittests") { testonly = true diff --git a/call/DEPS b/call/DEPS index 98a8a4b68d..e74b22f677 100644 --- a/call/DEPS +++ b/call/DEPS @@ -70,8 +70,5 @@ specific_include_rules = { ], "call\.cc": [ "+media/base/codec.h", - ], - "fake_payload_type_suggester": [ - "+media/base/codec.h", ] } diff --git a/call/fake_payload_type_suggester.h b/call/fake_payload_type_suggester.h deleted file mode 100644 index c2aa6ebd01..0000000000 --- a/call/fake_payload_type_suggester.h +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef CALL_FAKE_PAYLOAD_TYPE_SUGGESTER_H_ -#define CALL_FAKE_PAYLOAD_TYPE_SUGGESTER_H_ - -#include - -#include "api/rtc_error.h" -#include "call/payload_type.h" -#include "call/payload_type_picker.h" -#include "media/base/codec.h" - -namespace webrtc { -// Fake payload type suggester, for use in tests. -// It uses a real PayloadTypePicker in order to do consistent PT -// assignment. -class FakePayloadTypeSuggester : public webrtc::PayloadTypeSuggester { - public: - webrtc::RTCErrorOr SuggestPayloadType( - const std::string& mid, - cricket::Codec codec) override { - // Ignores mid argument. - return pt_picker_.SuggestMapping(codec, nullptr); - } - webrtc::RTCError AddLocalMapping(const std::string& mid, - webrtc::PayloadType payload_type, - const cricket::Codec& codec) override { - return webrtc::RTCError::OK(); - } - - private: - webrtc::PayloadTypePicker pt_picker_; -}; - -} // namespace webrtc - -#endif // CALL_FAKE_PAYLOAD_TYPE_SUGGESTER_H_ diff --git a/call/payload_type.h b/call/payload_type.h index 635533327c..c0520e8f4c 100644 --- a/call/payload_type.h +++ b/call/payload_type.h @@ -26,24 +26,11 @@ class PayloadType : public StrongAlias { // removed once calling code is upgraded. PayloadType(uint8_t pt) { value_ = pt; } // NOLINT: explicit constexpr operator uint8_t() const& { return value_; } // NOLINT: Explicit - static bool IsValid(PayloadType id, bool rtcp_mux) { - if (rtcp_mux && (id > 63 && id < 96)) { - return false; - } - return id >= 0 && id <= 127; - } }; -// Does not compile either within or after class -// static const PayloadType kFirstDynamicPayloadTypeUpperRange{96}; -// static const PayloadType kLastDynamicPayloadTypeUpperRange{127}; -// static const PayloadType kFirstDynamicPayloadTypeLowerRange{35}; -// static const PayloadType kLastDynamicPayloadTypeLowerRange{63}; - class PayloadTypeSuggester { public: virtual ~PayloadTypeSuggester() = default; - // Suggest a payload type for a given codec on a given media section. // Media section is indicated by MID. // The function will either return a PT already in use on the connection diff --git a/call/payload_type_picker.cc b/call/payload_type_picker.cc index 915a3b8b22..12854cfec3 100644 --- a/call/payload_type_picker.cc +++ b/call/payload_type_picker.cc @@ -140,11 +140,10 @@ RTCErrorOr PayloadTypePicker::SuggestMapping( // The first matching entry is returned, unless excluder // maps it to something different. for (auto entry : entries_) { - if (MatchesWithReferenceAttributes(entry.codec(), codec)) { + if (MatchesWithCodecRules(entry.codec(), codec)) { if (excluder) { auto result = excluder->LookupCodec(entry.payload_type()); - if (result.ok() && - !MatchesWithReferenceAttributes(result.value(), codec)) { + if (result.ok() && !MatchesWithCodecRules(result.value(), codec)) { continue; } } @@ -165,7 +164,7 @@ RTCError PayloadTypePicker::AddMapping(PayloadType payload_type, // Multiple mappings for the same codec and the same PT are legal; for (auto entry : entries_) { if (payload_type == entry.payload_type() && - MatchesWithReferenceAttributes(codec, entry.codec())) { + MatchesWithCodecRules(codec, entry.codec())) { return RTCError::OK(); } } @@ -178,7 +177,7 @@ RTCError PayloadTypeRecorder::AddMapping(PayloadType payload_type, cricket::Codec codec) { auto existing_codec_it = payload_type_to_codec_.find(payload_type); if (existing_codec_it != payload_type_to_codec_.end() && - !MatchesWithReferenceAttributes(codec, existing_codec_it->second)) { + !MatchesWithCodecRules(codec, existing_codec_it->second)) { if (absl::EqualsIgnoreCase(codec.name, existing_codec_it->second.name)) { // The difference is in clock rate, channels or FMTP parameters. RTC_LOG(LS_INFO) << "Warning: Attempt to change a codec's parameters"; @@ -186,8 +185,8 @@ RTCError PayloadTypeRecorder::AddMapping(PayloadType payload_type, // This is done in production today, so we can't return an error. } else { RTC_LOG(LS_WARNING) << "Warning: You attempted to redefine a codec from " - << existing_codec_it->second << " to " - << " new codec " << codec; + << existing_codec_it->second.ToString() << " to " + << " new codec " << codec.ToString(); // This is a spec violation. // TODO: https://issues.webrtc.org/41480892 - return an error. } @@ -212,7 +211,7 @@ RTCErrorOr PayloadTypeRecorder::LookupPayloadType( auto result = std::find_if(payload_type_to_codec_.begin(), payload_type_to_codec_.end(), [codec](const auto& iter) { - return MatchesWithReferenceAttributes(iter.second, codec); + return MatchesWithCodecRules(iter.second, codec); }); if (result == payload_type_to_codec_.end()) { return RTCError(RTCErrorType::INVALID_PARAMETER, diff --git a/media/BUILD.gn b/media/BUILD.gn index 994dddb026..5126519d81 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -818,7 +818,6 @@ if (rtc_include_tests) { "../api/video:video_rtp_headers", "../api/video_codecs:video_codecs_api", "../call:call_interfaces", - "../call:fake_payload_type_suggester", "../call:mock_rtp_interfaces", "../call:payload_type", "../call:payload_type_picker", diff --git a/media/DEPS b/media/DEPS index 736601df27..12cad68da6 100644 --- a/media/DEPS +++ b/media/DEPS @@ -25,9 +25,6 @@ specific_include_rules = { ".*webrtc_video_engine\.h": [ "+video/config", ], - ".*webrtc_video_engine\.cc": [ - "+video/config", - ], ".*media_channel\.h": [ "+video/config", ], diff --git a/media/base/codec_comparators.cc b/media/base/codec_comparators.cc index 4ed48b96b3..38f133ac1c 100644 --- a/media/base/codec_comparators.cc +++ b/media/base/codec_comparators.cc @@ -342,11 +342,6 @@ bool MatchesWithCodecRules(const Codec& left_codec, const Codec& right_codec) { return matches_id && matches_type_specific(); } -bool MatchesWithReferenceAttributes(const Codec& codec1, const Codec& codec2) { - return MatchesWithReferenceAttributesAndComparator( - codec1, codec2, [](int a, int b) { return a == b; }); -} - // Finds a codec in `codecs2` that matches `codec_to_match`, which is // a member of `codecs1`. If `codec_to_match` is an RED or RTX codec, both // the codecs themselves and their associated codecs must match. diff --git a/media/base/codec_comparators.h b/media/base/codec_comparators.h index d832d151ef..75ba006ab1 100644 --- a/media/base/codec_comparators.h +++ b/media/base/codec_comparators.h @@ -19,15 +19,14 @@ namespace webrtc { +// Comparison used in the PayloadTypePicker +bool MatchesForSdp(const cricket::Codec& codec_1, + const cricket::Codec& codec_2); + // Comparison used for the Codec::Matches function bool MatchesWithCodecRules(const cricket::Codec& left_codec, const cricket::Codec& codec); -// Comparison that also checks on codecs referenced by PT in the -// fmtp line, as used with RED and RTX "codecs". -bool MatchesWithReferenceAttributes(const cricket::Codec& left_codec, - const cricket::Codec& right_codec); - // Finds a codec in `codecs2` that matches `codec_to_match`, which is // a member of `codecs1`. If `codec_to_match` is an RED or RTX codec, both // the codecs themselves and their associated codecs must match. diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h index 8f0b773902..c4e5f615a2 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -37,6 +37,7 @@ #include "api/environment/environment.h" #include "api/frame_transformer_interface.h" #include "api/media_types.h" +#include "api/rtc_error.h" #include "api/rtp_headers.h" #include "api/rtp_parameters.h" #include "api/rtp_sender_interface.h" @@ -53,14 +54,15 @@ #include "call/audio_receive_stream.h" #include "call/audio_send_stream.h" #include "call/call.h" -#include "call/fake_payload_type_suggester.h" #include "call/flexfec_receive_stream.h" #include "call/packet_receiver.h" #include "call/payload_type.h" +#include "call/payload_type_picker.h" #include "call/rtp_transport_controller_send_interface.h" #include "call/test/mock_rtp_transport_controller_send.h" #include "call/video_receive_stream.h" #include "call/video_send_stream.h" +#include "media/base/codec.h" #include "modules/rtp_rtcp/include/receive_statistics.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/buffer.h" @@ -393,6 +395,26 @@ class FakeFlexfecReceiveStream final : public webrtc::FlexfecReceiveStream { webrtc::FlexfecReceiveStream::Config config_; }; +// Fake payload type suggester. +// This is injected into FakeCall at initialization. +class FakePayloadTypeSuggester : public webrtc::PayloadTypeSuggester { + public: + webrtc::RTCErrorOr SuggestPayloadType( + const std::string& mid, + cricket::Codec codec) override { + // Ignores mid argument. + return pt_picker_.SuggestMapping(codec, nullptr); + } + webrtc::RTCError AddLocalMapping(const std::string& mid, + webrtc::PayloadType payload_type, + const cricket::Codec& codec) override { + return webrtc::RTCError::OK(); + } + + private: + webrtc::PayloadTypePicker pt_picker_; +}; + class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver { public: explicit FakeCall(const webrtc::Environment& env); @@ -536,7 +558,7 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver { int num_created_send_streams_; int num_created_receive_streams_; - webrtc::FakePayloadTypeSuggester pt_suggester_; + FakePayloadTypeSuggester pt_suggester_; }; } // namespace cricket diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index bfb2e22b46..27512b90ea 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -14,46 +14,28 @@ #include #include -#include #include -#include -#include #include #include #include +#include #include -#include #include "absl/algorithm/container.h" +#include "absl/container/inlined_vector.h" #include "absl/functional/bind_front.h" #include "absl/strings/match.h" -#include "absl/strings/string_view.h" -#include "api/array_view.h" -#include "api/crypto/crypto_options.h" -#include "api/crypto/frame_decryptor_interface.h" -#include "api/field_trials_view.h" -#include "api/frame_transformer_interface.h" #include "api/make_ref_counted.h" #include "api/media_stream_interface.h" #include "api/media_types.h" #include "api/priority.h" #include "api/rtc_error.h" -#include "api/rtp_headers.h" #include "api/rtp_parameters.h" -#include "api/rtp_sender_interface.h" #include "api/rtp_transceiver_direction.h" -#include "api/scoped_refptr.h" -#include "api/sequence_checker.h" -#include "api/task_queue/pending_task_safety_flag.h" -#include "api/task_queue/task_queue_base.h" -#include "api/transport/rtp/rtp_source.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" -#include "api/video/recordable_encoded_frame.h" -#include "api/video/video_bitrate_allocator_factory.h" +#include "api/video/resolution.h" #include "api/video/video_codec_type.h" -#include "api/video/video_sink_interface.h" -#include "api/video/video_source_interface.h" #include "api/video_codecs/scalability_mode.h" #include "api/video_codecs/sdp_video_format.h" #include "api/video_codecs/video_codec.h" @@ -61,30 +43,24 @@ #include "api/video_codecs/video_encoder.h" #include "api/video_codecs/video_encoder_factory.h" #include "call/call.h" -#include "call/flexfec_receive_stream.h" #include "call/packet_receiver.h" #include "call/receive_stream.h" #include "call/rtp_config.h" #include "call/rtp_transport_controller_send_interface.h" -#include "call/video_receive_stream.h" #include "call/video_send_stream.h" #include "common_video/frame_counts.h" +#include "common_video/include/quality_limitation_reason.h" #include "media/base/codec.h" #include "media/base/codec_comparators.h" #include "media/base/media_channel.h" -#include "media/base/media_channel_impl.h" -#include "media/base/media_config.h" #include "media/base/media_constants.h" -#include "media/base/media_engine.h" #include "media/base/rid_description.h" #include "media/base/rtp_utils.h" -#include "media/base/stream_params.h" #include "media/engine/webrtc_media_engine.h" #include "modules/rtp_rtcp/include/receive_statistics.h" +#include "modules/rtp_rtcp/include/report_block_data.h" #include "modules/rtp_rtcp/include/rtcp_statistics.h" -#include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" -#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_util.h" #include "modules/video_coding/svc/scalability_mode_util.h" #include "rtc_base/checks.h" @@ -93,10 +69,8 @@ #include "rtc_base/logging.h" #include "rtc_base/socket.h" #include "rtc_base/strings/string_builder.h" -#include "rtc_base/synchronization/mutex.h" #include "rtc_base/time_utils.h" #include "rtc_base/trace_event.h" -#include "video/config/video_encoder_config.h" namespace cricket { @@ -610,7 +584,8 @@ std::vector MapCodecs(const std::vector& codecs) { const int payload_type = in_codec.id; if (payload_codec_type.find(payload_type) != payload_codec_type.end()) { - RTC_LOG(LS_ERROR) << "Payload type already registered: " << in_codec; + RTC_LOG(LS_ERROR) << "Payload type already registered: " + << in_codec.ToString(); return {}; } payload_codec_type[payload_type] = in_codec.GetResiliencyType(); diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 894f8b7992..d1435115d5 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -2025,7 +2025,6 @@ if (rtc_include_tests && !build_with_chromium) { "../api/video:builtin_video_bitrate_allocator_factory", "../api/video:recordable_encoded_frame", "../api/video/test:mock_recordable_encoded_frame", - "../call:fake_payload_type_suggester", "../call:payload_type_picker", "../call:rtp_interfaces", "../call:rtp_receiver", @@ -2566,7 +2565,6 @@ if (rtc_include_tests && !build_with_chromium) { "../api:ice_transport_interface", "../api:libjingle_logging_api", "../api:libjingle_peerconnection_api", - "../api:make_ref_counted", "../api:media_stream_interface", "../api:mock_async_dns_resolver", "../api:mock_rtp", @@ -2577,7 +2575,6 @@ if (rtc_include_tests && !build_with_chromium) { "../api:rtp_sender_interface", "../api:rtp_transceiver_direction", "../api:scoped_refptr", - "../api:sequence_checker", "../api/audio:audio_device", "../api/audio:audio_mixer_api", "../api/audio:audio_processing", @@ -2585,7 +2582,6 @@ if (rtc_include_tests && !build_with_chromium) { "../api/crypto:frame_decryptor_interface", "../api/crypto:frame_encryptor_interface", "../api/crypto:options", - "../api/metronome", "../api/rtc_event_log", "../api/rtc_event_log:rtc_event_log_factory", "../api/task_queue", @@ -2632,8 +2628,6 @@ if (rtc_include_tests && !build_with_chromium) { "../rtc_base:rtc_json", "../rtc_base:safe_conversions", "../rtc_base:socket_address", - "../rtc_base:socket_factory", - "../rtc_base:socket_server", "../rtc_base:ssl", "../rtc_base:ssl_adapter", "../rtc_base:task_queue_for_test", @@ -2651,7 +2645,6 @@ if (rtc_include_tests && !build_with_chromium) { "../test/pc/sctp:fake_sctp_transport", "../test/time_controller", "//third_party/abseil-cpp/absl/algorithm:container", - "//third_party/abseil-cpp/absl/functional:any_invocable", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings:string_view", ] diff --git a/pc/media_session.cc b/pc/media_session.cc index 1c1ac3286a..3d6eccda06 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -495,6 +495,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()); } } @@ -640,10 +641,9 @@ const Codec* GetAssociatedCodecForRed(const std::vector& 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 std::vector& reference_codecs, - std::vector* offered_codecs, - std::string mid, - webrtc::PayloadTypeSuggester& pt_suggester) { +void MergeCodecs(const std::vector& reference_codecs, + std::vector* 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. @@ -653,11 +653,7 @@ webrtc::RTCError MergeCodecs(const std::vector& reference_codecs, !webrtc::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); } } @@ -685,35 +681,15 @@ webrtc::RTCError MergeCodecs(const std::vector& 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 && !webrtc::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 = webrtc::FindMatchingCodec( reference_codecs, *offered_codecs, *associated_codec); @@ -727,15 +703,10 @@ webrtc::RTCError MergeCodecs(const std::vector& 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(); } // `codecs` is a full list of codecs with correct payload type mappings, which @@ -824,28 +795,20 @@ std::vector MatchCodecPreference( } // Compute the union of `codecs1` and `codecs2`. -webrtc::RTCErrorOr> ComputeCodecsUnion( - const std::vector& codecs1, - const std::vector& codecs2, - std::string mid, - webrtc::PayloadTypeSuggester& pt_suggester) { +std::vector ComputeCodecsUnion(const std::vector& codecs1, + const std::vector& codecs2) { std::vector 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; } @@ -1184,8 +1147,7 @@ webrtc::RTCErrorOr GetNegotiatedCodecsForAnswer( const MediaSessionOptions& session_options, const ContentInfo* current_content, const std::vector& codecs, - const std::vector& supported_codecs, - webrtc::PayloadTypeSuggester& pt_suggester) { + const std::vector& supported_codecs) { std::vector filtered_codecs; if (!media_description_options.codec_preferences.empty()) { @@ -1225,19 +1187,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 && @@ -1314,7 +1264,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_ = media_engine->voice().send_codecs(); audio_recv_codecs_ = media_engine->voice().recv_codecs(); @@ -1777,29 +1726,20 @@ const Codecs& MediaSessionDescriptionFactory::GetVideoCodecsForAnswer( RTC_CHECK_NOTREACHED(); } -webrtc::RTCError MergeCodecsFromDescription( +void MergeCodecsFromDescription( const std::vector& current_active_contents, Codecs* audio_codecs, Codecs* 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(content->media_description()->codecs(), audio_codecs, - content->name, pt_suggester); - if (!error.ok()) { - return error; - } + MergeCodecs(content->media_description()->codecs(), audio_codecs, + used_pltypes); } else if (IsMediaContentOfType(content, MEDIA_TYPE_VIDEO)) { - webrtc::RTCError error = - MergeCodecs(content->media_description()->codecs(), video_codecs, - content->name, pt_suggester); - if (!error.ok()) { - return error; - } + MergeCodecs(content->media_description()->codecs(), video_codecs, + used_pltypes); } } - return webrtc::RTCError::OK(); } // Getting codecs for an offer involves these steps: @@ -1815,15 +1755,13 @@ 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. - webrtc::RTCError error = MergeCodecsFromDescription( - current_active_contents, audio_codecs, video_codecs, *pt_suggester_); - RTC_CHECK(error.ok()); + UsedPayloadTypes used_pltypes; + MergeCodecsFromDescription(current_active_contents, audio_codecs, + video_codecs, &used_pltypes); // Add our codecs that are not in the current description. - error = MergeCodecs(all_audio_codecs_, audio_codecs, "", *pt_suggester_); - RTC_CHECK(error.ok()); - error = MergeCodecs(all_video_codecs_, video_codecs, "", *pt_suggester_); - RTC_CHECK(error.ok()); + MergeCodecs(all_audio_codecs_, audio_codecs, &used_pltypes); + MergeCodecs(all_video_codecs_, video_codecs, &used_pltypes); } // Getting codecs for an answer involves these steps: @@ -1841,9 +1779,9 @@ 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. - webrtc::RTCError error = MergeCodecsFromDescription( - current_active_contents, audio_codecs, video_codecs, *pt_suggester_); - RTC_CHECK(error.ok()); + UsedPayloadTypes used_pltypes; + MergeCodecsFromDescription(current_active_contents, audio_codecs, + video_codecs, &used_pltypes); // Second - filter out codecs that we don't support at all and should ignore. Codecs filtered_offered_audio_codecs; @@ -1876,12 +1814,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_codecs, "", - *pt_suggester_); - RTC_CHECK(error.ok()); - error = MergeCodecs(filtered_offered_video_codecs, video_codecs, "", - *pt_suggester_); - RTC_CHECK(error.ok()); + MergeCodecs(filtered_offered_audio_codecs, audio_codecs, &used_pltypes); + MergeCodecs(filtered_offered_video_codecs, video_codecs, &used_pltypes); } MediaSessionDescriptionFactory::AudioVideoRtpHeaderExtensions @@ -2189,8 +2123,7 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForAnswer( : GetVideoCodecsForAnswer(offer_rtd, answer_rtd); webrtc::RTCErrorOr> error_or_filtered_codecs = GetNegotiatedCodecsForAnswer(media_description_options, session_options, - current_content, codecs, supported_codecs, - *pt_suggester_); + current_content, codecs, supported_codecs); if (!error_or_filtered_codecs.ok()) { return error_or_filtered_codecs.MoveError(); } @@ -2412,10 +2345,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(video_recv_codecs_, 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/media_session_unittest.cc b/pc/media_session_unittest.cc index 71172ccd54..c17ab75570 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -23,13 +23,11 @@ #include "absl/algorithm/container.h" #include "absl/strings/match.h" #include "absl/strings/string_view.h" -#include "api/array_view.h" #include "api/audio_codecs/audio_format.h" #include "api/candidate.h" #include "api/media_types.h" #include "api/rtp_parameters.h" #include "api/rtp_transceiver_direction.h" -#include "call/fake_payload_type_suggester.h" #include "media/base/codec.h" #include "media/base/media_constants.h" #include "media/base/rid_description.h" @@ -119,26 +117,6 @@ const Codec kVideoCodecs2[] = {CreateVideoCodec(126, "H264"), const Codec kVideoCodecsAnswer[] = {CreateVideoCodec(97, "H264")}; -// Match two codec lists for content, but ignore the ID. -bool CodecListsMatch(rtc::ArrayView list1, - rtc::ArrayView list2) { - if (list1.size() != list2.size()) { - return false; - } - for (size_t i = 0; i < list1.size(); ++i) { - Codec codec1 = list1[i]; - Codec codec2 = list2[i]; - codec1.id = Codec::kIdNotSet; - codec2.id = Codec::kIdNotSet; - if (codec1 != codec2) { - RTC_LOG(LS_ERROR) << "Mismatch at position " << i << " between " << codec1 - << " and " << codec2; - return false; - } - } - return true; -} - const RtpExtension kAudioRtpExtension1[] = { RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 8), RtpExtension("http://google.com/testing/audio_something", 10), @@ -460,8 +438,8 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { MediaSessionDescriptionFactoryTest() : tdf1_(field_trials), tdf2_(field_trials), - f1_(nullptr, false, &ssrc_generator1, &tdf1_, &pt_suggester_1_), - f2_(nullptr, false, &ssrc_generator2, &tdf2_, &pt_suggester_2_) { + f1_(nullptr, false, &ssrc_generator1, &tdf1_, nullptr), + f2_(nullptr, false, &ssrc_generator2, &tdf2_, nullptr) { f1_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs1), MAKE_VECTOR(kAudioCodecs1)); f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1), @@ -710,8 +688,6 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { UniqueRandomIdGenerator ssrc_generator2; TransportDescriptionFactory tdf1_; TransportDescriptionFactory tdf2_; - webrtc::FakePayloadTypeSuggester pt_suggester_1_; - webrtc::FakePayloadTypeSuggester pt_suggester_2_; MediaSessionDescriptionFactory f1_; MediaSessionDescriptionFactory f2_; }; @@ -2971,11 +2947,11 @@ TEST_F(MediaSessionDescriptionFactoryTest, const AudioContentDescription* updated_acd = GetFirstAudioContentDescription(updated_offer.get()); - EXPECT_TRUE(CodecListsMatch(updated_acd->codecs(), kUpdatedAudioCodecOffer)); + EXPECT_THAT(updated_acd->codecs(), ElementsAreArray(kUpdatedAudioCodecOffer)); const VideoContentDescription* updated_vcd = GetFirstVideoContentDescription(updated_offer.get()); - EXPECT_TRUE(CodecListsMatch(updated_vcd->codecs(), kUpdatedVideoCodecOffer)); + EXPECT_THAT(updated_vcd->codecs(), ElementsAreArray(kUpdatedVideoCodecOffer)); } // Test that a reoffer does not reuse audio codecs from a previous media section @@ -3003,13 +2979,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, // section was not recycled the payload types would match the initial offerer. const AudioContentDescription* acd = GetFirstAudioContentDescription(reoffer.get()); - // EXPECT_THAT(acd->codecs(), ElementsAreArray(kAudioCodecs2)), - // except that we don't want to check the PT numbers. - EXPECT_EQ(acd->codecs().size(), - sizeof(kAudioCodecs2) / sizeof(kAudioCodecs2[0])); - for (size_t i = 0; i < acd->codecs().size(); ++i) { - EXPECT_EQ(acd->codecs()[i].name, kAudioCodecs2[i].name); - } + EXPECT_THAT(acd->codecs(), ElementsAreArray(kAudioCodecs2)); } // Test that a reoffer does not reuse video codecs from a previous media section @@ -3036,7 +3006,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, // section was not recycled the payload types would match the initial offerer. const VideoContentDescription* vcd = GetFirstVideoContentDescription(reoffer.get()); - EXPECT_TRUE(CodecListsMatch(vcd->codecs(), kVideoCodecs2)); + EXPECT_THAT(vcd->codecs(), ElementsAreArray(kVideoCodecs2)); } // Test that a reanswer does not reuse audio codecs from a previous media @@ -3134,7 +3104,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, std::vector expected_codecs = MAKE_VECTOR(kVideoCodecsAnswer); AddRtxCodec(CreateVideoRtxCodec(126, kVideoCodecs1[1].id), &expected_codecs); - EXPECT_TRUE(CodecListsMatch(expected_codecs, vcd->codecs())); + EXPECT_EQ(expected_codecs, vcd->codecs()); // Now, make sure we get same result (except for the order) if `f2_` creates // an updated offer even though the default payload types between `f1_` and @@ -3149,7 +3119,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, const VideoContentDescription* updated_vcd = GetFirstVideoContentDescription(updated_answer.get()); - EXPECT_TRUE(CodecListsMatch(expected_codecs, updated_vcd->codecs())); + EXPECT_EQ(expected_codecs, updated_vcd->codecs()); } // Regression test for: @@ -3304,7 +3274,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, // New offer should attempt to add H263, and RTX for H264. expected_codecs.push_back(kVideoCodecs2[1]); AddRtxCodec(CreateVideoRtxCodec(125, kVideoCodecs1[1].id), &expected_codecs); - EXPECT_TRUE(CodecListsMatch(expected_codecs, updated_vcd->codecs())); + EXPECT_EQ(expected_codecs, updated_vcd->codecs()); } // Test that RTX is ignored when there is no associated payload type parameter. @@ -3413,7 +3383,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, std::vector expected_codecs = MAKE_VECTOR(kVideoCodecsAnswer); AddRtxCodec(CreateVideoRtxCodec(126, kVideoCodecs1[1].id), &expected_codecs); - EXPECT_TRUE(CodecListsMatch(expected_codecs, vcd->codecs())); + EXPECT_EQ(expected_codecs, vcd->codecs()); } // Test that after one RTX codec has been negotiated, a new offer can attempt @@ -3436,7 +3406,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, AddSecondRtxInNewOffer) { std::vector expected_codecs = MAKE_VECTOR(kVideoCodecs1); AddRtxCodec(CreateVideoRtxCodec(126, kVideoCodecs1[1].id), &expected_codecs); - EXPECT_TRUE(CodecListsMatch(expected_codecs, vcd->codecs())); + EXPECT_EQ(expected_codecs, vcd->codecs()); // Now, attempt to add RTX for H264-SVC. AddRtxCodec(CreateVideoRtxCodec(125, kVideoCodecs1[0].id), &f1_codecs); @@ -3448,7 +3418,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, AddSecondRtxInNewOffer) { vcd = GetFirstVideoContentDescription(updated_offer.get()); AddRtxCodec(CreateVideoRtxCodec(125, kVideoCodecs1[0].id), &expected_codecs); - EXPECT_TRUE(CodecListsMatch(expected_codecs, vcd->codecs())); + EXPECT_EQ(expected_codecs, vcd->codecs()); } // Test that when RTX is used in conjunction with simulcast, an RTX ssrc is @@ -4537,8 +4507,8 @@ class MediaProtocolTest : public testing::TestWithParam { MediaProtocolTest() : tdf1_(field_trials_), tdf2_(field_trials_), - f1_(nullptr, false, &ssrc_generator1, &tdf1_, &pt_suggester_1_), - f2_(nullptr, false, &ssrc_generator2, &tdf2_, &pt_suggester_2_) { + f1_(nullptr, false, &ssrc_generator1, &tdf1_, nullptr), + f2_(nullptr, false, &ssrc_generator2, &tdf2_, nullptr) { f1_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs1), MAKE_VECTOR(kAudioCodecs1)); f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1), @@ -4557,8 +4527,6 @@ class MediaProtocolTest : public testing::TestWithParam { webrtc::test::ScopedKeyValueConfig field_trials_; TransportDescriptionFactory tdf1_; TransportDescriptionFactory tdf2_; - webrtc::FakePayloadTypeSuggester pt_suggester_1_; - webrtc::FakePayloadTypeSuggester pt_suggester_2_; MediaSessionDescriptionFactory f1_; MediaSessionDescriptionFactory f2_; UniqueRandomIdGenerator ssrc_generator1; @@ -4603,9 +4571,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestSetAudioCodecs) { std::unique_ptr(new rtc::FakeSSLIdentity("id")))); UniqueRandomIdGenerator ssrc_generator; - webrtc::FakePayloadTypeSuggester pt_suggester; MediaSessionDescriptionFactory sf(nullptr, false, &ssrc_generator, &tdf, - &pt_suggester); + nullptr); std::vector send_codecs = MAKE_VECTOR(kAudioCodecs1); std::vector recv_codecs = MAKE_VECTOR(kAudioCodecs2); @@ -4676,9 +4643,8 @@ void TestAudioCodecsOffer(RtpTransceiverDirection direction) { std::unique_ptr(new rtc::FakeSSLIdentity("id")))); UniqueRandomIdGenerator ssrc_generator; - webrtc::FakePayloadTypeSuggester pt_suggester; MediaSessionDescriptionFactory sf(nullptr, false, &ssrc_generator, &tdf, - &pt_suggester); + nullptr); const std::vector send_codecs = MAKE_VECTOR(kAudioCodecs1); const std::vector recv_codecs = MAKE_VECTOR(kAudioCodecs2); const std::vector sendrecv_codecs = MAKE_VECTOR(kAudioCodecsAnswer); @@ -4719,15 +4685,13 @@ void TestAudioCodecsOffer(RtpTransceiverDirection direction) { } } -// Since the PT suggester reserves the static range for specific codecs, -// PT numbers from the 36-63 range are used. -const Codec kOfferAnswerCodecs[] = {CreateAudioCodec(40, "codec0", 16000, 1), - CreateAudioCodec(41, "codec1", 8000, 1), - CreateAudioCodec(42, "codec2", 8000, 1), - CreateAudioCodec(43, "codec3", 8000, 1), - CreateAudioCodec(44, "codec4", 8000, 2), - CreateAudioCodec(45, "codec5", 32000, 1), - CreateAudioCodec(46, "codec6", 48000, 1)}; +const Codec kOfferAnswerCodecs[] = {CreateAudioCodec(0, "codec0", 16000, 1), + CreateAudioCodec(1, "codec1", 8000, 1), + CreateAudioCodec(2, "codec2", 8000, 1), + CreateAudioCodec(3, "codec3", 8000, 1), + CreateAudioCodec(4, "codec4", 8000, 2), + CreateAudioCodec(5, "codec5", 32000, 1), + CreateAudioCodec(6, "codec6", 48000, 1)}; /* The codecs groups below are chosen as per the matrix below. The objective * is to have different sets of codecs in the inputs, to get unique sets of @@ -4783,12 +4747,10 @@ void TestAudioCodecsAnswer(RtpTransceiverDirection offer_direction, rtc::RTCCertificate::Create(std::unique_ptr( new rtc::FakeSSLIdentity("answer_id")))); UniqueRandomIdGenerator ssrc_generator1, ssrc_generator2; - webrtc::FakePayloadTypeSuggester offer_pt_suggester; MediaSessionDescriptionFactory offer_factory(nullptr, false, &ssrc_generator1, - &offer_tdf, &offer_pt_suggester); - webrtc::FakePayloadTypeSuggester answer_pt_suggester; + &offer_tdf, nullptr); MediaSessionDescriptionFactory answer_factory( - nullptr, false, &ssrc_generator2, &answer_tdf, &answer_pt_suggester); + nullptr, false, &ssrc_generator2, &answer_tdf, nullptr); offer_factory.set_audio_codecs( VectorFromIndices(kOfferAnswerCodecs, kOfferSendCodecs), @@ -4871,7 +4833,7 @@ void TestAudioCodecsAnswer(RtpTransceiverDirection offer_direction, bool first = true; os << "{"; for (const auto& c : codecs) { - os << (first ? " " : ", ") << c.id << ":" << c.name; + os << (first ? " " : ", ") << c.id; first = false; } os << " }"; diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index b4e774d53c..0542910941 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -84,6 +84,7 @@ #include "pc/stream_collection.h" #include "pc/transceiver_list.h" #include "pc/usage_pattern.h" +#include "pc/used_ids.h" #include "pc/webrtc_session_description_factory.h" #include "rtc_base/checks.h" #include "rtc_base/crypto_random.h" @@ -591,7 +592,8 @@ RTCError ValidatePayloadTypes(const cricket::SessionDescription& description) { if (type == cricket::MEDIA_TYPE_AUDIO || type == cricket::MEDIA_TYPE_VIDEO) { for (const auto& codec : media_description->codecs()) { - if (!PayloadType::IsValid(codec.id, media_description->rtcp_mux())) { + 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/test/integration_test_helpers.cc b/pc/test/integration_test_helpers.cc index bd9298c3b1..525a370a40 100644 --- a/pc/test/integration_test_helpers.cc +++ b/pc/test/integration_test_helpers.cc @@ -10,37 +10,7 @@ #include "pc/test/integration_test_helpers.h" -#include -#include -#include -#include -#include -#include - -#include "absl/functional/any_invocable.h" #include "api/audio/builtin_audio_processing_factory.h" -#include "api/enable_media_with_defaults.h" -#include "api/jsep.h" -#include "api/peer_connection_interface.h" -#include "api/rtc_event_log/rtc_event_log_factory.h" -#include "api/sequence_checker.h" -#include "api/stats/rtcstats_objects.h" -#include "api/task_queue/default_task_queue_factory.h" -#include "api/task_queue/pending_task_safety_flag.h" -#include "api/task_queue/task_queue_base.h" -#include "api/transport/field_trial_based_config.h" -#include "api/units/time_delta.h" -#include "logging/rtc_event_log/fake_rtc_event_log_factory.h" -#include "p2p/base/basic_packet_socket_factory.h" -#include "p2p/base/port_allocator.h" -#include "p2p/client/basic_port_allocator.h" -#include "pc/peer_connection_factory.h" -#include "pc/test/fake_audio_capture_module.h" -#include "rtc_base/checks.h" -#include "rtc_base/fake_network.h" -#include "rtc_base/socket_server.h" -#include "rtc_base/thread.h" -#include "test/gtest.h" namespace webrtc { diff --git a/pc/test/integration_test_helpers.h b/pc/test/integration_test_helpers.h index 5f10dc5319..15bbc1ab60 100644 --- a/pc/test/integration_test_helpers.h +++ b/pc/test/integration_test_helpers.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -26,44 +27,66 @@ #include #include -#include "absl/functional/any_invocable.h" +#include "absl/algorithm/container.h" #include "absl/memory/memory.h" #include "absl/strings/string_view.h" +#include "api/audio/audio_device.h" +#include "api/audio/audio_processing.h" #include "api/audio_options.h" #include "api/candidate.h" #include "api/crypto/crypto_options.h" #include "api/data_channel_interface.h" +#include "api/enable_media_with_defaults.h" #include "api/field_trials_view.h" #include "api/ice_transport_interface.h" #include "api/jsep.h" -#include "api/make_ref_counted.h" #include "api/media_stream_interface.h" #include "api/media_types.h" -#include "api/metronome/metronome.h" #include "api/peer_connection_interface.h" #include "api/rtc_error.h" +#include "api/rtc_event_log/rtc_event_log_factory.h" +#include "api/rtc_event_log/rtc_event_log_factory_interface.h" #include "api/rtc_event_log_output.h" #include "api/rtp_receiver_interface.h" #include "api/rtp_sender_interface.h" #include "api/rtp_transceiver_interface.h" #include "api/scoped_refptr.h" -#include "api/sequence_checker.h" +#include "api/stats/rtc_stats.h" #include "api/stats/rtc_stats_report.h" #include "api/stats/rtcstats_objects.h" +#include "api/task_queue/default_task_queue_factory.h" #include "api/task_queue/pending_task_safety_flag.h" +#include "api/task_queue/task_queue_factory.h" #include "api/test/mock_async_dns_resolver.h" +#include "api/transport/field_trial_based_config.h" +#include "api/uma_metrics.h" #include "api/units/time_delta.h" #include "api/video/video_rotation.h" +#include "api/video_codecs/sdp_video_format.h" +#include "api/video_codecs/video_decoder_factory.h" +#include "api/video_codecs/video_encoder_factory.h" +#include "call/call.h" #include "logging/rtc_event_log/fake_rtc_event_log_factory.h" +#include "media/base/media_engine.h" #include "media/base/stream_params.h" +#include "media/engine/fake_webrtc_video_engine.h" #include "p2p/base/fake_ice_transport.h" #include "p2p/base/ice_transport_internal.h" +#include "p2p/base/p2p_constants.h" #include "p2p/base/port.h" #include "p2p/base/port_allocator.h" #include "p2p/base/port_interface.h" +#include "p2p/base/test_stun_server.h" #include "p2p/base/test_turn_customizer.h" #include "p2p/base/test_turn_server.h" +#include "p2p/client/basic_port_allocator.h" +#include "pc/dtmf_sender.h" +#include "pc/local_audio_source.h" +#include "pc/media_session.h" +#include "pc/peer_connection.h" #include "pc/peer_connection_factory.h" +#include "pc/peer_connection_proxy.h" +#include "pc/rtp_media_utils.h" #include "pc/session_description.h" #include "pc/test/fake_audio_capture_module.h" #include "pc/test/fake_periodic_video_source.h" @@ -74,23 +97,28 @@ #include "pc/video_track_source.h" #include "rtc_base/checks.h" #include "rtc_base/crypto_random.h" +#include "rtc_base/event.h" +#include "rtc_base/fake_clock.h" #include "rtc_base/fake_mdns_responder.h" #include "rtc_base/fake_network.h" #include "rtc_base/firewall_socket_server.h" #include "rtc_base/gunit.h" #include "rtc_base/ip_address.h" #include "rtc_base/logging.h" +#include "rtc_base/mdns_responder_interface.h" +#include "rtc_base/numerics/safe_conversions.h" +#include "rtc_base/rtc_certificate_generator.h" #include "rtc_base/socket_address.h" -#include "rtc_base/socket_factory.h" -#include "rtc_base/socket_server.h" #include "rtc_base/ssl_stream_adapter.h" #include "rtc_base/task_queue_for_test.h" +#include "rtc_base/task_utils/repeating_task.h" +#include "rtc_base/test_certificate_verifier.h" #include "rtc_base/thread.h" +#include "rtc_base/thread_annotations.h" #include "rtc_base/time_utils.h" #include "rtc_base/virtual_socket_server.h" #include "system_wrappers/include/metrics.h" #include "test/gmock.h" -#include "test/gtest.h" #include "test/scoped_key_value_config.h" namespace webrtc { @@ -638,7 +666,9 @@ class PeerConnectionIntegrationWrapper : public PeerConnectionObserver, bool SetRemoteDescription(std::unique_ptr desc) { auto observer = rtc::make_ref_counted(); - RTC_LOG(LS_INFO) << debug_name_ << ": SetRemoteDescription SDP:" << desc; + std::string str; + desc->ToString(&str); + RTC_LOG(LS_INFO) << debug_name_ << ": SetRemoteDescription SDP:\n" << str; pc()->SetRemoteDescription(std::move(desc), observer); // desc.release()); RemoveUnusedVideoRenderers(); EXPECT_TRUE_WAIT(observer->called(), kDefaultTimeout); 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 {