From e6ac8ff162eca0b4d1554a8532cc59bd0adad9fe Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Fri, 26 Jun 2020 13:51:08 +0200 Subject: [PATCH] Propagate active decode targets bitmask into DependencyDescriptor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:10342 Change-Id: I5e8a204881b94fe5786b14e27cefce2fe056e91b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178140 Reviewed-by: Björn Terelius Reviewed-by: Philip Eliasson Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#31579} --- call/rtp_payload_params.cc | 1 + .../source/active_decode_targets_helper.cc | 7 ++-- .../source/active_decode_targets_helper.h | 3 ++ modules/rtp_rtcp/source/rtp_sender_video.cc | 15 +++++++- modules/rtp_rtcp/source/rtp_sender_video.h | 4 +++ .../source/rtp_sender_video_unittest.cc | 34 +++++++++++++++++++ modules/rtp_rtcp/source/rtp_video_header.h | 2 ++ 7 files changed, 62 insertions(+), 4 deletions(-) diff --git a/call/rtp_payload_params.cc b/call/rtp_payload_params.cc index 110db2e9fa..ad979a590a 100644 --- a/call/rtp_payload_params.cc +++ b/call/rtp_payload_params.cc @@ -245,6 +245,7 @@ RtpPayloadParams::GenericDescriptorFromFrameInfo( generic.spatial_index = frame_info.spatial_id; generic.temporal_index = frame_info.temporal_id; generic.decode_target_indications = frame_info.decode_target_indications; + generic.active_decode_targets = frame_info.active_decode_targets; return generic; } diff --git a/modules/rtp_rtcp/source/active_decode_targets_helper.cc b/modules/rtp_rtcp/source/active_decode_targets_helper.cc index a14426e144..5ab4e0e1bc 100644 --- a/modules/rtp_rtcp/source/active_decode_targets_helper.cc +++ b/modules/rtp_rtcp/source/active_decode_targets_helper.cc @@ -93,6 +93,7 @@ void ActiveDecodeTargetsHelper::OnFrame( if (is_keyframe) { // Key frame resets the state. last_active_decode_targets_ = all_decode_targets; + last_active_chains_ = AllActive(num_chains); unsent_on_chain_.reset(); } else { // Update state assuming previous frame was sent. @@ -108,12 +109,12 @@ void ActiveDecodeTargetsHelper::OnFrame( return; } last_active_decode_targets_ = active_decode_targets; - + 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_ = ActiveChains(decode_target_protected_by_chain, num_chains, - active_decode_targets); + 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. diff --git a/modules/rtp_rtcp/source/active_decode_targets_helper.h b/modules/rtp_rtcp/source/active_decode_targets_helper.h index b51144d9cb..13755e8d80 100644 --- a/modules/rtp_rtcp/source/active_decode_targets_helper.h +++ b/modules/rtp_rtcp/source/active_decode_targets_helper.h @@ -47,11 +47,14 @@ class ActiveDecodeTargetsHelper { return last_active_decode_targets_.to_ulong(); } + std::bitset<32> ActiveChainsBitmask() const { return last_active_chains_; } + private: // `unsent_on_chain_[i]` indicates last active decode // target bitmask wasn't attached to a packet on the chain with id `i`. std::bitset<32> unsent_on_chain_ = 0; std::bitset<32> last_active_decode_targets_ = 0; + std::bitset<32> last_active_chains_ = 0; int64_t last_frame_id_ = 0; }; diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 58a8699688..9ebfa77b8a 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -335,6 +335,10 @@ void RTPSenderVideo::AddRtpHeaderExtensions( descriptor.frame_dependencies.decode_target_indications.size(), video_structure_->num_decode_targets); + if (first_packet) { + descriptor.active_decode_targets_bitmask = + active_decode_targets_tracker_.ActiveDecodeTargetsBitmask(); + } // To avoid extra structure copy, temporary share ownership of the // video_structure with the dependency descriptor. if (video_header.frame_type == VideoFrameType::kVideoFrameKey && @@ -343,7 +347,8 @@ void RTPSenderVideo::AddRtpHeaderExtensions( absl::WrapUnique(video_structure_.get()); } extension_is_set = packet->SetExtension( - *video_structure_, descriptor); + *video_structure_, + active_decode_targets_tracker_.ActiveChainsBitmask(), descriptor); // Remove the temporary shared ownership. descriptor.attached_structure.release(); @@ -415,6 +420,14 @@ bool RTPSenderVideo::SendVideo( playout_delay_pending_ = true; } + if (video_structure_ != nullptr && video_header.generic) { + active_decode_targets_tracker_.OnFrame( + video_structure_->decode_target_protected_by_chain, + video_header.generic->active_decode_targets, + video_header.frame_type == VideoFrameType::kVideoFrameKey, + video_header.generic->frame_id, video_header.generic->chain_diffs); + } + // Maximum size of packet including rtp headers. // Extra space left in case packet will be resent using fec or rtx. int packet_capacity = rtp_sender_->MaxRtpPacketSize() - FecPacketOverhead() - diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h index 699734efa3..6a4c73d517 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/modules/rtp_rtcp/source/rtp_sender_video.h @@ -27,6 +27,7 @@ #include "modules/include/module_common_types.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/absolute_capture_time_sender.h" +#include "modules/rtp_rtcp/source/active_decode_targets_helper.h" #include "modules/rtp_rtcp/source/rtp_rtcp_config.h" #include "modules/rtp_rtcp/source/rtp_sender.h" #include "modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.h" @@ -214,6 +215,9 @@ class RTPSenderVideo { const bool generic_descriptor_auth_experiment_; AbsoluteCaptureTimeSender absolute_capture_time_sender_; + // Tracks updates to the active decode targets and decides when active decode + // targets bitmask should be attached to the dependency descriptor. + ActiveDecodeTargetsHelper active_decode_targets_tracker_; const rtc::scoped_refptr frame_transformer_delegate_; diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc index 32e138f840..5e8cf15de9 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc @@ -593,6 +593,40 @@ TEST_P(RtpSenderVideoTest, PropagatesChainDiffsIntoDependencyDescriptor) { ContainerEq(generic.chain_diffs)); } +TEST_P(RtpSenderVideoTest, + PropagatesActiveDecodeTargetsIntoDependencyDescriptor) { + const int64_t kFrameId = 100000; + uint8_t kFrame[100]; + rtp_module_->RegisterRtpHeaderExtension( + RtpDependencyDescriptorExtension::kUri, kDependencyDescriptorId); + FrameDependencyStructure video_structure; + video_structure.num_decode_targets = 2; + video_structure.num_chains = 1; + video_structure.decode_target_protected_by_chain = {0, 0}; + video_structure.templates = { + FrameDependencyTemplate().S(0).T(0).Dtis("SS").ChainDiffs({1}), + }; + rtp_sender_video_.SetVideoStructure(&video_structure); + + 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 = {1}; + hdr.frame_type = VideoFrameType::kVideoFrameKey; + rtp_sender_video_.SendVideo(kPayload, kType, kTimestamp, 0, kFrame, nullptr, + hdr, kDefaultExpectedRetransmissionTimeMs); + + ASSERT_EQ(transport_.packets_sent(), 1); + DependencyDescriptor descriptor_key; + ASSERT_TRUE(transport_.last_sent_packet() + .GetExtension( + nullptr, &descriptor_key)); + EXPECT_EQ(descriptor_key.active_decode_targets_bitmask, 0b01u); +} + TEST_P(RtpSenderVideoTest, SetDiffentVideoStructureAvoidsCollisionWithThePreviousStructure) { const int64_t kFrameId = 100000; diff --git a/modules/rtp_rtcp/source/rtp_video_header.h b/modules/rtp_rtcp/source/rtp_video_header.h index 514340add6..a9c144033d 100644 --- a/modules/rtp_rtcp/source/rtp_video_header.h +++ b/modules/rtp_rtcp/source/rtp_video_header.h @@ -10,6 +10,7 @@ #ifndef MODULES_RTP_RTCP_SOURCE_RTP_VIDEO_HEADER_H_ #define MODULES_RTP_RTCP_SOURCE_RTP_VIDEO_HEADER_H_ +#include #include #include "absl/container/inlined_vector.h" @@ -53,6 +54,7 @@ struct RTPVideoHeader { absl::InlinedVector decode_target_indications; absl::InlinedVector dependencies; absl::InlinedVector chain_diffs; + std::bitset<32> active_decode_targets = ~uint32_t{0}; }; RTPVideoHeader();