admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Extend FindMatchingCodec to support multi-codec RED

Browse Source 
This was exercised by a test, but multi-codec RED is not currently
generated by WebRTC.
RED spec allows it, so failing in comparator seems wrong.
This was one of the cases where the referenced bug was triggered,
but not the only one.

Bug: webrtc:384756621
Change-Id: I28c101aa34a62083b72b5f7fc12d25fc637db209
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/372060
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43607}
This commit is contained in:
Harald Alvestrand 2024-12-19 06:43:33 +00:00 committed by WebRTC LUCI CQ
parent f0ca2dc934
commit 9f827f51f8
2 changed files with 54 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
media/base/codec_comparators.cc
View File

@ -9,6 +9,7 @@
*/
#include "media/base/codec_comparators.h"
#include <algorithm>
#include <cstddef>
#include <optional>
#include <string>
@ -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<absl::string_view> redundant_payloads_1 =
rtc::split(red_parameters_1->second, '/');
std::vector<absl::string_view> 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.

35
media/base/codec_comparators_unittest.cc
View File

@ -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;