diff --git a/call/BUILD.gn b/call/BUILD.gn index 8f37af0be1..7637f4f969 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -773,17 +773,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 9bd3d3bd07..c0520e8f4c 100644 --- a/call/payload_type.h +++ b/call/payload_type.h @@ -26,18 +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; - } }; 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 d26abb754d..3e75db4c37 100644 --- a/call/payload_type_picker.cc +++ b/call/payload_type_picker.cc @@ -12,7 +12,6 @@ #include #include -#include #include #include @@ -48,78 +47,21 @@ struct MapTableEntry { int payload_type; }; -// Helper function to determine whether a codec should use the [35, 63] range. -// Should be used when adding new codecs (or variants). -bool CodecPrefersLowerRange(const cricket::Codec& codec) { - // All audio codecs prefer upper range. - if (codec.type == cricket::Codec::Type::kAudio) { - return false; - } - if (absl::EqualsIgnoreCase(codec.name, cricket::kFlexfecCodecName) || - absl::EqualsIgnoreCase(codec.name, cricket::kAv1CodecName)) { - return true; - } else if (absl::EqualsIgnoreCase(codec.name, cricket::kH264CodecName)) { - std::string profile_level_id; - std::string packetization_mode; - - if (codec.GetParam(cricket::kH264FmtpProfileLevelId, &profile_level_id)) { - if (absl::StartsWithIgnoreCase(profile_level_id, "4d00")) { - if (codec.GetParam(cricket::kH264FmtpPacketizationMode, - &packetization_mode)) { - return packetization_mode == "0"; - } - } - // H264 with YUV444. - return absl::StartsWithIgnoreCase(profile_level_id, "f400"); - } - } else if (absl::EqualsIgnoreCase(codec.name, cricket::kVp9CodecName)) { - std::string profile_id; - - if (codec.GetParam(cricket::kVP9ProfileId, &profile_id)) { - if (profile_id.compare("1") == 0 || profile_id.compare("3") == 0) { - return true; - } - } - } - return false; -} - -RTCErrorOr FindFreePayloadType(const cricket::Codec& codec, - std::set seen_pt) { - // Prefer to use lower range for codecs that can handle it. - bool prefer_lower_range = CodecPrefersLowerRange(codec); - if (prefer_lower_range) { - for (auto i = kFirstDynamicPayloadTypeLowerRange; - i <= kLastDynamicPayloadTypeLowerRange; i++) { - if (seen_pt.count(PayloadType(i)) == 0) { - return PayloadType(i); - } - } - } +RTCErrorOr FindFreePayloadType(std::set seen_pt) { for (auto i = kFirstDynamicPayloadTypeUpperRange; - i <= kLastDynamicPayloadTypeUpperRange; i++) { + i < kLastDynamicPayloadTypeUpperRange; i++) { if (seen_pt.count(PayloadType(i)) == 0) { return PayloadType(i); } } - // If the upper range is full, we do lower range also for codecs - // that prefer the upper range. - if (!prefer_lower_range) { - for (auto i = kFirstDynamicPayloadTypeLowerRange; - i <= kLastDynamicPayloadTypeLowerRange; i++) { - if (seen_pt.count(PayloadType(i)) == 0) { - return PayloadType(i); - } + for (auto i = kFirstDynamicPayloadTypeLowerRange; + i < kLastDynamicPayloadTypeLowerRange; i++) { + if (seen_pt.count(PayloadType(i)) == 0) { + return PayloadType(i); } } - if (prefer_lower_range) { - return RTCError(RTCErrorType::RESOURCE_EXHAUSTED, - "All available dynamic PTs have been assigned"); - } else { - return RTCError( - RTCErrorType::RESOURCE_EXHAUSTED, - "All available dynamic PTs have been assigned, codec preferred upper"); - } + return RTCError(RTCErrorType::RESOURCE_EXHAUSTED, + "All available dynamic PTs have been assigned"); } } // namespace @@ -199,11 +141,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; } } @@ -211,8 +152,7 @@ RTCErrorOr PayloadTypePicker::SuggestMapping( } } // Assign the first free payload type. - RTCErrorOr found_pt = - FindFreePayloadType(codec, seen_payload_types_); + RTCErrorOr found_pt = FindFreePayloadType(seen_payload_types_); if (found_pt.ok()) { AddMapping(found_pt.value(), codec); } @@ -225,7 +165,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(); } } @@ -285,7 +225,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 41ace6d420..1b8f4cc12d 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -824,7 +824,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 10636cc5af..663bf7feef 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 45a1b9b8cf..49525f48f4 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 64423c7346..a797dc8375 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 28dbe294ca..04724d831f 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" @@ -395,6 +397,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); @@ -541,7 +563,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 b62714fe04..aaf14df41b 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,31 +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/payload_type_picker.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" @@ -94,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 { @@ -108,6 +81,10 @@ using ::webrtc::ParseRtpSsrc; constexpr int64_t kUnsignaledSsrcCooldownMs = rtc::kNumMillisecsPerSec / 2; +// TODO(bugs.webrtc.org/13166): Remove AV1X when backwards compatibility is not +// needed. +constexpr char kAv1xCodecName[] = "AV1X"; + // This constant is really an on/off, lower-level configurable NACK history // duration hasn't been implemented. const int kNackHistoryMs = 1000; @@ -158,6 +135,38 @@ void AddDefaultFeedbackParams(Codec* codec, } } +// Helper function to determine whether a codec should use the [35, 63] range. +// Should be used when adding new codecs (or variants). +bool IsCodecValidForLowerRange(const Codec& codec) { + if (absl::EqualsIgnoreCase(codec.name, kFlexfecCodecName) || + absl::EqualsIgnoreCase(codec.name, kAv1CodecName) || + absl::EqualsIgnoreCase(codec.name, kAv1xCodecName)) { + return true; + } else if (absl::EqualsIgnoreCase(codec.name, kH264CodecName)) { + std::string profile_level_id; + std::string packetization_mode; + + if (codec.GetParam(kH264FmtpProfileLevelId, &profile_level_id)) { + if (absl::StartsWithIgnoreCase(profile_level_id, "4d00")) { + if (codec.GetParam(kH264FmtpPacketizationMode, &packetization_mode)) { + return packetization_mode == "0"; + } + } + // H264 with YUV444. + return absl::StartsWithIgnoreCase(profile_level_id, "f400"); + } + } else if (absl::EqualsIgnoreCase(codec.name, kVp9CodecName)) { + std::string profile_id; + + if (codec.GetParam(kVP9ProfileId, &profile_id)) { + if (profile_id.compare("1") == 0 || profile_id.compare("3") == 0) { + return true; + } + } + } + return false; +} + // Get the default set of supported codecs. // is_decoder_factory is needed to keep track of the implict assumption that any // H264 decoder also supports constrained base line profile. @@ -199,47 +208,70 @@ std::vector GetDefaultSupportedFormats( // This function will assign dynamic payload types (in the range [96, 127] // and then [35, 63]) to the input codecs, and also add ULPFEC, RED, FlexFEC, +// and associated RTX codecs for recognized codecs (VP8, VP9, H264, and RED). // It will also add default feedback params to the codecs. -webrtc::RTCErrorOr> AssignPayloadTypes( +std::vector AssignPayloadTypesAndAddRtx( const std::vector& supported_formats, - webrtc::PayloadTypePicker& pt_mapper, + bool include_rtx, const webrtc::FieldTrialsView& trials) { + // Due to interoperability issues with old Chrome/WebRTC versions that + // ignore the [35, 63] range prefer the lower range for new codecs. + static const int kFirstDynamicPayloadTypeLowerRange = 35; + static const int kLastDynamicPayloadTypeLowerRange = 63; + + static const int kFirstDynamicPayloadTypeUpperRange = 96; + static const int kLastDynamicPayloadTypeUpperRange = 127; + int payload_type_upper = kFirstDynamicPayloadTypeUpperRange; + int payload_type_lower = kFirstDynamicPayloadTypeLowerRange; + std::vector output_codecs; for (const webrtc::SdpVideoFormat& format : supported_formats) { Codec codec = cricket::CreateVideoCodec(format); - codec.id = pt_mapper.SuggestMapping(codec, nullptr).value(); - // TODO: https://issues.webrtc.org/360058654 - Handle running out of IDs. - AddDefaultFeedbackParams(&codec, trials); - output_codecs.push_back(codec); - } - return output_codecs; -} - -// This function will add associated RTX codecs for recognized codecs (VP8, VP9, -// H264, and RED). -webrtc::RTCErrorOr> AddRtx( - const std::vector& input_codecs, - webrtc::PayloadTypePicker& pt_mapper) { - std::vector output_codecs; - for (const Codec& codec : input_codecs) { - // We want to interleave the input codecs with the output codecs. - output_codecs.push_back(codec); bool isFecCodec = absl::EqualsIgnoreCase(codec.name, kUlpfecCodecName) || absl::EqualsIgnoreCase(codec.name, kFlexfecCodecName); + + // Check if we ran out of payload types. + if (payload_type_lower > kLastDynamicPayloadTypeLowerRange) { + // TODO(https://bugs.chromium.org/p/webrtc/issues/detail?id=12248): + // return an error. + RTC_LOG(LS_ERROR) << "Out of dynamic payload types [35,63] after " + "fallback from [96, 127], skipping the rest."; + RTC_DCHECK_EQ(payload_type_upper, kLastDynamicPayloadTypeUpperRange); + break; + } + + // Lower range gets used for "new" codecs or when running out of payload + // types in the upper range. + if (IsCodecValidForLowerRange(codec) || + payload_type_upper >= kLastDynamicPayloadTypeUpperRange) { + codec.id = payload_type_lower++; + } else { + codec.id = payload_type_upper++; + } + AddDefaultFeedbackParams(&codec, trials); + output_codecs.push_back(codec); + // Add associated RTX codec for non-FEC codecs. - if (!isFecCodec) { - Codec rtx_codec = - cricket::CreateVideoRtxCodec(Codec::kIdNotSet, codec.id); - auto result = pt_mapper.SuggestMapping(rtx_codec, nullptr); - if (!result.ok()) { - if (result.error().type() == webrtc::RTCErrorType::RESOURCE_EXHAUSTED) { - // Out of payload types. Stop adding RTX codecs. + if (include_rtx) { + if (!isFecCodec) { + // Check if we ran out of payload types. + if (payload_type_lower > kLastDynamicPayloadTypeLowerRange) { + // TODO(https://bugs.chromium.org/p/webrtc/issues/detail?id=12248): + // return an error. + RTC_LOG(LS_ERROR) << "Out of dynamic payload types [35,63] after " + "fallback from [96, 127], skipping the rest."; + RTC_DCHECK_EQ(payload_type_upper, kLastDynamicPayloadTypeUpperRange); break; } - return result.MoveError(); + if (IsCodecValidForLowerRange(codec) || + payload_type_upper >= kLastDynamicPayloadTypeUpperRange) { + output_codecs.push_back( + cricket::CreateVideoRtxCodec(payload_type_lower++, codec.id)); + } else { + output_codecs.push_back( + cricket::CreateVideoRtxCodec(payload_type_upper++, codec.id)); + } } - rtx_codec.id = result.value(); - output_codecs.push_back(rtx_codec); } } return output_codecs; @@ -256,19 +288,7 @@ std::vector GetPayloadTypesAndDefaultCodecs( auto supported_formats = GetDefaultSupportedFormats(factory, is_decoder_factory, trials); - // Temporary: Use PayloadTypePicker for assignments. - // TODO: https://issues.webrtc.org/360058654 - stop assigning PTs here. - webrtc::PayloadTypePicker pt_mapper; - std::vector output_codecs; - auto result = AssignPayloadTypes(supported_formats, pt_mapper, trials); - RTC_DCHECK(result.ok()); - output_codecs = result.MoveValue(); - if (include_rtx) { - auto result = AddRtx(output_codecs, pt_mapper); - RTC_DCHECK(result.ok()); - return result.MoveValue(); - } - return output_codecs; + return AssignPayloadTypesAndAddRtx(supported_formats, include_rtx, trials); } static std::string CodecVectorToString(const std::vector& codecs) { @@ -568,11 +588,11 @@ std::vector MapCodecs(const std::vector& codecs) { std::optional flexfec_payload_type; for (const Codec& in_codec : codecs) { - RTC_LOG(LS_ERROR) << "MapCodecs registering " << in_codec; 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 ecde41a547..3dbc6b6583 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -2029,7 +2029,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", diff --git a/pc/media_session.cc b/pc/media_session.cc index a9beadb25c..9cb82347b5 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -499,6 +499,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()); } } @@ -644,10 +645,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. @@ -657,11 +657,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); } } @@ -689,35 +685,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); @@ -731,15 +707,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 @@ -825,28 +796,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; } @@ -1241,8 +1204,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()) { @@ -1282,19 +1244,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 && @@ -1371,7 +1321,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(); @@ -1858,29 +1807,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: @@ -1896,15 +1836,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: @@ -1922,9 +1860,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; @@ -1957,12 +1895,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 @@ -2273,8 +2207,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(); } @@ -2505,10 +2438,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 3a157520a3..fa71feec0c 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" @@ -166,25 +164,6 @@ const Codec kVideoCodecsH265Level52[] = { CreateVideoCodec(96, kH265MainProfileLevel52Sdp)}; const Codec kVideoCodecsH265Level6[] = { CreateVideoCodec(96, kH265MainProfileLevel6Sdp)}; -// 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), @@ -507,8 +486,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), @@ -757,8 +736,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_; }; @@ -902,6 +879,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateOfferWithCustomCodecs) { // Fields in codec are set during the gen process, so simple compare // does not work. EXPECT_EQ(acd->codecs()[0].name, custom_audio_codec.name); + RTC_LOG(LS_ERROR) << "DEBUG: audio PT assigned is " << acd->codecs()[0].id; EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type()); ASSERT_EQ(vcd->codecs().size(), 1U); @@ -3017,11 +2995,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 @@ -3049,13 +3027,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 @@ -3082,7 +3054,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 @@ -3180,7 +3152,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 @@ -3195,7 +3167,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: @@ -3350,7 +3322,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. @@ -3459,7 +3431,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 @@ -3482,7 +3454,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); @@ -3494,7 +3466,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 @@ -4583,8 +4555,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), @@ -4603,8 +4575,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; @@ -4649,9 +4619,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); @@ -4722,9 +4691,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); @@ -4765,15 +4733,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 @@ -4829,12 +4795,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), @@ -4917,7 +4881,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 << " }"; @@ -4982,12 +4946,12 @@ class VideoCodecsOfferH265LevelIdTest : public testing::Test { false, &ssrc_generator_offerer_, &tdf_offerer_, - &pt_suggester_offerer_), + nullptr), sf_answerer_(nullptr, false, &ssrc_generator_answerer_, &tdf_answerer_, - &pt_suggester_answerer_) { + nullptr) { tdf_offerer_.set_certificate( rtc::RTCCertificate::Create(std::unique_ptr( new rtc::FakeSSLIdentity("offer_id")))); @@ -5015,8 +4979,6 @@ class VideoCodecsOfferH265LevelIdTest : public testing::Test { UniqueRandomIdGenerator ssrc_generator_answerer_; MediaSessionDescriptionFactory sf_offerer_; MediaSessionDescriptionFactory sf_answerer_; - webrtc::FakePayloadTypeSuggester pt_suggester_offerer_; - webrtc::FakePayloadTypeSuggester pt_suggester_answerer_; }; // Both sides support H.265 level 5.2 for encoding and decoding. diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index c39b14e629..3f7e74bb1e 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/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc index 5e8ae14816..029878ff77 100644 --- a/pc/sdp_offer_answer_unittest.cc +++ b/pc/sdp_offer_answer_unittest.cc @@ -8,25 +8,19 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include -#include #include -#include -#include #include #include -#include "absl/strings/match.h" #include "absl/strings/str_replace.h" +#include "api/audio/audio_device.h" +#include "api/audio/audio_mixer.h" +#include "api/audio/audio_processing.h" #include "api/audio_codecs/builtin_audio_decoder_factory.h" #include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "api/create_peerconnection_factory.h" -#include "api/jsep.h" #include "api/media_types.h" #include "api/peer_connection_interface.h" -#include "api/rtc_error.h" -#include "api/rtp_parameters.h" -#include "api/rtp_transceiver_direction.h" #include "api/rtp_transceiver_interface.h" #include "api/scoped_refptr.h" #include "api/video_codecs/video_decoder_factory_template.h" @@ -39,14 +33,12 @@ #include "api/video_codecs/video_encoder_factory_template_libvpx_vp8_adapter.h" #include "api/video_codecs/video_encoder_factory_template_libvpx_vp9_adapter.h" #include "api/video_codecs/video_encoder_factory_template_open_h264_adapter.h" -#include "media/base/codec.h" -#include "media/base/media_constants.h" -#include "media/base/stream_params.h" +#include "p2p/base/port_allocator.h" #include "pc/peer_connection_wrapper.h" #include "pc/session_description.h" #include "pc/test/fake_audio_capture_module.h" #include "pc/test/mock_peer_connection_observers.h" -#include "rtc_base/string_encode.h" +#include "rtc_base/rtc_certificate_generator.h" #include "rtc_base/thread.h" #include "system_wrappers/include/metrics.h" #include "test/gmock.h" @@ -666,28 +658,19 @@ TEST_F(SdpOfferAnswerTest, SimulcastOfferWithMixedCodec) { // Verify that the serialized SDP includes pt=. std::string sdp; offer->ToString(&sdp); - int vp8_pt = cricket::Codec::kIdNotSet; - int vp9_pt = cricket::Codec::kIdNotSet; - for (auto& codec : send_codecs) { - if (codec.name == vp8_codec->name && vp8_pt == cricket::Codec::kIdNotSet) { - vp8_pt = codec.id; - } - if (codec.name == vp9_codec->name && vp9_pt == cricket::Codec::kIdNotSet) { - vp9_pt = codec.id; - } - } - EXPECT_THAT(sdp, - testing::HasSubstr("a=rid:1 send pt=" + std::to_string(vp8_pt))); - EXPECT_THAT(sdp, - testing::HasSubstr("a=rid:2 send pt=" + std::to_string(vp9_pt))); + EXPECT_THAT(sdp, testing::HasSubstr("a=rid:1 send pt=" + + std::to_string(send_codecs[0].id))); + EXPECT_THAT(sdp, testing::HasSubstr("a=rid:2 send pt=" + + std::to_string(send_codecs[1].id))); // Verify that SDP containing pt= can be parsed correctly. auto offer2 = CreateSessionDescription(SdpType::kOffer, sdp); auto& offer_contents2 = offer2->description()->contents(); auto send_rids2 = offer_contents2[0].media_description()->streams()[0].rids(); + auto send_codecs2 = offer_contents2[0].media_description()->codecs(); EXPECT_EQ(send_rids2[0].payload_types.size(), 1u); - EXPECT_EQ(send_rids2[0].payload_types[0], vp8_pt); + EXPECT_EQ(send_rids2[0].payload_types[0], send_codecs2[0].id); EXPECT_EQ(send_rids2[1].payload_types.size(), 1u); - EXPECT_EQ(send_rids2[1].payload_types[0], vp9_pt); + EXPECT_EQ(send_rids2[1].payload_types[0], send_codecs2[1].id); } TEST_F(SdpOfferAnswerTest, SimulcastAnswerWithPayloadType) { diff --git a/pc/test/integration_test_helpers.h b/pc/test/integration_test_helpers.h index 28318e4bf3..7d9216e752 100644 --- a/pc/test/integration_test_helpers.h +++ b/pc/test/integration_test_helpers.h @@ -686,7 +686,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 {