From 15b2ca7e772d7181f1ee3e426f1c341e89c9bbc2 Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Thu, 4 Aug 2022 11:52:37 +0000 Subject: [PATCH] Clean up VCMFrameInformation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Removes unused userData * Switches render time to a timestamp. Bug: None Change-Id: If6e055e9f5486081a850691f6c481c89b59d5de2 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/270580 Reviewed-by: Erik Språng Commit-Queue: Evan Shrubsole Cr-Commit-Position: refs/heads/main@{#37698} --- modules/video_coding/generic_decoder.cc | 30 +++++++++------- modules/video_coding/generic_decoder.h | 4 +-- modules/video_coding/timestamp_map.cc | 16 ++++----- modules/video_coding/timestamp_map.h | 23 ++++++------ .../video_coding/timestamp_map_unittest.cc | 36 +++++++++---------- 5 files changed, 57 insertions(+), 52 deletions(-) diff --git a/modules/video_coding/generic_decoder.cc b/modules/video_coding/generic_decoder.cc index 5cb20f9411..dd8f52aef0 100644 --- a/modules/video_coding/generic_decoder.cc +++ b/modules/video_coding/generic_decoder.cc @@ -15,11 +15,11 @@ #include #include +#include "absl/types/optional.h" #include "api/video/video_timing.h" #include "modules/video_coding/include/video_error_codes.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" -#include "rtc_base/time_utils.h" #include "rtc_base/trace_event.h" #include "system_wrappers/include/clock.h" @@ -74,7 +74,7 @@ void VCMDecodedFrameCallback::Decoded(VideoFrame& decodedImage, "timestamp", decodedImage.timestamp()); // TODO(holmer): We should improve this so that we can handle multiple // callbacks from one call to Decode(). - absl::optional frameInfo; + absl::optional frameInfo; int timestamp_map_size = 0; int dropped_frames = 0; { @@ -113,14 +113,14 @@ void VCMDecodedFrameCallback::Decoded(VideoFrame& decodedImage, } decodedImage.set_render_parameters(render_parameters); - RTC_DCHECK(frameInfo->decodeStart); + RTC_DCHECK(frameInfo->decode_start); const Timestamp now = _clock->CurrentTime(); const TimeDelta decode_time = decode_time_ms ? TimeDelta::Millis(*decode_time_ms) - : now - *frameInfo->decodeStart; + : now - *frameInfo->decode_start; _timing->StopDecodeTimer(decode_time, now); decodedImage.set_processing_time( - {*frameInfo->decodeStart, *frameInfo->decodeStart + decode_time}); + {*frameInfo->decode_start, *frameInfo->decode_start + decode_time}); // Report timing information. TimingFrameInfo timing_frame_info; @@ -164,16 +164,17 @@ void VCMDecodedFrameCallback::Decoded(VideoFrame& decodedImage, } timing_frame_info.flags = frameInfo->timing.flags; - timing_frame_info.decode_start_ms = frameInfo->decodeStart->ms(); + timing_frame_info.decode_start_ms = frameInfo->decode_start->ms(); timing_frame_info.decode_finish_ms = now.ms(); - timing_frame_info.render_time_ms = frameInfo->renderTimeMs; + timing_frame_info.render_time_ms = + frameInfo->render_time ? frameInfo->render_time->ms() : -1; timing_frame_info.rtp_timestamp = decodedImage.timestamp(); timing_frame_info.receive_start_ms = frameInfo->timing.receive_start_ms; timing_frame_info.receive_finish_ms = frameInfo->timing.receive_finish_ms; _timing->SetTimingFrameInfo(timing_frame_info); - decodedImage.set_timestamp_us(frameInfo->renderTimeMs * - rtc::kNumMicrosecsPerMillisec); + decodedImage.set_timestamp_us( + frameInfo->render_time ? frameInfo->render_time->us() : -1); _receiveCallback->FrameToRender(decodedImage, qp, decode_time, frameInfo->content_type); } @@ -184,7 +185,7 @@ void VCMDecodedFrameCallback::OnDecoderImplementationName( } void VCMDecodedFrameCallback::Map(uint32_t timestamp, - const VCMFrameInformation& frameInfo) { + const FrameInformation& frameInfo) { int dropped_frames = 0; { MutexLock lock(&lock_); @@ -237,9 +238,12 @@ bool VCMGenericDecoder::Configure(const VideoDecoder::Settings& settings) { int32_t VCMGenericDecoder::Decode(const VCMEncodedFrame& frame, Timestamp now) { TRACE_EVENT1("webrtc", "VCMGenericDecoder::Decode", "timestamp", frame.Timestamp()); - VCMFrameInformation frame_info; - frame_info.decodeStart = now; - frame_info.renderTimeMs = frame.RenderTimeMs(); + FrameInformation frame_info; + frame_info.decode_start = now; + frame_info.render_time = + frame.RenderTimeMs() >= 0 + ? absl::make_optional(Timestamp::Millis(frame.RenderTimeMs())) + : absl::nullopt; frame_info.rotation = frame.rotation(); frame_info.timing = frame.video_timing(); frame_info.ntp_time_ms = frame.EncodedImage().ntp_time_ms_; diff --git a/modules/video_coding/generic_decoder.h b/modules/video_coding/generic_decoder.h index 636bd1f0f4..70c79dace2 100644 --- a/modules/video_coding/generic_decoder.h +++ b/modules/video_coding/generic_decoder.h @@ -45,7 +45,7 @@ class VCMDecodedFrameCallback : public DecodedImageCallback { void OnDecoderImplementationName(const char* implementation_name); - void Map(uint32_t timestamp, const VCMFrameInformation& frameInfo); + void Map(uint32_t timestamp, const FrameInformation& frameInfo); void ClearTimestampMap(); private: @@ -60,7 +60,7 @@ class VCMDecodedFrameCallback : public DecodedImageCallback { VCMReceiveCallback* _receiveCallback = nullptr; VCMTiming* _timing; Mutex lock_; - VCMTimestampMap _timestampMap RTC_GUARDED_BY(lock_); + TimestampMap _timestampMap RTC_GUARDED_BY(lock_); int64_t ntp_offset_; }; diff --git a/modules/video_coding/timestamp_map.cc b/modules/video_coding/timestamp_map.cc index f843ea693a..ef5b4b2550 100644 --- a/modules/video_coding/timestamp_map.cc +++ b/modules/video_coding/timestamp_map.cc @@ -16,15 +16,15 @@ namespace webrtc { -VCMTimestampMap::VCMTimestampMap(size_t capacity) +TimestampMap::TimestampMap(size_t capacity) : ring_buffer_(new TimestampDataTuple[capacity]), capacity_(capacity), next_add_idx_(0), next_pop_idx_(0) {} -VCMTimestampMap::~VCMTimestampMap() {} +TimestampMap::~TimestampMap() {} -void VCMTimestampMap::Add(uint32_t timestamp, const VCMFrameInformation& data) { +void TimestampMap::Add(uint32_t timestamp, const FrameInformation& data) { ring_buffer_[next_add_idx_].timestamp = timestamp; ring_buffer_[next_add_idx_].data = data; next_add_idx_ = (next_add_idx_ + 1) % capacity_; @@ -35,11 +35,11 @@ void VCMTimestampMap::Add(uint32_t timestamp, const VCMFrameInformation& data) { } } -absl::optional VCMTimestampMap::Pop(uint32_t timestamp) { +absl::optional TimestampMap::Pop(uint32_t timestamp) { while (!IsEmpty()) { if (ring_buffer_[next_pop_idx_].timestamp == timestamp) { // Found start time for this timestamp. - const VCMFrameInformation& data = ring_buffer_[next_pop_idx_].data; + const FrameInformation& data = ring_buffer_[next_pop_idx_].data; ring_buffer_[next_pop_idx_].timestamp = 0; next_pop_idx_ = (next_pop_idx_ + 1) % capacity_; return data; @@ -57,11 +57,11 @@ absl::optional VCMTimestampMap::Pop(uint32_t timestamp) { return absl::nullopt; } -bool VCMTimestampMap::IsEmpty() const { +bool TimestampMap::IsEmpty() const { return (next_add_idx_ == next_pop_idx_); } -size_t VCMTimestampMap::Size() const { +size_t TimestampMap::Size() const { // The maximum number of elements in the list is `capacity_` - 1. The list is // empty if the add and pop indices are equal. return next_add_idx_ >= next_pop_idx_ @@ -69,7 +69,7 @@ size_t VCMTimestampMap::Size() const { : next_add_idx_ + capacity_ - next_pop_idx_; } -void VCMTimestampMap::Clear() { +void TimestampMap::Clear() { while (!IsEmpty()) { ring_buffer_[next_pop_idx_].timestamp = 0; next_pop_idx_ = (next_pop_idx_ + 1) % capacity_; diff --git a/modules/video_coding/timestamp_map.h b/modules/video_coding/timestamp_map.h index dc20a0551c..5ff75b10e3 100644 --- a/modules/video_coding/timestamp_map.h +++ b/modules/video_coding/timestamp_map.h @@ -19,14 +19,15 @@ #include "api/video/encoded_image.h" #include "api/video/video_content_type.h" #include "api/video/video_rotation.h" -#include "api/video/video_timing.h" namespace webrtc { -struct VCMFrameInformation { - int64_t renderTimeMs; - absl::optional decodeStart; - void* userData; +struct FrameInformation { + // This is likely not optional, but some inputs seem to sometimes be negative. + // TODO(bugs.webrtc.org/13756): See if this can be replaced with Timestamp + // once all inputs to this field use Timestamp instead of an integer. + absl::optional render_time; + absl::optional decode_start; VideoRotation rotation; VideoContentType content_type; EncodedImage::Timing timing; @@ -35,20 +36,20 @@ struct VCMFrameInformation { // ColorSpace is not stored here, as it might be modified by decoders. }; -class VCMTimestampMap { +class TimestampMap { public: - explicit VCMTimestampMap(size_t capacity); - ~VCMTimestampMap(); + explicit TimestampMap(size_t capacity); + ~TimestampMap(); - void Add(uint32_t timestamp, const VCMFrameInformation& data); - absl::optional Pop(uint32_t timestamp); + void Add(uint32_t timestamp, const FrameInformation& data); + absl::optional Pop(uint32_t timestamp); size_t Size() const; void Clear(); private: struct TimestampDataTuple { uint32_t timestamp; - VCMFrameInformation data; + FrameInformation data; }; bool IsEmpty() const; diff --git a/modules/video_coding/timestamp_map_unittest.cc b/modules/video_coding/timestamp_map_unittest.cc index 5e90786b95..4fc8897dbe 100644 --- a/modules/video_coding/timestamp_map_unittest.cc +++ b/modules/video_coding/timestamp_map_unittest.cc @@ -24,12 +24,12 @@ constexpr int kTimestamp4 = 4; constexpr int kTimestamp5 = 5; constexpr int kTimestamp6 = 6; constexpr int kTimestamp7 = 7; -constexpr int64_t kRenderTime1 = 1000; -constexpr int64_t kRenderTime2 = 2000; -constexpr int64_t kRenderTime4 = 4000; -constexpr int64_t kRenderTime5 = 5000; -constexpr int64_t kRenderTime6 = 6000; -constexpr int64_t kRenderTime7 = 7000; +constexpr Timestamp kRenderTime1 = Timestamp::Seconds(1); +constexpr Timestamp kRenderTime2 = Timestamp::Seconds(2); +constexpr Timestamp kRenderTime4 = Timestamp::Seconds(4); +constexpr Timestamp kRenderTime5 = Timestamp::Seconds(5); +constexpr Timestamp kRenderTime6 = Timestamp::Seconds(6); +constexpr Timestamp kRenderTime7 = Timestamp::Seconds(7); } // namespace class VcmTimestampMapTest : public ::testing::Test { @@ -37,25 +37,25 @@ class VcmTimestampMapTest : public ::testing::Test { VcmTimestampMapTest() : _timestampMap(kTimestampMapSize) {} void SetUp() override { - _timestampMap.Add(kTimestamp1, VCMFrameInformation({kRenderTime1})); - _timestampMap.Add(kTimestamp2, VCMFrameInformation({kRenderTime2})); - _timestampMap.Add(kTimestamp4, VCMFrameInformation({kRenderTime4})); + _timestampMap.Add(kTimestamp1, FrameInformation({kRenderTime1})); + _timestampMap.Add(kTimestamp2, FrameInformation({kRenderTime2})); + _timestampMap.Add(kTimestamp4, FrameInformation({kRenderTime4})); } - VCMTimestampMap _timestampMap; + TimestampMap _timestampMap; }; TEST_F(VcmTimestampMapTest, PopExistingFrameInfo) { EXPECT_EQ(_timestampMap.Size(), 3u); auto frameInfo = _timestampMap.Pop(kTimestamp1); ASSERT_TRUE(frameInfo); - EXPECT_EQ(frameInfo->renderTimeMs, kRenderTime1); + EXPECT_EQ(frameInfo->render_time, kRenderTime1); frameInfo = _timestampMap.Pop(kTimestamp2); ASSERT_TRUE(frameInfo); - EXPECT_EQ(frameInfo->renderTimeMs, kRenderTime2); + EXPECT_EQ(frameInfo->render_time, kRenderTime2); frameInfo = _timestampMap.Pop(kTimestamp4); ASSERT_TRUE(frameInfo); - EXPECT_EQ(frameInfo->renderTimeMs, kRenderTime4); + EXPECT_EQ(frameInfo->render_time, kRenderTime4); } TEST_F(VcmTimestampMapTest, PopNonexistingClearsOlderFrameInfos) { @@ -66,9 +66,9 @@ TEST_F(VcmTimestampMapTest, PopNonexistingClearsOlderFrameInfos) { TEST_F(VcmTimestampMapTest, SizeIsIncrementedWhenAddingNewFrameInfo) { EXPECT_EQ(_timestampMap.Size(), 3u); - _timestampMap.Add(kTimestamp5, VCMFrameInformation({kRenderTime5})); + _timestampMap.Add(kTimestamp5, FrameInformation({kRenderTime5})); EXPECT_EQ(_timestampMap.Size(), 4u); - _timestampMap.Add(kTimestamp6, VCMFrameInformation({kRenderTime6})); + _timestampMap.Add(kTimestamp6, FrameInformation({kRenderTime6})); EXPECT_EQ(_timestampMap.Size(), 5u); } @@ -101,11 +101,11 @@ TEST_F(VcmTimestampMapTest, PopLastAddedClearsMap) { TEST_F(VcmTimestampMapTest, LastAddedIsDiscardedIfMapGetsFull) { EXPECT_EQ(_timestampMap.Size(), 3u); - _timestampMap.Add(kTimestamp5, VCMFrameInformation({kRenderTime5})); + _timestampMap.Add(kTimestamp5, FrameInformation({kRenderTime5})); EXPECT_EQ(_timestampMap.Size(), 4u); - _timestampMap.Add(kTimestamp6, VCMFrameInformation({kRenderTime6})); + _timestampMap.Add(kTimestamp6, FrameInformation({kRenderTime6})); EXPECT_EQ(_timestampMap.Size(), 5u); - _timestampMap.Add(kTimestamp7, VCMFrameInformation({kRenderTime7})); + _timestampMap.Add(kTimestamp7, FrameInformation({kRenderTime7})); // Size is not incremented since the oldest element is discarded. EXPECT_EQ(_timestampMap.Size(), 5u); EXPECT_FALSE(_timestampMap.Pop(kTimestamp1));