Fix out-of-range dependency descriptor frame diffs The dependency descriptor can't represent frame diffs over 4096 or chain diffs over 255. RtpDependencyDescriptorWriter only DCHECKed them, and release builds wrote them truncated, so the descriptor referenced the wrong frame. - Reject unrepresentable diffs in RtpDependencyDescriptorWriter. - In RTPSenderVideo, send chain diffs over 255 as 0, as RtpPayloadParams does for VP9, and drop delta frames that the dependency descriptor can't describe instead of disabling the descriptor. Fixed: webrtc:566342228 Change-Id: Icdb1c36d3e6684bab7d1253e5c06265274ce670c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/505920 Reviewed-by: Danil Chapovalov <danilchap@webrtc.org> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Cr-Commit-Position: refs/heads/main@{#48753}
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 9b054e7..7f1dff0 100644 --- a/modules/rtp_rtcp/source/rtp_dependency_descriptor_extension_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_dependency_descriptor_extension_unittest.cc
@@ -24,6 +24,7 @@ namespace { using ::testing::Each; +using ::testing::ElementsAre; TEST(RtpDependencyDescriptorExtensionTest, Writer3BytesForPerfectTemplate) { uint8_t buffer[3]; @@ -180,5 +181,111 @@ descriptor)); } +// The largest frame diff the dependency descriptor can represent is 4096. +TEST(RtpDependencyDescriptorExtensionTest, FailsToWriteTooLargeFrameDiff) { + uint8_t buffer[256]; + FrameDependencyStructure structure; + structure.num_decode_targets = 1; + structure.templates = {FrameDependencyTemplate().Dtis("S")}; + DependencyDescriptor descriptor; + descriptor.frame_dependencies = structure.templates[0]; + descriptor.frame_dependencies.frame_diffs = {5275}; + + EXPECT_EQ(RtpDependencyDescriptorExtension::ValueSize(structure, descriptor), + 0u); + EXPECT_FALSE( + RtpDependencyDescriptorExtension::Write(buffer, structure, descriptor)); +} + +TEST(RtpDependencyDescriptorExtensionTest, FailsToWriteNonPositiveFrameDiff) { + uint8_t buffer[256]; + FrameDependencyStructure structure; + structure.num_decode_targets = 1; + structure.templates = {FrameDependencyTemplate().Dtis("S")}; + for (int fdiff : {0, -1}) { + SCOPED_TRACE(fdiff); + DependencyDescriptor descriptor; + descriptor.frame_dependencies = structure.templates[0]; + descriptor.frame_dependencies.frame_diffs = {fdiff}; + + EXPECT_EQ( + RtpDependencyDescriptorExtension::ValueSize(structure, descriptor), 0u); + EXPECT_FALSE( + RtpDependencyDescriptorExtension::Write(buffer, structure, descriptor)); + } +} + +// Chain diffs of active chains must be in the range [0, 255]. +TEST(RtpDependencyDescriptorExtensionTest, + FailsToWriteInvalidChainDiffForActiveChain) { + uint8_t buffer[256]; + FrameDependencyStructure structure; + structure.num_decode_targets = 1; + structure.num_chains = 1; + structure.decode_target_protected_by_chain = {0}; + structure.templates = {FrameDependencyTemplate().Dtis("S").ChainDiffs({1})}; + for (int chain_diff : {-1, 256}) { + SCOPED_TRACE(chain_diff); + DependencyDescriptor descriptor; + descriptor.frame_dependencies = structure.templates[0]; + descriptor.frame_dependencies.chain_diffs = {chain_diff}; + + EXPECT_EQ( + RtpDependencyDescriptorExtension::ValueSize(structure, descriptor), 0u); + EXPECT_FALSE( + RtpDependencyDescriptorExtension::Write(buffer, structure, descriptor)); + } +} + +TEST(RtpDependencyDescriptorExtensionTest, RoundTripsFrameDiffsUpToLimit) { + FrameDependencyStructure structure; + structure.num_decode_targets = 1; + structure.templates = {FrameDependencyTemplate().Dtis("S")}; + // Boundaries of the 4, 8 and 12 bit representations of a frame diff. + for (int fdiff : {1, 16, 17, 256, 257, 4096}) { + SCOPED_TRACE(fdiff); + DependencyDescriptor descriptor; + descriptor.frame_dependencies = structure.templates[0]; + descriptor.frame_dependencies.frame_diffs = {fdiff}; + uint8_t buffer[16]; + size_t value_size = + RtpDependencyDescriptorExtension::ValueSize(structure, descriptor); + ASSERT_GT(value_size, 0u); + ASSERT_LE(value_size, sizeof(buffer)); + std::span<uint8_t> data = std::span(buffer).first(value_size); + + ASSERT_TRUE( + RtpDependencyDescriptorExtension::Write(data, structure, descriptor)); + DependencyDescriptor parsed; + ASSERT_TRUE( + RtpDependencyDescriptorExtension::Parse(data, &structure, &parsed)); + EXPECT_THAT(parsed.frame_dependencies.frame_diffs, ElementsAre(fdiff)); + } +} + +TEST(RtpDependencyDescriptorExtensionTest, RoundTripsChainDiffUpToLimit) { + FrameDependencyStructure structure; + structure.num_decode_targets = 1; + structure.num_chains = 1; + structure.decode_target_protected_by_chain = {0}; + structure.templates = {FrameDependencyTemplate().Dtis("S").ChainDiffs({1})}; + DependencyDescriptor descriptor; + descriptor.frame_dependencies = structure.templates[0]; + descriptor.frame_dependencies.chain_diffs = {255}; + uint8_t buffer[16]; + size_t value_size = + RtpDependencyDescriptorExtension::ValueSize(structure, descriptor); + ASSERT_GT(value_size, 0u); + ASSERT_LE(value_size, sizeof(buffer)); + std::span<uint8_t> data = std::span(buffer).first(value_size); + + ASSERT_TRUE( + RtpDependencyDescriptorExtension::Write(data, structure, descriptor)); + DependencyDescriptor parsed; + ASSERT_TRUE( + RtpDependencyDescriptorExtension::Parse(data, &structure, &parsed)); + EXPECT_THAT(parsed.frame_dependencies.chain_diffs, ElementsAre(255)); +} + } // 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 3d12605..0095f83 100644 --- a/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.cc +++ b/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.cc
@@ -27,6 +27,21 @@ namespace webrtc { namespace { +// Largest diffs that can be written for a frame. They are limited by the +// per-frame fields of the dependency descriptor, see frame_fdiffs() and +// frame_chains() in https://aomediacodec.github.io/av1-rtp-spec/#a82-syntax. +// Frame dependency templates use narrower 4 bit fields, and frames whose diffs +// differ from their template are written using the per-frame fields, so these +// limits apply regardless of which template is used for the frame. + +// A frame diff is written as `fdiff_minus_one`, using 4 * `next_fdiff_size` +// bits where `next_fdiff_size` is 1, 2 or 3. The widest field is 12 bits, so +// the largest diff is ((1 << 12) - 1) + 1. A diff of 0 can't be represented. +constexpr int kMaxFrameDiff = 1 << 12; +// A chain diff is written as-is in the 8 bit `frame_chain_fdiff` field, where +// 0 means that the chain has no previous frame. +constexpr int kMaxChainDiff = (1 << 8) - 1; + enum class NextLayerIdc : uint64_t { kSameLayer = 0, kNextTemporal = 1, @@ -75,6 +90,10 @@ build_failed_ = true; return; } + if (!HasSerializableDiffs()) { + build_failed_ = true; + return; + } FindBestTemplate(); } @@ -175,6 +194,23 @@ return result; } +bool RtpDependencyDescriptorWriter::HasSerializableDiffs() const { + for (int fdiff : descriptor_.frame_dependencies.frame_diffs) { + if (fdiff <= 0 || fdiff > kMaxFrameDiff) { + return false; + } + } + for (int i = 0; i < structure_.num_chains; ++i) { + // Chain diffs of inactive chains are not serialized, see + // WriteFrameChains(). + int chain_diff = descriptor_.frame_dependencies.chain_diffs[i]; + if (active_chains_[i] && (chain_diff < 0 || chain_diff > kMaxChainDiff)) { + return false; + } + } + return true; +} + void RtpDependencyDescriptorWriter::FindBestTemplate() { const std::vector<FrameDependencyTemplate>& templates = structure_.templates; // Find range of templates with matching spatial/temporal id. @@ -376,7 +412,7 @@ void RtpDependencyDescriptorWriter::WriteFrameFdiffs() { for (int fdiff : descriptor_.frame_dependencies.frame_diffs) { RTC_DCHECK_GT(fdiff, 0); - RTC_DCHECK_LE(fdiff, 1 << 12); + RTC_DCHECK_LE(fdiff, kMaxFrameDiff); if (fdiff <= (1 << 4)) WriteBits((1u << 4) | (fdiff - 1), 2 + 4); else if (fdiff <= (1 << 8)) @@ -395,7 +431,7 @@ int chain_diff = active_chains_[i] ? descriptor_.frame_dependencies.chain_diffs[i] : 0; RTC_DCHECK_GE(chain_diff, 0); - RTC_DCHECK_LT(chain_diff, 1 << 8); + RTC_DCHECK_LE(chain_diff, kMaxChainDiff); WriteBits(chain_diff, 8); } }
diff --git a/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.h b/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.h index 1feb21e..3e9fbbc 100644 --- a/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.h +++ b/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.h
@@ -22,15 +22,18 @@ namespace webrtc { class RtpDependencyDescriptorWriter { public: - // Assumes `structure` and `descriptor` are valid and - // `descriptor` matches the `structure`. + // Assumes `structure` is valid and `descriptor` matches the `structure`. + // Rejects `descriptor` if its frame diffs or chain diffs are outside of the + // range that the dependency descriptor can represent, in which case + // `ValueSizeBits()` returns 0 and `Write()` returns false. RtpDependencyDescriptorWriter(std::span<uint8_t> data, const FrameDependencyStructure& structure, std::bitset<32> active_chains, const DependencyDescriptor& descriptor); // Serializes DependencyDescriptor rtp header extension. - // Returns false if `data` is too small to serialize the `descriptor`. + // Returns false if `data` is too small to serialize the `descriptor` or if + // the `descriptor` can't be serialized. bool Write(); // Returns minimum number of bits needed to serialize descriptor with respect @@ -51,6 +54,9 @@ }; int StructureSizeBits() const; TemplateMatch CalculateMatch(TemplateIterator frame_template) const; + // Returns false if any frame diff or chain diff of an active chain in + // `descriptor_` is outside of the range that can be serialized. + bool HasSerializableDiffs() const; void FindBestTemplate(); bool ShouldWriteActiveDecodeTargetsBitmask() const; bool HasExtendedFields() const;
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 73a3697..d703aa5 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -10,6 +10,8 @@ #include "modules/rtp_rtcp/source/rtp_sender_video.h" +#include <bitset> +#include <cstddef> #include <cstdint> #include <cstdlib> #include <cstring> @@ -21,6 +23,7 @@ #include <vector> #include "absl/algorithm/container.h" +#include "absl/container/inlined_vector.h" #include "absl/memory/memory.h" #include "api/crypto/frame_encryptor_interface.h" #include "api/field_trials_view.h" @@ -75,6 +78,11 @@ constexpr size_t kRedForFecHeaderLength = 1; constexpr TimeDelta kMaxUnretransmittableFrameInterval = TimeDelta::Millis(33 * 4); +// Minimum interval between repeated warnings. +constexpr TimeDelta kWarningLogInterval = TimeDelta::Seconds(10); +// Largest chain diff that the dependency descriptor can represent, see +// `frame_chain_fdiff` in the dependency descriptor specification. +constexpr int kMaxChainDiff = (1 << 8) - 1; void BuildRedPayload(const RtpPacketToSend& media_packet, RtpPacketToSend* red_packet) { @@ -522,6 +530,39 @@ } } +bool RTPSenderVideo::HandleMissingDependencyDescriptor( + const RTPVideoHeader& video_header, + const RtpPacketToSend& packet) { + if (video_structure_ == nullptr || + !packet.IsRegistered<RtpDependencyDescriptorExtension>()) { + // The dependency descriptor isn't used. + return true; + } + if (video_header.frame_type == VideoFrameType::kVideoFrameKey) { + // Disable attaching dependency descriptor to delta packets (including + // non-first packet of a key frame) when it wasn't attached to a key frame, + // as dependency descriptor can't be usable in such case. + // This can also happen when the descriptor is larger than 15 bytes and + // two-byte header extensions are not negotiated using extmap-allow-mixed. + RTC_LOG(LS_WARNING) << "Disable dependency descriptor because failed to " + "attach it to a key frame."; + video_structure_ = nullptr; + return true; + } + // The dependency descriptor can't describe this delta frame, e.g. because it + // references a frame too far back. Drop the frame rather than send it + // without a description of its dependencies, which receivers and middleboxes + // rely on. Unlike for key frames, keep the dependency descriptor enabled for + // the frames that follow. + Timestamp now = clock_->CurrentTime(); + if (now >= last_undescribable_frame_log_ + kWarningLogInterval) { + RTC_LOG(LS_WARNING) << "Failed to describe frame dependencies. Frame is " + "dropped."; + last_undescribable_frame_log_ = now; + } + return false; +} + bool RTPSenderVideo::SendVideo(int payload_type, VideoCodecType codec_type, uint32_t rtp_timestamp, @@ -595,6 +636,31 @@ video_header.generic->active_decode_targets, video_header.frame_type == VideoFrameType::kVideoFrameKey, video_header.generic->frame_id, video_header.generic->chain_diffs); + // Chain diffs of inactive chains aren't serialized, so they may be + // arbitrarily large, e.g. when a spatial layer isn't being produced. + const std::bitset<32> active_chains = + active_decode_targets_tracker_.ActiveChainsBitmask(); + absl::InlinedVector<int, 4>& chain_diffs = + video_header.generic->chain_diffs; + for (size_t i = 0; i < chain_diffs.size() && i < active_chains.size(); + ++i) { + int& chain_diff = chain_diffs[i]; + if (!active_chains[i] || chain_diff <= kMaxChainDiff) { + continue; + } + // A chain diff that the dependency descriptor can't represent would + // make the frame undescribable. Instead, describe the chain as having + // no previous frame, like RtpPayloadParams does for VP9. That can lead + // to video corruption if a previous frame of the chain was lost. + Timestamp now = clock_->CurrentTime(); + if (now >= last_chain_diff_log_ + kWarningLogInterval) { + RTC_LOG(LS_WARNING) << "Chain diff " << chain_diff + << " is too large for the dependency descriptor. " + "Sending 0 instead."; + last_chain_diff_log_ = now; + } + chain_diff = 0; + } } // No FEC protection for upper temporal layers, if used. @@ -647,18 +713,9 @@ AddRtpHeaderExtensions(video_header, /*first_packet=*/true, /*last_packet=*/true, single_packet.get()); - if (video_structure_ != nullptr && - single_packet->IsRegistered<RtpDependencyDescriptorExtension>() && - !single_packet->HasExtension<RtpDependencyDescriptorExtension>()) { - RTC_DCHECK_EQ(video_header.frame_type, VideoFrameType::kVideoFrameKey); - // Disable attaching dependency descriptor to delta packets (including - // non-first packet of a key frame) when it wasn't attached to a key frame, - // as dependency descriptor can't be usable in such case. - // This can also happen when the descriptor is larger than 15 bytes and - // two-byte header extensions are not negotiated using extmap-allow-mixed. - RTC_LOG(LS_WARNING) << "Disable dependency descriptor because failed to " - "attach it to a key frame."; - video_structure_ = nullptr; + if (!single_packet->HasExtension<RtpDependencyDescriptorExtension>() && + !HandleMissingDependencyDescriptor(video_header, *single_packet)) { + return false; } AddRtpHeaderExtensions(video_header, @@ -743,7 +800,7 @@ if (num_packets == 0) { Timestamp now = clock_->CurrentTime(); - if (now >= last_fail_packetize_log_ + TimeDelta::Seconds(10)) { + if (now >= last_fail_packetize_log_ + kWarningLogInterval) { RTC_LOG(LS_WARNING) << "Failed to packetize " << codec_type << " video frame of size " << payload.size() << ". Frame is dropped.";
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h index 0bba6be..577d886 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/modules/rtp_rtcp/source/rtp_sender_video.h
@@ -192,6 +192,15 @@ RtpPacketToSend* packet) const RTC_EXCLUSIVE_LOCKS_REQUIRED(send_checker_); + // Called when the dependency descriptor isn't attached to `packet`, a packet + // of the frame described by `video_header`. Disables the dependency + // descriptor if it couldn't be attached to a key frame. Returns false if the + // frame should be dropped because the dependency descriptor can't describe + // it. + bool HandleMissingDependencyDescriptor(const RTPVideoHeader& video_header, + const RtpPacketToSend& packet) + RTC_EXCLUSIVE_LOCKS_REQUIRED(send_checker_); + size_t FecPacketOverhead() const RTC_EXCLUSIVE_LOCKS_REQUIRED(send_checker_); void LogAndSendToNetwork( @@ -252,6 +261,10 @@ OneTimeEvent first_frame_sent_; Timestamp last_fail_packetize_log_ RTC_GUARDED_BY(send_checker_) = Timestamp::MinusInfinity(); + Timestamp last_undescribable_frame_log_ RTC_GUARDED_BY(send_checker_) = + Timestamp::MinusInfinity(); + Timestamp last_chain_diff_log_ RTC_GUARDED_BY(send_checker_) = + Timestamp::MinusInfinity(); // E2EE Custom Video Frame Encryptor (optional) FrameEncryptorInterface* const frame_encryptor_ = nullptr;
diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc index e5794b9..4c0ac41 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
@@ -782,6 +782,106 @@ .HasExtension<RtpDependencyDescriptorExtension>()); } +// A delta frame whose dependencies the dependency descriptor can't describe, +// e.g. because it references a frame more than 4096 frames back, is dropped. +// The dependency descriptor stays enabled for the frames that follow. +TEST_F(RtpSenderVideoTest, + DropsDeltaFrameThatDependencyDescriptorCannotDescribe) { + const int64_t kFrameId = 100000; + uint8_t kFrame[100]; + rtp_module_->RegisterRtpHeaderExtension( + RtpDependencyDescriptorExtension::Uri(), kDependencyDescriptorId); + FrameDependencyStructure video_structure; + video_structure.num_decode_targets = 1; + video_structure.templates = { + FrameDependencyTemplate().Dtis("S"), + FrameDependencyTemplate().Dtis("S").FrameDiffs({1}), + }; + rtp_sender_video_->SetVideoStructure(&video_structure); + + // Send key frame. + RTPVideoHeader hdr; + RTPVideoHeader::GenericDescriptorInfo& generic = hdr.generic.emplace(); + generic.frame_id = kFrameId; + generic.decode_target_indications = {DecodeTargetIndication::kSwitch}; + hdr.frame_type = VideoFrameType::kVideoFrameKey; + ASSERT_TRUE(rtp_sender_video_->SendVideoFrame( + kPayloadType, kType, kTimestampInfo, fake_clock_.CurrentTime(), kFrame, + sizeof(kFrame), hdr, kDefaultExpectedRetransmissionTime, {})); + ASSERT_EQ(transport_.packets_sent(), 1); + DependencyDescriptor descriptor_key; + ASSERT_TRUE(transport_.last_sent_packet() + .GetExtension<RtpDependencyDescriptorExtension>( + nullptr, &descriptor_key)); + ASSERT_TRUE(descriptor_key.attached_structure); + + // Send delta frame that references the key frame 5275 frames back. + generic.frame_id = kFrameId + 5275; + generic.dependencies = {kFrameId}; + hdr.frame_type = VideoFrameType::kVideoFrameDelta; + EXPECT_FALSE(rtp_sender_video_->SendVideoFrame( + kPayloadType, kType, kTimestampInfo, fake_clock_.CurrentTime(), kFrame, + sizeof(kFrame), hdr, kDefaultExpectedRetransmissionTime, {})); + EXPECT_EQ(transport_.packets_sent(), 1); + + // Send delta frame that references the dropped frame. + generic.frame_id = kFrameId + 5276; + generic.dependencies = {kFrameId + 5275}; + EXPECT_TRUE(rtp_sender_video_->SendVideoFrame( + kPayloadType, kType, kTimestampInfo, fake_clock_.CurrentTime(), kFrame, + sizeof(kFrame), hdr, kDefaultExpectedRetransmissionTime, {})); + ASSERT_EQ(transport_.packets_sent(), 2); + DependencyDescriptor descriptor_delta; + ASSERT_TRUE( + transport_.last_sent_packet() + .GetExtension<RtpDependencyDescriptorExtension>( + descriptor_key.attached_structure.get(), &descriptor_delta)); + EXPECT_EQ(descriptor_delta.frame_number, (kFrameId + 5276) & 0xFFFF); + EXPECT_THAT(descriptor_delta.frame_dependencies.frame_diffs, ElementsAre(1)); +} + +// The generic frame descriptor, even if negotiated, is not used instead of the +// dependency descriptor when the latter can't describe a delta frame. +TEST_F(RtpSenderVideoTest, DoesNotFallBackToGenericFrameDescriptor) { + const int64_t kFrameId = 100000; + uint8_t kFrame[100]; + rtp_module_->RegisterRtpHeaderExtension( + RtpDependencyDescriptorExtension::Uri(), kDependencyDescriptorId); + rtp_module_->RegisterRtpHeaderExtension( + RtpGenericFrameDescriptorExtension00::Uri(), kGenericDescriptorId); + FrameDependencyStructure video_structure; + video_structure.num_decode_targets = 1; + video_structure.templates = { + FrameDependencyTemplate().Dtis("S"), + FrameDependencyTemplate().Dtis("S").FrameDiffs({1}), + }; + rtp_sender_video_->SetVideoStructure(&video_structure); + + // Send key frame. + RTPVideoHeader hdr; + RTPVideoHeader::GenericDescriptorInfo& generic = hdr.generic.emplace(); + generic.frame_id = kFrameId; + generic.decode_target_indications = {DecodeTargetIndication::kSwitch}; + hdr.frame_type = VideoFrameType::kVideoFrameKey; + ASSERT_TRUE(rtp_sender_video_->SendVideoFrame( + kPayloadType, kType, kTimestampInfo, fake_clock_.CurrentTime(), kFrame, + sizeof(kFrame), hdr, kDefaultExpectedRetransmissionTime, {})); + ASSERT_EQ(transport_.packets_sent(), 1); + ASSERT_TRUE(transport_.last_sent_packet() + .HasExtension<RtpDependencyDescriptorExtension>()); + + // Send delta frame that references the key frame 5275 frames back, which is + // too far back for the dependency descriptor, but not for the generic frame + // descriptor. + generic.frame_id = kFrameId + 5275; + generic.dependencies = {kFrameId}; + hdr.frame_type = VideoFrameType::kVideoFrameDelta; + EXPECT_FALSE(rtp_sender_video_->SendVideoFrame( + kPayloadType, kType, kTimestampInfo, fake_clock_.CurrentTime(), kFrame, + sizeof(kFrame), hdr, kDefaultExpectedRetransmissionTime, {})); + EXPECT_EQ(transport_.packets_sent(), 1); +} + TEST_F(RtpSenderVideoTest, PropagatesChainDiffsIntoDependencyDescriptor) { const int64_t kFrameId = 100000; uint8_t kFrame[100]; @@ -816,6 +916,116 @@ ContainerEq(generic.chain_diffs)); } +// A chain diff that the dependency descriptor can't represent, e.g. because +// the chain's previous frame was sent before a long pause, is sent as 0, i.e. +// the chain has no previous frame, rather than making the frame undescribable. +TEST_F(RtpSenderVideoTest, ReplacesUnrepresentableChainDiffWithZero) { + const int64_t kFrameId = 100000; + uint8_t kFrame[100]; + rtp_module_->RegisterRtpHeaderExtension( + RtpDependencyDescriptorExtension::Uri(), kDependencyDescriptorId); + FrameDependencyStructure video_structure; + video_structure.num_decode_targets = 1; + video_structure.num_chains = 1; + video_structure.decode_target_protected_by_chain = {0}; + video_structure.templates = { + FrameDependencyTemplate().Dtis("S").ChainDiffs({0}), + FrameDependencyTemplate().Dtis("S").FrameDiffs({1}).ChainDiffs({1}), + }; + rtp_sender_video_->SetVideoStructure(&video_structure); + + // Send key frame. + RTPVideoHeader hdr; + RTPVideoHeader::GenericDescriptorInfo& generic = hdr.generic.emplace(); + generic.frame_id = kFrameId; + generic.decode_target_indications = {DecodeTargetIndication::kSwitch}; + generic.chain_diffs = {0}; + hdr.frame_type = VideoFrameType::kVideoFrameKey; + ASSERT_TRUE(rtp_sender_video_->SendVideoFrame( + kPayloadType, kType, kTimestampInfo, fake_clock_.CurrentTime(), kFrame, + sizeof(kFrame), hdr, kDefaultExpectedRetransmissionTime, {})); + ASSERT_EQ(transport_.packets_sent(), 1); + DependencyDescriptor descriptor_key; + ASSERT_TRUE(transport_.last_sent_packet() + .GetExtension<RtpDependencyDescriptorExtension>( + nullptr, &descriptor_key)); + ASSERT_TRUE(descriptor_key.attached_structure); + + // Send delta frame with a chain diff that doesn't fit in 8 bits. + generic.frame_id = kFrameId + 1; + generic.dependencies = {kFrameId}; + generic.chain_diffs = {300}; + hdr.frame_type = VideoFrameType::kVideoFrameDelta; + EXPECT_TRUE(rtp_sender_video_->SendVideoFrame( + kPayloadType, kType, kTimestampInfo, fake_clock_.CurrentTime(), kFrame, + sizeof(kFrame), hdr, kDefaultExpectedRetransmissionTime, {})); + ASSERT_EQ(transport_.packets_sent(), 2); + DependencyDescriptor descriptor_delta; + ASSERT_TRUE( + transport_.last_sent_packet() + .GetExtension<RtpDependencyDescriptorExtension>( + descriptor_key.attached_structure.get(), &descriptor_delta)); + EXPECT_THAT(descriptor_delta.frame_dependencies.frame_diffs, ElementsAre(1)); + EXPECT_THAT(descriptor_delta.frame_dependencies.chain_diffs, ElementsAre(0)); +} + +TEST_F(RtpSenderVideoTest, SendsFrameWithLargeChainDiffOfInactiveChain) { + const int64_t kFrameId = 100000; + uint8_t kFrame[100]; + rtp_module_->RegisterRtpHeaderExtension( + RtpDependencyDescriptorExtension::Uri(), kDependencyDescriptorId); + FrameDependencyStructure video_structure; + video_structure.num_decode_targets = 2; + video_structure.num_chains = 2; + video_structure.decode_target_protected_by_chain = {0, 1}; + video_structure.templates = { + FrameDependencyTemplate().Dtis("SS").ChainDiffs({0, 0}), + FrameDependencyTemplate().Dtis("SS").FrameDiffs({1}).ChainDiffs({1, 1}), + }; + rtp_sender_video_->SetVideoStructure(&video_structure); + + // Send key frame with only the first decode target, and thereby only the + // first chain, active. + RTPVideoHeader hdr; + RTPVideoHeader::GenericDescriptorInfo& generic = hdr.generic.emplace(); + generic.frame_id = kFrameId; + generic.decode_target_indications = {DecodeTargetIndication::kSwitch, + DecodeTargetIndication::kSwitch}; + generic.active_decode_targets = 0b01; + generic.chain_diffs = {0, 0}; + hdr.frame_type = VideoFrameType::kVideoFrameKey; + ASSERT_TRUE(rtp_sender_video_->SendVideoFrame( + kPayloadType, kType, kTimestampInfo, fake_clock_.CurrentTime(), kFrame, + sizeof(kFrame), hdr, kDefaultExpectedRetransmissionTime, {})); + ASSERT_EQ(transport_.packets_sent(), 1); + DependencyDescriptor descriptor_key; + ASSERT_TRUE(transport_.last_sent_packet() + .GetExtension<RtpDependencyDescriptorExtension>( + nullptr, &descriptor_key)); + ASSERT_TRUE(descriptor_key.attached_structure); + + // Send delta frame with a chain diff that doesn't fit in 8 bits on the + // inactive chain. That chain diff isn't serialized. + generic.frame_id = kFrameId + 1; + generic.dependencies = {kFrameId}; + generic.chain_diffs = {1, 300}; + hdr.frame_type = VideoFrameType::kVideoFrameDelta; + EXPECT_TRUE(rtp_sender_video_->SendVideoFrame( + kPayloadType, kType, kTimestampInfo, fake_clock_.CurrentTime(), kFrame, + sizeof(kFrame), hdr, kDefaultExpectedRetransmissionTime, {})); + ASSERT_EQ(transport_.packets_sent(), 2); + DependencyDescriptor descriptor_delta; + ASSERT_TRUE( + transport_.last_sent_packet() + .GetExtension<RtpDependencyDescriptorExtension>( + descriptor_key.attached_structure.get(), &descriptor_delta)); + EXPECT_THAT(descriptor_delta.frame_dependencies.frame_diffs, ElementsAre(1)); + // The diff of the inactive chain has no meaning, so only the diff of the + // active chain is checked. + EXPECT_THAT(descriptor_delta.frame_dependencies.chain_diffs, + ElementsAre(1, _)); +} + TEST_F(RtpSenderVideoTest, PropagatesActiveDecodeTargetsIntoDependencyDescriptor) { const int64_t kFrameId = 100000;