From 1c0dea8675436aac4e2177f6e0d7eaf3738cbddb Mon Sep 17 00:00:00 2001 From: nisse Date: Mon, 30 Jan 2017 02:43:18 -0800 Subject: [PATCH] Delete VideoFrame::set_render_time_ms. Also mark the render_time_ms getter function and the ntp timestamp as deprecated. BUG=webrtc:6977 Review-Url: https://codereview.webrtc.org/2633493002 Cr-Commit-Position: refs/heads/master@{#16354} --- webrtc/api/video/video_frame.cc | 4 ---- webrtc/api/video/video_frame.h | 6 +++--- webrtc/common_video/i420_video_frame_unittest.cc | 14 +++++++------- webrtc/modules/video_coding/generic_decoder.cc | 3 ++- webrtc/test/frame_generator_unittest.cc | 2 +- webrtc/video/video_send_stream_tests.cc | 3 ++- webrtc/video/vie_encoder.cc | 13 ++++++++----- 7 files changed, 23 insertions(+), 22 deletions(-) diff --git a/webrtc/api/video/video_frame.cc b/webrtc/api/video/video_frame.cc index bd9c72c352..0e3efb2be1 100644 --- a/webrtc/api/video/video_frame.cc +++ b/webrtc/api/video/video_frame.cc @@ -55,10 +55,6 @@ rtc::scoped_refptr VideoFrame::video_frame_buffer() const { return video_frame_buffer_; } -void VideoFrame::set_render_time_ms(int64_t render_time_ms) { - set_timestamp_us(render_time_ms * rtc::kNumMicrosecsPerMillisec); -} - int64_t VideoFrame::render_time_ms() const { return timestamp_us() / rtc::kNumMicrosecsPerMillisec; } diff --git a/webrtc/api/video/video_frame.h b/webrtc/api/video/video_frame.h index f960fa9200..5c57213f01 100644 --- a/webrtc/api/video/video_frame.h +++ b/webrtc/api/video/video_frame.h @@ -71,9 +71,11 @@ class VideoFrame { uint32_t transport_frame_id() const { return timestamp(); } // Set capture ntp time in milliseconds. + // TODO(nisse): Deprecated. Migrate all users to timestamp_us(). void set_ntp_time_ms(int64_t ntp_time_ms) { ntp_time_ms_ = ntp_time_ms; } // Get capture ntp time in milliseconds. + // TODO(nisse): Deprecated. Migrate all users to timestamp_us(). int64_t ntp_time_ms() const { return ntp_time_ms_; } // Naming convention for Coordination of Video Orientation. Please see @@ -89,10 +91,8 @@ class VideoFrame { VideoRotation rotation() const { return rotation_; } void set_rotation(VideoRotation rotation) { rotation_ = rotation; } - // Set render time in milliseconds. - void set_render_time_ms(int64_t render_time_ms); - // Get render time in milliseconds. + // TODO(nisse): Deprecated. Migrate all users to timestamp_us(). int64_t render_time_ms() const; // Return the underlying buffer. Never nullptr for a properly diff --git a/webrtc/common_video/i420_video_frame_unittest.cc b/webrtc/common_video/i420_video_frame_unittest.cc index e03762eb39..8c97ca7ac4 100644 --- a/webrtc/common_video/i420_video_frame_unittest.cc +++ b/webrtc/common_video/i420_video_frame_unittest.cc @@ -129,7 +129,7 @@ TEST(TestVideoFrame, WidthHeightValues) { TEST(TestVideoFrame, ShallowCopy) { uint32_t timestamp = 1; int64_t ntp_time_ms = 2; - int64_t render_time_ms = 3; + int64_t timestamp_us = 3; int stride_y = 15; int stride_u = 10; int stride_v = 10; @@ -155,7 +155,7 @@ TEST(TestVideoFrame, ShallowCopy) { kRotation, 0); frame1.set_timestamp(timestamp); frame1.set_ntp_time_ms(ntp_time_ms); - frame1.set_render_time_ms(render_time_ms); + frame1.set_timestamp_us(timestamp_us); VideoFrame frame2(frame1); EXPECT_EQ(frame1.video_frame_buffer(), frame2.video_frame_buffer()); @@ -168,17 +168,17 @@ TEST(TestVideoFrame, ShallowCopy) { EXPECT_EQ(frame2.timestamp(), frame1.timestamp()); EXPECT_EQ(frame2.ntp_time_ms(), frame1.ntp_time_ms()); - EXPECT_EQ(frame2.render_time_ms(), frame1.render_time_ms()); + EXPECT_EQ(frame2.timestamp_us(), frame1.timestamp_us()); EXPECT_EQ(frame2.rotation(), frame1.rotation()); frame2.set_timestamp(timestamp + 1); frame2.set_ntp_time_ms(ntp_time_ms + 1); - frame2.set_render_time_ms(render_time_ms + 1); + frame2.set_timestamp_us(timestamp_us + 1); frame2.set_rotation(kVideoRotation_90); EXPECT_NE(frame2.timestamp(), frame1.timestamp()); EXPECT_NE(frame2.ntp_time_ms(), frame1.ntp_time_ms()); - EXPECT_NE(frame2.render_time_ms(), frame1.render_time_ms()); + EXPECT_NE(frame2.timestamp_us(), frame1.timestamp_us()); EXPECT_NE(frame2.rotation(), frame1.rotation()); } @@ -195,8 +195,8 @@ TEST(TestVideoFrame, TextureInitialValues) { frame.set_timestamp(200); EXPECT_EQ(200u, frame.timestamp()); - frame.set_render_time_ms(20); - EXPECT_EQ(20, frame.render_time_ms()); + frame.set_timestamp_us(20); + EXPECT_EQ(20, frame.timestamp_us()); } TEST(TestI420FrameBuffer, Copy) { diff --git a/webrtc/modules/video_coding/generic_decoder.cc b/webrtc/modules/video_coding/generic_decoder.cc index b95571b8f7..b1059e8a1b 100644 --- a/webrtc/modules/video_coding/generic_decoder.cc +++ b/webrtc/modules/video_coding/generic_decoder.cc @@ -74,7 +74,8 @@ int32_t VCMDecodedFrameCallback::Decoded(VideoFrame& decodedImage, _timing->StopDecodeTimer(decodedImage.timestamp(), decode_time_ms, now_ms, frameInfo->renderTimeMs); - decodedImage.set_render_time_ms(frameInfo->renderTimeMs); + decodedImage.set_timestamp_us( + frameInfo->renderTimeMs * rtc::kNumMicrosecsPerMillisec); decodedImage.set_rotation(frameInfo->rotation); // TODO(sakal): Investigate why callback is NULL sometimes and replace if // statement with a DCHECK. diff --git a/webrtc/test/frame_generator_unittest.cc b/webrtc/test/frame_generator_unittest.cc index 843a4c348c..8867c0154e 100644 --- a/webrtc/test/frame_generator_unittest.cc +++ b/webrtc/test/frame_generator_unittest.cc @@ -75,7 +75,7 @@ class FrameGeneratorTest : public ::testing::Test { // Mutate to something arbitrary non-zero. frame->set_ntp_time_ms(11); - frame->set_render_time_ms(12); + frame->set_timestamp_us(12); frame->set_timestamp(13); } diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 2d6f9fb660..d128399f8a 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -1939,7 +1939,8 @@ VideoFrame CreateVideoFrame(int width, int height, uint8_t data) { I420Buffer::Create(width, height, width, width / 2, width / 2), kVideoRotation_0, data); frame.set_timestamp(data); - frame.set_render_time_ms(data); + // Use data as a ms timestamp. + frame.set_timestamp_us(data * rtc::kNumMicrosecsPerMillisec); return frame; } diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index a7eecd7fd2..01e64feebd 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -456,8 +456,11 @@ void ViEEncoder::OnFrame(const VideoFrame& video_frame) { VideoFrame incoming_frame = video_frame; // Local time in webrtc time base. - int64_t current_time = clock_->TimeInMilliseconds(); - incoming_frame.set_render_time_ms(current_time); + int64_t current_time_us = clock_->TimeInMicroseconds(); + int64_t current_time_ms = current_time_us / rtc::kNumMicrosecsPerMillisec; + // TODO(nisse): This always overrides the incoming timestamp. Don't + // do that, trust the frame source. + incoming_frame.set_timestamp_us(current_time_us); // Capture time may come from clock with an offset and drift from clock_. int64_t capture_ntp_time_ms; @@ -466,7 +469,7 @@ void ViEEncoder::OnFrame(const VideoFrame& video_frame) { } else if (video_frame.render_time_ms() != 0) { capture_ntp_time_ms = video_frame.render_time_ms() + delta_ntp_internal_ms_; } else { - capture_ntp_time_ms = current_time + delta_ntp_internal_ms_; + capture_ntp_time_ms = current_time_ms + delta_ntp_internal_ms_; } incoming_frame.set_ntp_time_ms(capture_ntp_time_ms); @@ -485,8 +488,8 @@ void ViEEncoder::OnFrame(const VideoFrame& video_frame) { } bool log_stats = false; - if (current_time - last_frame_log_ms_ > kFrameLogIntervalMs) { - last_frame_log_ms_ = current_time; + if (current_time_ms - last_frame_log_ms_ > kFrameLogIntervalMs) { + last_frame_log_ms_ = current_time_ms; log_stats = true; }