Clean up VCMFrameInformation
* 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 <sprang@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37698}
diff --git a/modules/video_coding/generic_decoder.cc b/modules/video_coding/generic_decoder.cc
index 5cb20f9..dd8f52a 100644
--- a/modules/video_coding/generic_decoder.cc
+++ b/modules/video_coding/generic_decoder.cc
@@ -15,11 +15,11 @@
#include <algorithm>
#include <cmath>
+#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 @@
"timestamp", decodedImage.timestamp());
// TODO(holmer): We should improve this so that we can handle multiple
// callbacks from one call to Decode().
- absl::optional<VCMFrameInformation> frameInfo;
+ absl::optional<FrameInformation> frameInfo;
int timestamp_map_size = 0;
int dropped_frames = 0;
{
@@ -113,14 +113,14 @@
}
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 @@
}
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::Map(uint32_t timestamp,
- const VCMFrameInformation& frameInfo) {
+ const FrameInformation& frameInfo) {
int dropped_frames = 0;
{
MutexLock lock(&lock_);
@@ -237,9 +238,12 @@
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 636bd1f..70c79da 100644
--- a/modules/video_coding/generic_decoder.h
+++ b/modules/video_coding/generic_decoder.h
@@ -45,7 +45,7 @@
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 @@
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 f843ea6..ef5b4b2 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 @@
}
}
-absl::optional<VCMFrameInformation> VCMTimestampMap::Pop(uint32_t timestamp) {
+absl::optional<FrameInformation> 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 @@
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 @@
: 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 dc20a05..5ff75b1 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<Timestamp> 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<Timestamp> render_time;
+ absl::optional<Timestamp> decode_start;
VideoRotation rotation;
VideoContentType content_type;
EncodedImage::Timing timing;
@@ -35,20 +36,20 @@
// 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<VCMFrameInformation> Pop(uint32_t timestamp);
+ void Add(uint32_t timestamp, const FrameInformation& data);
+ absl::optional<FrameInformation> 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 5e90786..4fc8897 100644
--- a/modules/video_coding/timestamp_map_unittest.cc
+++ b/modules/video_coding/timestamp_map_unittest.cc
@@ -24,12 +24,12 @@
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 @@
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, 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, 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));