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

Add some tests for CodecList consistency

Browse Source 
Bug: webrtc:360058654
Change-Id: Ida26eca237c4f882cf03204a3d87780c25c1890c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/371640
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43604}
This commit is contained in:
Harald Alvestrand 2024-12-18 23:06:33 +00:00 committed by WebRTC LUCI CQ
parent 84b33a4439
commit 33f38f2f38
3 changed files with 73 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
media/BUILD.gn
View File

@ -388,7 +388,10 @@ rtc_library("codec_list") {
]
deps = [
":codec",
":media_constants",
"../rtc_base:checks",
"../rtc_base:logging",
"../rtc_base:stringutils",
]
}

71
media/base/codec_list.cc
View File

@ -10,15 +10,82 @@
#include "media/base/codec_list.h"
#include <cstddef>
#include <map>
#include "media/base/codec.h"
#include "media/base/media_constants.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/string_encode.h"
namespace cricket {
void CodecList::CheckConsistency() {
#if RTC_DCHECK_IS_ON
for (Codec codec : codecs_) {
// Do some checking
std::map<int, int> pt_to_index;
// Create a map of payload type to index, and ensure
// that there are no duplicates.
for (size_t i = 0; i < codecs_.size(); i++) {
const Codec& codec = codecs_[i];
if (codec.id != Codec::kIdNotSet) {
// Not true - the test PeerConnectionMediaTest.RedFmtpPayloadMixed
// fails this check. In that case, the duplicates are identical.
// TODO: https://issues.webrtc.org/384756621 - fix test and enable check.
// RTC_DCHECK(pt_to_index.count(codec.id) == 0);
if (pt_to_index.count(codec.id) != 0) {
RTC_LOG(LS_WARNING) << "Surprising condition: Two codecs on same PT. "
<< "First: " << codecs_[pt_to_index[codec.id]]
<< " Second: " << codec;
// Skip this codec in the map, and go on.
continue;
}
pt_to_index.insert({codec.id, i});
}
}
for (Codec& codec : codecs_) {
switch (codec.GetResiliencyType()) {
case Codec::ResiliencyType::kRed:
// Check that the target codec exists
break;
case Codec::ResiliencyType::kRtx: {
// Check that the target codec exists
const auto apt_it = codec.params.find(kCodecParamAssociatedPayloadType);
// Not true - there's a test that deliberately injects a wrong
// RTX codec (MediaSessionDescriptionFactoryTest.RtxWithoutApt)
// TODO: https://issues.webrtc.org/384756622 - reject codec earlier and
// enable check. RTC_DCHECK(apt_it != codec.params.end()); Until that is
// fixed:
if (apt_it == codec.params.end()) {
RTC_LOG(LS_WARNING) << "Surprising condition: RTX codec without"
<< " apt parameter: " << codec;
break;
}
int associated_pt;
RTC_DCHECK(rtc::FromString(apt_it->second, &associated_pt));
// Not true:
// RTC_DCHECK(pt_to_index.count(associated_pt) == 1);
// TODO: https://issues.webrtc.org/384954756 - drop codecs before we get
// here
if (pt_to_index.count(associated_pt) != 1) {
RTC_LOG(LS_WARNING)
<< "Surprising condition: RTX codec APT not found: " << codec;
}
// const Codec& referred_codec = codecs_[pt_to_index[associated_pt]];
// Not true:
// RTC_DCHECK(referred_codec.type == Codec::Type::kVideo);
// Not true:
// RTC_DCHECK(referred_codec.GetResiliencyType() ==
// Codec::ResiliencyType::kNone);
// TODO: https://issues.webrtc.org/384756623 - figure out if this is
// expected or not.
break;
}
case Codec::ResiliencyType::kNone:
break; // nothing to see here
default:
break; // don't know what to check yet
}
}
#endif
}

1
pc/media_session.cc
View File

@ -720,6 +720,7 @@ void MergeCodecs(const CodecList& reference_codecs,
offered_codecs.push_back(red_codec);
}
}
offered_codecs.CheckConsistency();
}
// `codecs` is a full list of codecs with correct payload type mappings, which