Add comparators unittest, and abandon MatchesForSdp

Use the same code in PayloadTypePicker as in Codec.Matches()

Bug: webrtc:360058654
Change-Id: I549ed24860648cfdb6a173a19773daf01db827b0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365102
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43217}

call/payload_type_picker.cc

@@ -140,10 +140,10 @@ RTCErrorOr<PayloadType> 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<PayloadType> 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(
+  auto result =
-      payload_type_to_codec_.begin(), payload_type_to_codec_.end(),
+      std::find_if(payload_type_to_codec_.begin(), payload_type_to_codec_.end(),
-      [codec](const auto& iter) { return MatchesForSdp(iter.second, codec); });
+                   [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");

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 a_codec =
-  cricket::Codec b_codec = cricket::CreateVideoCodec(0, "vp9");
+      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());

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",

media/base/codec_comparators.cc

@@ -138,13 +138,6 @@ bool ReferencedCodecsMatch(const std::vector<Codec>& 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.

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