diff --git a/api/transport/rtp/dependency_descriptor.h b/api/transport/rtp/dependency_descriptor.h index a30608c162..6967c83517 100644 --- a/api/transport/rtp/dependency_descriptor.h +++ b/api/transport/rtp/dependency_descriptor.h @@ -91,8 +91,7 @@ struct FrameDependencyStructure { int num_decode_targets = 0; int num_chains = 0; // If chains are used (num_chains > 0), maps decode target index into index of - // the chain protecting that target or |num_chains| value if decode target is - // not protected by a chain. + // the chain protecting that target. absl::InlinedVector decode_target_protected_by_chain; absl::InlinedVector resolutions; std::vector templates; diff --git a/modules/rtp_rtcp/source/active_decode_targets_helper.cc b/modules/rtp_rtcp/source/active_decode_targets_helper.cc index 5ab4e0e1bc..71e7e8cf78 100644 --- a/modules/rtp_rtcp/source/active_decode_targets_helper.cc +++ b/modules/rtp_rtcp/source/active_decode_targets_helper.cc @@ -50,12 +50,9 @@ std::bitset<32> ActiveChains( if (dt < active_decode_targets.size() && !active_decode_targets[dt]) { continue; } - // chain_idx == num_chains is valid and means the decode target is - // not protected by any chain. int chain_idx = decode_target_protected_by_chain[dt]; - if (chain_idx < num_chains) { - active_chains.set(chain_idx); - } + RTC_DCHECK_LT(chain_idx, num_chains); + active_chains.set(chain_idx); } return active_chains; } @@ -109,20 +106,19 @@ void ActiveDecodeTargetsHelper::OnFrame( return; } last_active_decode_targets_ = active_decode_targets; + + if (active_decode_targets.none()) { + RTC_LOG(LS_ERROR) << "It is invalid to produce a frame (" << frame_id + << ") while there are no active decode targets"; + return; + } last_active_chains_ = ActiveChains(decode_target_protected_by_chain, num_chains, active_decode_targets); // Frames that are part of inactive chains might not be produced by the // encoder. Thus stop sending `active_decode_target` bitmask when it is sent // on all active chains rather than on all chains. unsent_on_chain_ = last_active_chains_; - if (unsent_on_chain_.none()) { - // Active decode targets are not protected by any chains. To be on the - // safe side always send the active_decode_targets_bitmask from now on. - RTC_LOG(LS_WARNING) - << "Active decode targets protected by no chains. (In)active decode " - "targets information will be send overreliably."; - unsent_on_chain_.set(1); - } + RTC_DCHECK(!unsent_on_chain_.none()); } } // namespace webrtc diff --git a/modules/rtp_rtcp/source/active_decode_targets_helper_unittest.cc b/modules/rtp_rtcp/source/active_decode_targets_helper_unittest.cc index 651ab22e54..6f64fd1418 100644 --- a/modules/rtp_rtcp/source/active_decode_targets_helper_unittest.cc +++ b/modules/rtp_rtcp/source/active_decode_targets_helper_unittest.cc @@ -241,29 +241,6 @@ TEST(ActiveDecodeTargetsHelperTest, ReturnsNulloptWhenChainsAreNotUsed) { EXPECT_EQ(helper.ActiveDecodeTargetsBitmask(), absl::nullopt); } -TEST(ActiveDecodeTargetsHelperTest, - KeepReturningBitmaskWhenAllChainsAreInactive) { - // Two decode targets, but single chain. - // 2nd decode target is not protected by any chain. - constexpr int kDecodeTargetProtectedByChain[] = {0, 1}; - - ActiveDecodeTargetsHelper helper; - int chain_diffs_key[] = {0}; - helper.OnFrame(kDecodeTargetProtectedByChain, /*active_decode_targets=*/0b10, - /*is_keyframe=*/true, - /*frame_id=*/0, chain_diffs_key); - EXPECT_EQ(helper.ActiveDecodeTargetsBitmask(), 0b10u); - - // Even though previous frame is part of the only chain, that inactive chain - // doesn't provide guaranted delivery. - int chain_diffs_delta[] = {1}; - helper.OnFrame(kDecodeTargetProtectedByChain, - /*active_decode_targets=*/0b10, - /*is_keyframe=*/false, - /*frame_id=*/1, chain_diffs_delta); - EXPECT_EQ(helper.ActiveDecodeTargetsBitmask(), 0b10u); -} - TEST(ActiveDecodeTargetsHelperTest, Supports32DecodeTargets) { std::bitset<32> some; std::vector decode_target_protected_by_chain(32); diff --git a/modules/rtp_rtcp/source/rtp_dependency_descriptor_reader.cc b/modules/rtp_rtcp/source/rtp_dependency_descriptor_reader.cc index 01b893a94e..cba594dc6f 100644 --- a/modules/rtp_rtcp/source/rtp_dependency_descriptor_reader.cc +++ b/modules/rtp_rtcp/source/rtp_dependency_descriptor_reader.cc @@ -146,7 +146,7 @@ void RtpDependencyDescriptorReader::ReadTemplateChains() { if (structure->num_chains == 0) return; for (int i = 0; i < structure->num_decode_targets; ++i) { - uint32_t protected_by_chain = ReadNonSymmetric(structure->num_chains + 1); + uint32_t protected_by_chain = ReadNonSymmetric(structure->num_chains); structure->decode_target_protected_by_chain.push_back(protected_by_chain); } for (FrameDependencyTemplate& frame_template : structure->templates) { diff --git a/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.cc b/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.cc index c5f229c59f..25d221253b 100644 --- a/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.cc +++ b/modules/rtp_rtcp/source/rtp_dependency_descriptor_writer.cc @@ -111,8 +111,8 @@ int RtpDependencyDescriptorWriter::StructureSizeBits() const { structure_.num_chains, structure_.num_decode_targets + 1); if (structure_.num_chains > 0) { for (int protected_by : structure_.decode_target_protected_by_chain) { - bits += rtc::BitBufferWriter::SizeNonSymmetricBits( - protected_by, structure_.num_chains + 1); + bits += rtc::BitBufferWriter::SizeNonSymmetricBits(protected_by, + structure_.num_chains); } bits += 4 * structure_.templates.size() * structure_.num_chains; } @@ -288,8 +288,8 @@ void RtpDependencyDescriptorWriter::WriteTemplateChains() { structure_.num_decode_targets); for (int protected_by : structure_.decode_target_protected_by_chain) { RTC_DCHECK_GE(protected_by, 0); - RTC_DCHECK_LE(protected_by, structure_.num_chains); - WriteNonSymmetric(protected_by, structure_.num_chains + 1); + RTC_DCHECK_LT(protected_by, structure_.num_chains); + WriteNonSymmetric(protected_by, structure_.num_chains); } for (const auto& frame_template : structure_.templates) { RTC_DCHECK_EQ(frame_template.chain_diffs.size(), structure_.num_chains); diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc index 5e8cf15de9..c53725e339 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc @@ -567,8 +567,7 @@ TEST_P(RtpSenderVideoTest, PropagatesChainDiffsIntoDependencyDescriptor) { FrameDependencyStructure video_structure; video_structure.num_decode_targets = 2; video_structure.num_chains = 1; - // First decode target is protected by the only chain, second one - is not. - video_structure.decode_target_protected_by_chain = {0, 1}; + video_structure.decode_target_protected_by_chain = {0, 0}; video_structure.templates = { FrameDependencyTemplate().S(0).T(0).Dtis("SS").ChainDiffs({1}), }; diff --git a/modules/video_coding/codecs/av1/scalability_structure_unittest.cc b/modules/video_coding/codecs/av1/scalability_structure_unittest.cc index 77b34c3ce1..4d0e283fdd 100644 --- a/modules/video_coding/codecs/av1/scalability_structure_unittest.cc +++ b/modules/video_coding/codecs/av1/scalability_structure_unittest.cc @@ -104,7 +104,7 @@ TEST_P(ScalabilityStructureTest, } else { EXPECT_THAT(structure.decode_target_protected_by_chain, AllOf(SizeIs(structure.num_decode_targets), Each(Ge(0)), - Each(Le(structure.num_chains)))); + Each(Lt(structure.num_chains)))); } EXPECT_THAT(structure.templates, SizeIs(Lt(size_t{DependencyDescriptor::kMaxTemplates})));