From 02d71fb8825994fd25316544651e28bbb4ab4a4a Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 10 Feb 2020 16:22:57 +0100 Subject: [PATCH] Populate generic descriptor based on GenericFrameInfo when available. Bug: webrtc:10342 Change-Id: Iff769d2604fd79784bcb09874d2803793d20bde5 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/167000 Reviewed-by: Philip Eliasson Reviewed-by: Niels Moller Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#30505} --- call/BUILD.gn | 1 + call/rtp_payload_params.cc | 28 ++++++++++++ call/rtp_payload_params.h | 6 +++ call/rtp_payload_params_unittest.cc | 44 +++++++++++++++++++ modules/video_coding/BUILD.gn | 2 - .../frame_dependencies_calculator.cc | 3 -- .../frame_dependencies_calculator.h | 10 ++--- 7 files changed, 83 insertions(+), 11 deletions(-) diff --git a/call/BUILD.gn b/call/BUILD.gn index e14370c53d..b203377427 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -167,6 +167,7 @@ rtc_library("rtp_sender") { "../modules/rtp_rtcp:rtp_video_header", "../modules/utility", "../modules/video_coding:codec_globals_headers", + "../modules/video_coding:frame_dependencies_calculator", "../modules/video_coding:video_codec_interface", "../rtc_base", "../rtc_base:checks", diff --git a/call/rtp_payload_params.cc b/call/rtp_payload_params.cc index 408a2a85f6..f69a52b05e 100644 --- a/call/rtp_payload_params.cc +++ b/call/rtp_payload_params.cc @@ -14,6 +14,7 @@ #include +#include "absl/algorithm/container.h" #include "absl/container/inlined_vector.h" #include "absl/types/variant.h" #include "api/video/video_timing.h" @@ -21,6 +22,7 @@ #include "modules/video_coding/codecs/interface/common_constants.h" #include "modules/video_coding/codecs/vp8/include/vp8_globals.h" #include "modules/video_coding/codecs/vp9/include/vp9_globals.h" +#include "modules/video_coding/frame_dependencies_calculator.h" #include "rtc_base/arraysize.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" @@ -248,10 +250,36 @@ void RtpPayloadParams::SetCodecSpecific(RTPVideoHeader* rtp_video_header, } } +RTPVideoHeader::GenericDescriptorInfo +RtpPayloadParams::GenericDescriptorFromFrameInfo( + const GenericFrameInfo& frame_info, + int64_t frame_id, + VideoFrameType frame_type) { + RTPVideoHeader::GenericDescriptorInfo generic; + generic.frame_id = frame_id; + generic.dependencies = dependencies_calculator_.FromBuffersUsage( + frame_type, frame_id, frame_info.encoder_buffers); + generic.spatial_index = frame_info.spatial_id; + generic.temporal_index = frame_info.temporal_id; + generic.decode_target_indications = frame_info.decode_target_indications; + generic.discardable = + absl::c_linear_search(frame_info.decode_target_indications, + DecodeTargetIndication::kDiscardable); + return generic; +} + void RtpPayloadParams::SetGeneric(const CodecSpecificInfo* codec_specific_info, int64_t frame_id, bool is_keyframe, RTPVideoHeader* rtp_video_header) { + if (codec_specific_info && codec_specific_info->generic_frame_info && + !codec_specific_info->generic_frame_info->encoder_buffers.empty()) { + rtp_video_header->generic = + GenericDescriptorFromFrameInfo(*codec_specific_info->generic_frame_info, + frame_id, rtp_video_header->frame_type); + return; + } + switch (rtp_video_header->codec) { case VideoCodecType::kVideoCodecGeneric: GenericToGeneric(frame_id, is_keyframe, rtp_video_header); diff --git a/call/rtp_payload_params.h b/call/rtp_payload_params.h index b012398518..95a9cb762a 100644 --- a/call/rtp_payload_params.h +++ b/call/rtp_payload_params.h @@ -18,6 +18,7 @@ #include "call/rtp_config.h" #include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h" #include "modules/rtp_rtcp/source/rtp_video_header.h" +#include "modules/video_coding/frame_dependencies_calculator.h" #include "modules/video_coding/include/video_codec_interface.h" namespace webrtc { @@ -43,6 +44,10 @@ class RtpPayloadParams final { private: void SetCodecSpecific(RTPVideoHeader* rtp_video_header, bool first_frame_in_picture); + RTPVideoHeader::GenericDescriptorInfo GenericDescriptorFromFrameInfo( + const GenericFrameInfo& frame_info, + int64_t frame_id, + VideoFrameType frame_type); void SetGeneric(const CodecSpecificInfo* codec_specific_info, int64_t frame_id, bool is_keyframe, @@ -79,6 +84,7 @@ class RtpPayloadParams final { bool layer_sync, RTPVideoHeader::GenericDescriptorInfo* generic); + FrameDependenciesCalculator dependencies_calculator_; // TODO(bugs.webrtc.org/10242): Remove once all encoder-wrappers are updated. // Holds the last shared frame id for a given (spatial, temporal) layer. std::array, diff --git a/call/rtp_payload_params_unittest.cc b/call/rtp_payload_params_unittest.cc index ad5d8e1303..b8fd4a574e 100644 --- a/call/rtp_payload_params_unittest.cc +++ b/call/rtp_payload_params_unittest.cc @@ -371,6 +371,50 @@ TEST(RtpPayloadParamsTest, GenericDescriptorForGenericCodec) { EXPECT_THAT(header.generic->dependencies, ElementsAre(0)); } +TEST(RtpPayloadParamsTest, SetsGenericFromGenericFrameInfo) { + test::ScopedFieldTrials generic_picture_id( + "WebRTC-GenericDescriptor/Enabled/"); + RtpPayloadState state; + EncodedImage encoded_image; + CodecSpecificInfo codec_info; + + RtpPayloadParams params(kSsrc1, &state); + + encoded_image._frameType = VideoFrameType::kVideoFrameKey; + codec_info.generic_frame_info = + GenericFrameInfo::Builder().S(1).T(0).Dtis("S").Build(); + codec_info.generic_frame_info->encoder_buffers = { + {/*id=*/0, /*referenced=*/false, /*updated=*/true}}; + RTPVideoHeader key_header = + params.GetRtpVideoHeader(encoded_image, &codec_info, /*frame_id=*/1); + + ASSERT_TRUE(key_header.generic); + EXPECT_EQ(key_header.generic->spatial_index, 1); + EXPECT_EQ(key_header.generic->temporal_index, 0); + EXPECT_EQ(key_header.generic->frame_id, 1); + EXPECT_THAT(key_header.generic->dependencies, IsEmpty()); + EXPECT_THAT(key_header.generic->decode_target_indications, + ElementsAre(DecodeTargetIndication::kSwitch)); + EXPECT_FALSE(key_header.generic->discardable); + + encoded_image._frameType = VideoFrameType::kVideoFrameDelta; + codec_info.generic_frame_info = + GenericFrameInfo::Builder().S(2).T(3).Dtis("D").Build(); + codec_info.generic_frame_info->encoder_buffers = { + {/*id=*/0, /*referenced=*/true, /*updated=*/false}}; + RTPVideoHeader delta_header = + params.GetRtpVideoHeader(encoded_image, &codec_info, /*frame_id=*/3); + + ASSERT_TRUE(delta_header.generic); + EXPECT_EQ(delta_header.generic->spatial_index, 2); + EXPECT_EQ(delta_header.generic->temporal_index, 3); + EXPECT_EQ(delta_header.generic->frame_id, 3); + EXPECT_THAT(delta_header.generic->dependencies, ElementsAre(1)); + EXPECT_THAT(delta_header.generic->decode_target_indications, + ElementsAre(DecodeTargetIndication::kDiscardable)); + EXPECT_TRUE(delta_header.generic->discardable); +} + class RtpPayloadParamsVp8ToGenericTest : public ::testing::Test { public: enum LayerSync { kNoSync, kSync }; diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index 4ae65dfe2b..25cafbc318 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -46,8 +46,6 @@ rtc_library("frame_dependencies_calculator") { "../../common_video/generic_frame_descriptor", "../../rtc_base:checks", "../../rtc_base:logging", - "../../rtc_base:macromagic", - "../../rtc_base/synchronization:sequence_checker", "//third_party/abseil-cpp/absl/algorithm:container", "//third_party/abseil-cpp/absl/container:inlined_vector", "//third_party/abseil-cpp/absl/types:optional", diff --git a/modules/video_coding/frame_dependencies_calculator.cc b/modules/video_coding/frame_dependencies_calculator.cc index c3042d9fad..6de5081b94 100644 --- a/modules/video_coding/frame_dependencies_calculator.cc +++ b/modules/video_coding/frame_dependencies_calculator.cc @@ -20,7 +20,6 @@ #include "api/video/video_frame_type.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" -#include "rtc_base/synchronization/sequence_checker.h" namespace webrtc { @@ -28,8 +27,6 @@ absl::InlinedVector FrameDependenciesCalculator::FromBuffersUsage( VideoFrameType frame_type, int64_t frame_id, rtc::ArrayView buffers_usage) { - RTC_DCHECK_RUN_ON(&checker_); - absl::InlinedVector dependencies; RTC_DCHECK_GT(buffers_usage.size(), 0); for (const CodecBufferUsage& buffer_usage : buffers_usage) { diff --git a/modules/video_coding/frame_dependencies_calculator.h b/modules/video_coding/frame_dependencies_calculator.h index f723d0f031..b70eddfc53 100644 --- a/modules/video_coding/frame_dependencies_calculator.h +++ b/modules/video_coding/frame_dependencies_calculator.h @@ -20,16 +20,15 @@ #include "api/array_view.h" #include "api/video/video_frame_type.h" #include "common_video/generic_frame_descriptor/generic_frame_info.h" -#include "rtc_base/synchronization/sequence_checker.h" -#include "rtc_base/thread_annotations.h" namespace webrtc { +// This class is thread compatible. class FrameDependenciesCalculator { public: FrameDependenciesCalculator() = default; - FrameDependenciesCalculator(FrameDependenciesCalculator&&) = default; - FrameDependenciesCalculator& operator=(FrameDependenciesCalculator&&) = + FrameDependenciesCalculator(const FrameDependenciesCalculator&) = default; + FrameDependenciesCalculator& operator=(const FrameDependenciesCalculator&) = default; // Calculates frame dependencies based on previous encoder buffer usage. @@ -44,8 +43,7 @@ class FrameDependenciesCalculator { absl::InlinedVector dependencies; }; - SequenceChecker checker_; - absl::InlinedVector buffers_ RTC_GUARDED_BY(checker_); + absl::InlinedVector buffers_; }; } // namespace webrtc