From 812ceafb5a39ca705f01a2b926eba4b7313f4c37 Mon Sep 17 00:00:00 2001 From: Stefan Holmer Date: Tue, 15 May 2018 13:00:10 +0200 Subject: [PATCH] Ensure render time is zero when playout delay is zero so that minimal latency in the render pipeline is ensured. Bug: webrtc:9135 Change-Id: Id9ae8ec59536808ba8923c73dd46abfe3fa6fe79 Reviewed-on: https://webrtc-review.googlesource.com/75600 Commit-Queue: Stefan Holmer Reviewed-by: Philip Eliasson Cr-Commit-Position: refs/heads/master@{#23309} --- modules/video_coding/frame_buffer2.cc | 6 +++- .../video_coding/frame_buffer2_unittest.cc | 33 ++++++++++++++----- modules/video_coding/timing.cc | 12 +++---- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/modules/video_coding/frame_buffer2.cc b/modules/video_coding/frame_buffer2.cc index 9cf3652e7e..e6840f0207 100644 --- a/modules/video_coding/frame_buffer2.cc +++ b/modules/video_coding/frame_buffer2.cc @@ -219,10 +219,14 @@ bool FrameBuffer::HasBadRenderTiming(const EncodedFrame& frame, int64_t now_ms) { // Assume that render timing errors are due to changes in the video stream. int64_t render_time_ms = frame.RenderTimeMs(); - const int64_t kMaxVideoDelayMs = 10000; + // Zero render time means render immediately. + if (render_time_ms == 0) { + return false; + } if (render_time_ms < 0) { return true; } + const int64_t kMaxVideoDelayMs = 10000; if (std::abs(render_time_ms - now_ms) > kMaxVideoDelayMs) { int frame_delay = static_cast(std::abs(render_time_ms - now_ms)); RTC_LOG(LS_WARNING) diff --git a/modules/video_coding/frame_buffer2_unittest.cc b/modules/video_coding/frame_buffer2_unittest.cc index 84682fd21a..304910a9a5 100644 --- a/modules/video_coding/frame_buffer2_unittest.cc +++ b/modules/video_coding/frame_buffer2_unittest.cc @@ -133,7 +133,10 @@ class TestFrameBuffer2 : public ::testing::Test { : clock_(0), timing_(&clock_), jitter_estimator_(&clock_), - buffer_(&clock_, &jitter_estimator_, &timing_, &stats_callback_), + buffer_(new FrameBuffer(&clock_, + &jitter_estimator_, + &timing_, + &stats_callback_)), rand_(0x34678213), tear_down_(false), extract_thread_(&ExtractLoop, this, "Extract Thread"), @@ -168,7 +171,7 @@ class TestFrameBuffer2 : public ::testing::Test { for (size_t r = 0; r < references.size(); ++r) frame->references[r] = references[r]; - return buffer_.InsertFrame(std::move(frame)); + return buffer_->InsertFrame(std::move(frame)); } void ExtractFrame(int64_t max_wait_time = 0, bool keyframe_required = false) { @@ -176,7 +179,7 @@ class TestFrameBuffer2 : public ::testing::Test { if (max_wait_time == 0) { std::unique_ptr frame; FrameBuffer::ReturnReason res = - buffer_.NextFrame(0, &frame, keyframe_required); + buffer_->NextFrame(0, &frame, keyframe_required); if (res != FrameBuffer::ReturnReason::kStopped) frames_.emplace_back(std::move(frame)); crit_.Leave(); @@ -215,7 +218,7 @@ class TestFrameBuffer2 : public ::testing::Test { std::unique_ptr frame; FrameBuffer::ReturnReason res = - tfb->buffer_.NextFrame(tfb->max_wait_time_, &frame); + tfb->buffer_->NextFrame(tfb->max_wait_time_, &frame); if (res != FrameBuffer::ReturnReason::kStopped) tfb->frames_.emplace_back(std::move(frame)); } @@ -227,7 +230,7 @@ class TestFrameBuffer2 : public ::testing::Test { SimulatedClock clock_; VCMTimingFake timing_; ::testing::NiceMock jitter_estimator_; - FrameBuffer buffer_; + std::unique_ptr buffer_; std::vector> frames_; Random rand_; ::testing::NiceMock stats_callback_; @@ -271,11 +274,25 @@ TEST_F(TestFrameBuffer2, SetPlayoutDelay) { std::unique_ptr test_frame(new FrameObjectFake()); test_frame->id.picture_id = 0; test_frame->SetPlayoutDelay(kPlayoutDelayMs); - buffer_.InsertFrame(std::move(test_frame)); + buffer_->InsertFrame(std::move(test_frame)); EXPECT_EQ(kPlayoutDelayMs.min_ms, timing_.min_playout_delay()); EXPECT_EQ(kPlayoutDelayMs.max_ms, timing_.max_playout_delay()); } +TEST_F(TestFrameBuffer2, ZeroPlayoutDelay) { + VCMTiming timing(&clock_); + buffer_.reset( + new FrameBuffer(&clock_, &jitter_estimator_, &timing, &stats_callback_)); + const PlayoutDelay kPlayoutDelayMs = {0, 0}; + std::unique_ptr test_frame(new FrameObjectFake()); + test_frame->id.picture_id = 0; + test_frame->SetPlayoutDelay(kPlayoutDelayMs); + buffer_->InsertFrame(std::move(test_frame)); + ExtractFrame(0, false); + CheckFrame(0, 0, 0); + EXPECT_EQ(0, frames_[0]->RenderTimeMs()); +} + // Flaky test, see bugs.webrtc.org/7068. TEST_F(TestFrameBuffer2, DISABLED_OneUnorderedSuperFrame) { uint16_t pid = Rand(); @@ -433,7 +450,7 @@ TEST_F(TestFrameBuffer2, ProtectionMode) { InsertFrame(pid, 0, ts, false); ExtractFrame(); - buffer_.SetProtectionMode(kProtectionNackFEC); + buffer_->SetProtectionMode(kProtectionNackFEC); EXPECT_CALL(jitter_estimator_, GetJitterEstimate(0.0)); InsertFrame(pid + 1, 0, ts, false); ExtractFrame(); @@ -507,7 +524,7 @@ TEST_F(TestFrameBuffer2, StatsCallback) { frame->num_references = 0; frame->inter_layer_predicted = false; - EXPECT_EQ(buffer_.InsertFrame(std::move(frame)), pid); + EXPECT_EQ(buffer_->InsertFrame(std::move(frame)), pid); } ExtractFrame(); diff --git a/modules/video_coding/timing.cc b/modules/video_coding/timing.cc index 2ad61524e3..cf600199a4 100644 --- a/modules/video_coding/timing.cc +++ b/modules/video_coding/timing.cc @@ -204,23 +204,21 @@ void VCMTiming::IncomingTimestamp(uint32_t time_stamp, int64_t now_ms) { int64_t VCMTiming::RenderTimeMs(uint32_t frame_timestamp, int64_t now_ms) const { rtc::CritScope cs(&crit_sect_); - const int64_t render_time_ms = RenderTimeMsInternal(frame_timestamp, now_ms); - return render_time_ms; + return RenderTimeMsInternal(frame_timestamp, now_ms); } int64_t VCMTiming::RenderTimeMsInternal(uint32_t frame_timestamp, int64_t now_ms) const { + if (min_playout_delay_ms_ == 0 && max_playout_delay_ms_ == 0) { + // Render as soon as possible. + return 0; + } int64_t estimated_complete_time_ms = ts_extrapolator_->ExtrapolateLocalTime(frame_timestamp); if (estimated_complete_time_ms == -1) { estimated_complete_time_ms = now_ms; } - if (min_playout_delay_ms_ == 0 && max_playout_delay_ms_ == 0) { - // Render as soon as possible. - return now_ms; - } - // Make sure the actual delay stays in the range of |min_playout_delay_ms_| // and |max_playout_delay_ms_|. int actual_delay = std::max(current_delay_ms_, min_playout_delay_ms_);