diff --git a/media/base/codec_comparators.cc b/media/base/codec_comparators.cc index 45a1b9b8cf..bf26f17c63 100644 --- a/media/base/codec_comparators.cc +++ b/media/base/codec_comparators.cc @@ -9,6 +9,7 @@ */ #include "media/base/codec_comparators.h" +#include #include #include #include @@ -181,34 +182,34 @@ bool MatchesWithReferenceAttributesAndComparator( return true; } if (has_parameters_1 && has_parameters_2) { - // Different levels of redundancy between offer and answer are + // Different levels of redundancy between offer and answer are OK // since RED is considered to be declarative. std::vector redundant_payloads_1 = rtc::split(red_parameters_1->second, '/'); std::vector redundant_payloads_2 = rtc::split(red_parameters_2->second, '/'); - if (redundant_payloads_1.size() > 0 && redundant_payloads_2.size() > 0) { - // Mixed reference codecs (i.e. 111/112) are not supported. - for (size_t i = 1; i < redundant_payloads_1.size(); i++) { - if (redundant_payloads_1[i] != redundant_payloads_1[0]) { - return false; - } - } - for (size_t i = 1; i < redundant_payloads_2.size(); i++) { - if (redundant_payloads_2[i] != redundant_payloads_2[0]) { - return false; - } - } + // note: rtc::split returns at least 1 string even on empty strings. + size_t smallest_size = + std::min(redundant_payloads_1.size(), redundant_payloads_2.size()); + // If the smaller list is equivalent to the longer list, we consider them + // equivalent even if size differs. + for (size_t i = 0; i < smallest_size; i++) { int red_value_1; int red_value_2; - if (rtc::FromString(redundant_payloads_1[0], &red_value_1) && - rtc::FromString(redundant_payloads_2[0], &red_value_2)) { - if (reference_comparator(red_value_1, red_value_2)) { - return true; + if (rtc::FromString(redundant_payloads_1[i], &red_value_1) && + rtc::FromString(redundant_payloads_2[i], &red_value_2)) { + if (!reference_comparator(red_value_1, red_value_2)) { + return false; } + } else { + // At least one parameter was not an integer. + // This is a syntax error, but we allow it here if the whole parameter + // equals the other parameter, in order to not generate more errors + // by duplicating the bad parameter. + return red_parameters_1->second == red_parameters_2->second; } - return false; } + return true; } if (!has_parameters_1 && !has_parameters_2) { // Both parameters are missing. Happens for video RED. diff --git a/media/base/codec_comparators_unittest.cc b/media/base/codec_comparators_unittest.cc index 6f4a1990bf..f1286de977 100644 --- a/media/base/codec_comparators_unittest.cc +++ b/media/base/codec_comparators_unittest.cc @@ -80,6 +80,41 @@ TEST(CodecComparatorsTest, StaticPayloadTypesIgnoreName) { EXPECT_TRUE(MatchesWithCodecRules(codec_1, codec_2)); } +TEST(CodecComparatorsTest, MatchesWithReferenceAttributesRed) { + // Test that RED codecs' reference attributes get parsed correctly. + Codec codec_1 = + cricket::CreateAudioCodec(101, cricket::kRedCodecName, 48000, 2); + codec_1.SetParam(cricket::kCodecParamNotInNameValueFormat, "100/100"); + Codec codec_2 = + cricket::CreateAudioCodec(102, cricket::kRedCodecName, 48000, 2); + codec_2.SetParam(cricket::kCodecParamNotInNameValueFormat, "101/101"); + // Mixed codecs in RED + Codec codec_3 = + cricket::CreateAudioCodec(103, cricket::kRedCodecName, 48000, 2); + codec_3.SetParam(cricket::kCodecParamNotInNameValueFormat, "100/101"); + // Identical codecs always match. + EXPECT_TRUE(MatchesWithReferenceAttributes(codec_1, codec_1)); + EXPECT_TRUE(MatchesWithReferenceAttributes(codec_2, codec_2)); + EXPECT_TRUE(MatchesWithReferenceAttributes(codec_3, codec_3)); + // Mismatched reference codec lists. + EXPECT_FALSE(MatchesWithReferenceAttributes(codec_1, codec_2)); + EXPECT_FALSE(MatchesWithReferenceAttributes(codec_1, codec_3)); + EXPECT_FALSE(MatchesWithReferenceAttributes(codec_2, codec_3)); + // Overflow of longer lists are ignored. + // Overlong list - overflow should be ignored. + Codec codec_4 = + cricket::CreateAudioCodec(103, cricket::kRedCodecName, 48000, 2); + codec_4.SetParam(cricket::kCodecParamNotInNameValueFormat, "100/100/101/102"); + EXPECT_TRUE(MatchesWithReferenceAttributes(codec_4, codec_4)); + EXPECT_TRUE(MatchesWithReferenceAttributes(codec_1, codec_4)); + // Broken syntax will cause a non-match with anything except itself. + Codec codec_5 = + cricket::CreateAudioCodec(103, cricket::kRedCodecName, 48000, 2); + codec_5.SetParam(cricket::kCodecParamNotInNameValueFormat, ""); + EXPECT_TRUE(MatchesWithReferenceAttributes(codec_5, codec_5)); + EXPECT_FALSE(MatchesWithReferenceAttributes(codec_1, codec_5)); +} + struct TestParams { std::string name; SdpVideoFormat codec1;