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}
diff --git a/call/rtp_payload_params.cc b/call/rtp_payload_params.cc
index acc9e3c..ffe62eb 100644
--- a/call/rtp_payload_params.cc
+++ b/call/rtp_payload_params.cc
@@ -483,6 +483,10 @@
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);
diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn
index 3bbddc7..1f4a0ca 100644
--- a/modules/rtp_rtcp/BUILD.gn
+++ b/modules/rtp_rtcp/BUILD.gn
@@ -136,6 +136,7 @@
"../../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",
diff --git a/modules/rtp_rtcp/source/rtp_dependency_descriptor_extension_unittest.cc b/modules/rtp_rtcp/source/rtp_dependency_descriptor_extension_unittest.cc
index 148e4f9..e666d4a 100644
--- a/modules/rtp_rtcp/source/rtp_dependency_descriptor_extension_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_dependency_descriptor_extension_unittest.cc
@@ -132,5 +132,48 @@
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
diff --git a/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.cc b/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.cc
index 31df783..59a327a 100644
--- a/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.cc
+++ b/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.cc
@@ -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 @@
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();
}
diff --git a/video/rtp_video_stream_receiver2_unittest.cc b/video/rtp_video_stream_receiver2_unittest.cc
index b2f407e..fed4adb 100644
--- a/video/rtp_video_stream_receiver2_unittest.cc
+++ b/video/rtp_video_stream_receiver2_unittest.cc
@@ -1308,7 +1308,7 @@
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
