diff --git a/call/payload_type_picker.cc b/call/payload_type_picker.cc index 1c3c586646..12854cfec3 100644 --- a/call/payload_type_picker.cc +++ b/call/payload_type_picker.cc @@ -140,10 +140,10 @@ RTCErrorOr PayloadTypePicker::SuggestMapping( // The first matching entry is returned, unless excluder // maps it to something different. for (auto entry : entries_) { - if (MatchesForSdp(entry.codec(), codec)) { + if (MatchesWithCodecRules(entry.codec(), codec)) { if (excluder) { auto result = excluder->LookupCodec(entry.payload_type()); - if (result.ok() && !MatchesForSdp(result.value(), codec)) { + if (result.ok() && !MatchesWithCodecRules(result.value(), codec)) { continue; } } @@ -164,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() && - MatchesForSdp(codec, entry.codec())) { + MatchesWithCodecRules(codec, entry.codec())) { return RTCError::OK(); } } @@ -177,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() && - !MatchesForSdp(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"; @@ -208,9 +208,11 @@ RTCErrorOr PayloadTypeRecorder::LookupPayloadType( cricket::Codec codec) const { // Note that having multiple PTs mapping to the same codec is NOT an error. // In this case, we return the first found (not deterministic). - auto result = std::find_if( - payload_type_to_codec_.begin(), payload_type_to_codec_.end(), - [codec](const auto& iter) { return MatchesForSdp(iter.second, codec); }); + auto result = + std::find_if(payload_type_to_codec_.begin(), payload_type_to_codec_.end(), + [codec](const auto& iter) { + return MatchesWithCodecRules(iter.second, codec); + }); if (result == payload_type_to_codec_.end()) { return RTCError(RTCErrorType::INVALID_PARAMETER, "No payload type found for codec"); diff --git a/call/payload_type_picker_unittest.cc b/call/payload_type_picker_unittest.cc index e1704cf72a..0ecb80e24a 100644 --- a/call/payload_type_picker_unittest.cc +++ b/call/payload_type_picker_unittest.cc @@ -55,8 +55,10 @@ TEST(PayloadTypePicker, ModifyingPtIsIgnored) { PayloadTypePicker picker; PayloadTypeRecorder recorder(picker); const PayloadType a_payload_type(123); - cricket::Codec a_codec = cricket::CreateVideoCodec(0, "vp8"); - cricket::Codec b_codec = cricket::CreateVideoCodec(0, "vp9"); + cricket::Codec a_codec = + cricket::CreateVideoCodec(cricket::Codec::kIdNotSet, "vp8"); + cricket::Codec b_codec = + cricket::CreateVideoCodec(cricket::Codec::kIdNotSet, "vp9"); recorder.AddMapping(a_payload_type, a_codec); auto error = recorder.AddMapping(a_payload_type, b_codec); EXPECT_TRUE(error.ok()); diff --git a/media/BUILD.gn b/media/BUILD.gn index 02cf50374a..223a7de4e7 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -1034,6 +1034,7 @@ if (rtc_include_tests) { } sources = [ + "base/codec_comparators_unittest.cc", "base/codec_unittest.cc", "base/media_engine_unittest.cc", "base/rtp_utils_unittest.cc", diff --git a/media/base/codec_comparators.cc b/media/base/codec_comparators.cc index 30b1f2c480..a6be486c2a 100644 --- a/media/base/codec_comparators.cc +++ b/media/base/codec_comparators.cc @@ -138,13 +138,6 @@ bool ReferencedCodecsMatch(const std::vector& codecs1, } // namespace -bool MatchesForSdp(const Codec& codec_1, const Codec& codec_2) { - return absl::EqualsIgnoreCase(codec_1.name, codec_2.name) && - codec_1.type == codec_2.type && codec_1.channels == codec_2.channels && - codec_1.clockrate == codec_2.clockrate && - codec_1.params == codec_2.params; -} - bool MatchesWithCodecRules(const Codec& left_codec, const Codec& right_codec) { // Match the codec id/name based on the typical static/dynamic name rules. // Matching is case-insensitive. diff --git a/media/base/codec_comparators_unittest.cc b/media/base/codec_comparators_unittest.cc new file mode 100644 index 0000000000..dcbe08a318 --- /dev/null +++ b/media/base/codec_comparators_unittest.cc @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2024 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. + */ +#include "media/base/codec_comparators.h" + +#include "api/audio_codecs/audio_format.h" +#include "media/base/codec.h" +#include "media/base/media_constants.h" +#include "test/gtest.h" + +namespace webrtc { + +using cricket::Codec; +using cricket::CreateAudioCodec; +using cricket::CreateVideoCodec; +using cricket::kH264CodecName; +using cricket::kH264FmtpPacketizationMode; + +TEST(CodecComparatorsTest, CodecMatchesItself) { + Codec codec = cricket::CreateVideoCodec("custom"); + EXPECT_TRUE(MatchesWithCodecRules(codec, codec)); +} + +TEST(CodecComparatorsTest, MismatchedBasicParameters) { + Codec codec = CreateAudioCodec(SdpAudioFormat("opus", 48000, 2)); + Codec nonmatch_codec = codec; + nonmatch_codec.name = "g711"; + EXPECT_FALSE(MatchesWithCodecRules(nonmatch_codec, codec)); + nonmatch_codec = codec; + nonmatch_codec.clockrate = 8000; + EXPECT_FALSE(MatchesWithCodecRules(nonmatch_codec, codec)); + nonmatch_codec = codec; + nonmatch_codec.channels = 1; + EXPECT_FALSE(MatchesWithCodecRules(nonmatch_codec, codec)); +} + +TEST(CodecComparatorsTest, H264PacketizationModeMismatch) { + Codec pt_mode_1 = CreateVideoCodec(kH264CodecName); + Codec pt_mode_0 = pt_mode_1; + pt_mode_0.SetParam(kH264FmtpPacketizationMode, "0"); + EXPECT_FALSE(MatchesWithCodecRules(pt_mode_1, pt_mode_0)); + EXPECT_FALSE(MatchesWithCodecRules(pt_mode_0, pt_mode_1)); + Codec no_pt_mode = pt_mode_1; + no_pt_mode.RemoveParam(kH264FmtpPacketizationMode); + EXPECT_TRUE(MatchesWithCodecRules(pt_mode_0, no_pt_mode)); + EXPECT_TRUE(MatchesWithCodecRules(no_pt_mode, pt_mode_0)); + EXPECT_FALSE(MatchesWithCodecRules(no_pt_mode, pt_mode_1)); +} + +TEST(CodecComparatorsTest, AudioParametersIgnored) { + // Currently, all parameters on audio codecs are ignored for matching. + Codec basic_opus = CreateAudioCodec(SdpAudioFormat("opus", 48000, 2)); + Codec opus_with_parameters = basic_opus; + opus_with_parameters.SetParam("stereo", "0"); + EXPECT_TRUE(MatchesWithCodecRules(basic_opus, opus_with_parameters)); + EXPECT_TRUE(MatchesWithCodecRules(opus_with_parameters, basic_opus)); + opus_with_parameters.SetParam("nonsense", "stuff"); + EXPECT_TRUE(MatchesWithCodecRules(basic_opus, opus_with_parameters)); + EXPECT_TRUE(MatchesWithCodecRules(opus_with_parameters, basic_opus)); +} + +TEST(CodecComparatorsTest, StaticPayloadTypesIgnoreName) { + // This is the IANA registered format for PT 8 + Codec codec_1 = CreateAudioCodec(8, "pcma", 8000, 1); + Codec codec_2 = CreateAudioCodec(8, "nonsense", 8000, 1); + EXPECT_TRUE(MatchesWithCodecRules(codec_1, codec_2)); +} + +} // namespace webrtc