From d9c2d9462055e025406c63701bc5c65286a8903c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Tue, 30 Apr 2019 09:16:36 +0200 Subject: [PATCH] Move ownership of VCMJitterEstimator to FrameBuffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:7408 Change-Id: I8b33ead80abff1e84ae0b223e108266f71f03e2f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/134180 Reviewed-by: Philip Eliasson Reviewed-by: Åsa Persson Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#27823} --- modules/video_coding/frame_buffer2.cc | 13 ++++++------- modules/video_coding/frame_buffer2.h | 4 ++-- modules/video_coding/frame_buffer2_unittest.cc | 6 +----- test/fuzzers/frame_buffer2_fuzzer.cc | 5 +---- video/video_receive_stream.cc | 6 ++---- video/video_receive_stream.h | 2 -- video/video_stream_decoder_impl.cc | 2 -- video/video_stream_decoder_impl.h | 2 -- 8 files changed, 12 insertions(+), 28 deletions(-) diff --git a/modules/video_coding/frame_buffer2.cc b/modules/video_coding/frame_buffer2.cc index a327c1bf23..9b2b9079fa 100644 --- a/modules/video_coding/frame_buffer2.cc +++ b/modules/video_coding/frame_buffer2.cc @@ -49,13 +49,12 @@ constexpr int64_t kLogNonDecodedIntervalMs = 5000; } // namespace FrameBuffer::FrameBuffer(Clock* clock, - VCMJitterEstimator* jitter_estimator, VCMTiming* timing, VCMReceiveStatisticsCallback* stats_callback) : decoded_frames_history_(kMaxFramesHistory), clock_(clock), callback_queue_(nullptr), - jitter_estimator_(jitter_estimator), + jitter_estimator_(clock), timing_(timing), inter_frame_delay_(clock_->TimeInMilliseconds()), stopped_(false), @@ -266,7 +265,7 @@ EncodedFrame* FrameBuffer::GetNextFrame() { int64_t receive_time_ms = first_frame->ReceivedTime(); // Gracefully handle bad RTP timestamps and render time issues. if (HasBadRenderTiming(*first_frame, now_ms)) { - jitter_estimator_->Reset(); + jitter_estimator_.Reset(); timing_->Reset(); render_time_ms = timing_->RenderTimeMs(first_frame->Timestamp(), now_ms); } @@ -295,7 +294,7 @@ EncodedFrame* FrameBuffer::GetNextFrame() { if (inter_frame_delay_.CalculateDelay(first_frame->Timestamp(), &frame_delay, receive_time_ms)) { - jitter_estimator_->UpdateEstimate(frame_delay, superframe_size); + jitter_estimator_.UpdateEstimate(frame_delay, superframe_size); } float rtt_mult = protection_mode_ == kProtectionNackFEC ? 0.0 : 1.0; @@ -306,11 +305,11 @@ EncodedFrame* FrameBuffer::GetNextFrame() { jitter_est_cap_ms = 300.0; } timing_->SetJitterDelay( - jitter_estimator_->GetJitterEstimate(rtt_mult, jitter_est_cap_ms)); + jitter_estimator_.GetJitterEstimate(rtt_mult, jitter_est_cap_ms)); timing_->UpdateCurrentDelay(render_time_ms, now_ms); } else { if (RttMultExperiment::RttMultEnabled() || add_rtt_to_playout_delay_) - jitter_estimator_->FrameNacked(); + jitter_estimator_.FrameNacked(); } UpdateJitterDelay(); @@ -378,7 +377,7 @@ void FrameBuffer::Clear() { void FrameBuffer::UpdateRtt(int64_t rtt_ms) { rtc::CritScope lock(&crit_); - jitter_estimator_->UpdateRtt(rtt_ms); + jitter_estimator_.UpdateRtt(rtt_ms); } bool FrameBuffer::ValidReferences(const EncodedFrame& frame) const { diff --git a/modules/video_coding/frame_buffer2.h b/modules/video_coding/frame_buffer2.h index 57dbaa40e0..cffdf3e5cc 100644 --- a/modules/video_coding/frame_buffer2.h +++ b/modules/video_coding/frame_buffer2.h @@ -21,6 +21,7 @@ #include "api/video/encoded_frame.h" #include "modules/video_coding/include/video_coding_defines.h" #include "modules/video_coding/inter_frame_delay.h" +#include "modules/video_coding/jitter_estimator.h" #include "modules/video_coding/utility/decoded_frames_history.h" #include "rtc_base/constructor_magic.h" #include "rtc_base/critical_section.h" @@ -45,7 +46,6 @@ class FrameBuffer { enum ReturnReason { kFrameFound, kTimeout, kStopped }; FrameBuffer(Clock* clock, - VCMJitterEstimator* jitter_estimator, VCMTiming* timing, VCMReceiveStatisticsCallback* stats_callback); @@ -182,7 +182,7 @@ class FrameBuffer { bool keyframe_required_ RTC_GUARDED_BY(crit_); rtc::Event new_continuous_frame_event_; - VCMJitterEstimator* const jitter_estimator_ RTC_GUARDED_BY(crit_); + VCMJitterEstimator jitter_estimator_ RTC_GUARDED_BY(crit_); VCMTiming* const timing_ RTC_GUARDED_BY(crit_); VCMInterFrameDelay inter_frame_delay_ RTC_GUARDED_BY(crit_); absl::optional last_continuous_frame_ diff --git a/modules/video_coding/frame_buffer2_unittest.cc b/modules/video_coding/frame_buffer2_unittest.cc index b984e4fd93..d36707b48c 100644 --- a/modules/video_coding/frame_buffer2_unittest.cc +++ b/modules/video_coding/frame_buffer2_unittest.cc @@ -138,9 +138,7 @@ class TestFrameBuffer2 : public ::testing::Test { : trial_("WebRTC-AddRttToPlayoutDelay/Enabled/"), clock_(0), timing_(&clock_), - jitter_estimator_(&clock_), buffer_(new FrameBuffer(&clock_, - &jitter_estimator_, &timing_, &stats_callback_)), rand_(0x34678213), @@ -266,7 +264,6 @@ class TestFrameBuffer2 : public ::testing::Test { test::ScopedFieldTrials trial_; SimulatedClock clock_; VCMTimingFake timing_; - VCMJitterEstimator jitter_estimator_; std::unique_ptr buffer_; std::vector> frames_; Random rand_; @@ -306,8 +303,7 @@ TEST_F(TestFrameBuffer2, OneSuperFrame) { TEST_F(TestFrameBuffer2, ZeroPlayoutDelay) { VCMTiming timing(&clock_); - buffer_.reset( - new FrameBuffer(&clock_, &jitter_estimator_, &timing, &stats_callback_)); + buffer_.reset(new FrameBuffer(&clock_, &timing, &stats_callback_)); const PlayoutDelay kPlayoutDelayMs = {0, 0}; std::unique_ptr test_frame(new FrameObjectFake()); test_frame->id.picture_id = 0; diff --git a/test/fuzzers/frame_buffer2_fuzzer.cc b/test/fuzzers/frame_buffer2_fuzzer.cc index 579a2d7b5b..54d17131b3 100644 --- a/test/fuzzers/frame_buffer2_fuzzer.cc +++ b/test/fuzzers/frame_buffer2_fuzzer.cc @@ -10,7 +10,6 @@ #include "modules/video_coding/frame_buffer2.h" -#include "modules/video_coding/jitter_estimator.h" #include "modules/video_coding/timing.h" #include "system_wrappers/include/clock.h" @@ -67,10 +66,8 @@ void FuzzOneInput(const uint8_t* data, size_t size) { } DataReader reader(data, size); Clock* clock = Clock::GetRealTimeClock(); - VCMJitterEstimator jitter_estimator(clock); VCMTiming timing(clock); - video_coding::FrameBuffer frame_buffer(clock, &jitter_estimator, &timing, - nullptr); + video_coding::FrameBuffer frame_buffer(clock, &timing, nullptr); while (reader.MoreToRead()) { if (reader.GetNum() & 1) { diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index d885b0bcb4..3a73ba878f 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -35,7 +35,6 @@ #include "modules/video_coding/include/video_codec_interface.h" #include "modules/video_coding/include/video_coding_defines.h" #include "modules/video_coding/include/video_error_codes.h" -#include "modules/video_coding/jitter_estimator.h" #include "modules/video_coding/timing.h" #include "modules/video_coding/utility/vp8_header_parser.h" #include "rtc_base/checks.h" @@ -241,9 +240,8 @@ VideoReceiveStream::VideoReceiveStream( timing_->set_render_delay(config_.render_delay_ms); - jitter_estimator_.reset(new VCMJitterEstimator(clock_)); - frame_buffer_.reset(new video_coding::FrameBuffer( - clock_, jitter_estimator_.get(), timing_.get(), &stats_proxy_)); + frame_buffer_.reset( + new video_coding::FrameBuffer(clock_, timing_.get(), &stats_proxy_)); process_thread_->RegisterModule(&rtp_stream_sync_, RTC_FROM_HERE); diff --git a/video/video_receive_stream.h b/video/video_receive_stream.h index dc4e4b7e73..6cff06d96a 100644 --- a/video/video_receive_stream.h +++ b/video/video_receive_stream.h @@ -40,7 +40,6 @@ class RtpStreamReceiverInterface; class RtpStreamReceiverControllerInterface; class RtxReceiveStream; class VCMTiming; -class VCMJitterEstimator; namespace internal { @@ -181,7 +180,6 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, std::vector> video_decoders_; // Members for the new jitter buffer experiment. - std::unique_ptr jitter_estimator_; std::unique_ptr frame_buffer_; std::unique_ptr media_receiver_; diff --git a/video/video_stream_decoder_impl.cc b/video/video_stream_decoder_impl.cc index 42a543451c..edc87b7822 100644 --- a/video/video_stream_decoder_impl.cc +++ b/video/video_stream_decoder_impl.cc @@ -33,10 +33,8 @@ VideoStreamDecoderImpl::VideoStreamDecoderImpl( this, "video_stream_decoder_decode_thread", rtc::kHighestPriority), - jitter_estimator_(Clock::GetRealTimeClock()), timing_(Clock::GetRealTimeClock()), frame_buffer_(Clock::GetRealTimeClock(), - &jitter_estimator_, &timing_, nullptr), next_frame_timestamps_index_(0) { diff --git a/video/video_stream_decoder_impl.h b/video/video_stream_decoder_impl.h index 5578e70547..b7a5e41d72 100644 --- a/video/video_stream_decoder_impl.h +++ b/video/video_stream_decoder_impl.h @@ -18,7 +18,6 @@ #include "absl/types/optional.h" #include "api/video/video_stream_decoder.h" #include "modules/video_coding/frame_buffer2.h" -#include "modules/video_coding/jitter_estimator.h" #include "modules/video_coding/timing.h" #include "rtc_base/platform_thread.h" #include "rtc_base/task_queue.h" @@ -80,7 +79,6 @@ class VideoStreamDecoderImpl : public VideoStreamDecoderInterface, rtc::TaskQueue bookkeeping_queue_; rtc::PlatformThread decode_thread_; - VCMJitterEstimator jitter_estimator_; VCMTiming timing_; video_coding::FrameBuffer frame_buffer_; video_coding::VideoLayerFrameId last_continuous_id_;