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

Reland "Validate frame consistency when writing DependencyDescriptor"

Browse Source 
This reverts commit 81aa059b85949001dabbedaaf99574dc6390882f.

Reason for revert: downstream tests fixed

Original change's description:
> Revert "Validate frame consistency when writing DependencyDescriptor"
>
> This reverts commit 200fd82771ae29d23b2be40194be674b3437f0ab.
>
> Reason for revert: breaks downstream
>
> Original change's description:
> > Validate frame consistency when writing DependencyDescriptor
> >
> > To write DependencyDescriptor frame properties should be consistent with
> > the FrameDependencyStructure.
> > Historically that was ensured by webrtc codec wrappers, but with with frame transform api interface there are now more ways to inject video frame for packetizing.
> > Thus DependencyDescriptorWriter should be more protective to avoid crashes.
> >
> > Bug: chromium:379282549
> > Change-Id: I98f226ff09c32154e18888c8e811e7981567ad45
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/371301
> > Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
> > Reviewed-by: Åsa Persson <asapersson@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#43551}
>
> Bug: chromium:379282549
> Change-Id: I7711756f774648cbb85c51b61424bb950c1d3775
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/371420
> Commit-Queue: Jeremy Leconte <jleconte@webrtc.org>
> Owners-Override: Jeremy Leconte <jleconte@webrtc.org>
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/main@{#43556}

Bug: chromium:379282549
Change-Id: I71ef363d710b7f28b298d11543e1c8ad6c884f15
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/371304
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Jeremy Leconte <jleconte@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43563}
This commit is contained in:
Danil Chapovalov 2024-12-13 05:07:54 -08:00 committed by WebRTC LUCI CQ
parent adacadb678
commit 3e98919a6a
5 changed files with 61 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
call/rtp_payload_params.cc
View File

@ -483,6 +483,10 @@ void RtpPayloadParams::H264ToGeneric(const CodecSpecificInfoH264& h264_info,
temporal_index, DecodeTargetIndication::kNotPresent);
std::fill(it, generic.decode_target_indications.end(),
DecodeTargetIndication::kSwitch);
generic.chain_diffs = {
(is_keyframe || last_frame_id_[0][0] < 0)
? 0
: static_cast<int>(frame_id - last_frame_id_[0][0])};
if (is_keyframe) {
RTC_DCHECK_EQ(temporal_index, 0);

1
modules/rtp_rtcp/BUILD.gn
View File

@ -136,6 +136,7 @@ rtc_library("rtp_rtcp_format") {
"../../rtc_base:event_tracer",
"../../rtc_base:logging",
"../../rtc_base:macromagic",
"../../rtc_base:safe_compare",
"../../rtc_base:safe_conversions",
"../../rtc_base:stringutils",
"../../rtc_base/network:ecn_marking",

43
modules/rtp_rtcp/source/rtp_dependency_descriptor_extension_unittest.cc
View File

@ -132,5 +132,48 @@ TEST(RtpDependencyDescriptorExtensionTest, FailsToWriteInvalidDescriptor) {
descriptor));
}
TEST(RtpDependencyDescriptorExtensionTest,
FailsToWriteWhenNumberOfChainsMismatch) {
uint8_t buffer[256];
FrameDependencyStructure structure;
structure.num_decode_targets = 2;
structure.num_chains = 2;
structure.templates = {
FrameDependencyTemplate().T(0).Dtis("SR").ChainDiffs({2, 2})};
DependencyDescriptor descriptor;
descriptor.frame_dependencies = structure.templates[0];
// Structure has 2 chains, but frame provide 1 chain diff,
descriptor.frame_dependencies.chain_diffs = {2};
EXPECT_EQ(
RtpDependencyDescriptorExtension::ValueSize(structure, 0b11, descriptor),
0u);
EXPECT_FALSE(RtpDependencyDescriptorExtension::Write(buffer, structure, 0b11,
descriptor));
}
TEST(RtpDependencyDescriptorExtensionTest,
FailsToWriteWhenNumberOfDecodeTargetsMismatch) {
uint8_t buffer[256];
FrameDependencyStructure structure;
structure.num_decode_targets = 2;
structure.num_chains = 2;
structure.templates = {
FrameDependencyTemplate().T(0).Dtis("SR").ChainDiffs({2, 2})};
DependencyDescriptor descriptor;
descriptor.frame_dependencies = structure.templates[0];
// Structure has 2 decode targets, but frame provide 1 indication,
descriptor.frame_dependencies.decode_target_indications = {
DecodeTargetIndication::kSwitch};
EXPECT_EQ(
RtpDependencyDescriptorExtension::ValueSize(structure, 0b11, descriptor),
0u);
EXPECT_FALSE(RtpDependencyDescriptorExtension::Write(buffer, structure, 0b11,
descriptor));
}
} // namespace
} // namespace webrtc

12
modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.cc
View File

@ -20,6 +20,7 @@
#include "api/transport/rtp/dependency_descriptor.h"
#include "rtc_base/bit_buffer.h"
#include "rtc_base/checks.h"
#include "rtc_base/numerics/safe_compare.h"
namespace webrtc {
namespace {
@ -62,6 +63,17 @@ RtpDependencyDescriptorWriter::RtpDependencyDescriptorWriter(
structure_(structure),
active_chains_(active_chains),
bit_writer_(data.data(), data.size()) {
if (rtc::SafeNe(descriptor.frame_dependencies.chain_diffs.size(),
structure_.num_chains)) {
build_failed_ = true;
return;
}
if (rtc::SafeNe(
descriptor.frame_dependencies.decode_target_indications.size(),
structure_.num_decode_targets)) {
build_failed_ = true;
return;
}
FindBestTemplate();
}

2
video/rtp_video_stream_receiver2_unittest.cc
View File

@ -1308,7 +1308,7 @@ TEST_F(RtpVideoStreamReceiver2DependencyDescriptorTest, UnwrapsFrameId) {
deltaframe1_descriptor.frame_number = 0xfffe;
DependencyDescriptor deltaframe2_descriptor;
deltaframe1_descriptor.frame_dependencies = stream_structure.templates[1];
deltaframe2_descriptor.frame_dependencies = stream_structure.templates[1];
deltaframe2_descriptor.frame_number = 0x0002;
// Parser should unwrap frame ids correctly even if packets were reordered by