From ccb05f17b142c4aff743c91bc589abbb34f83467 Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Fri, 3 Jun 2022 13:09:51 +0000 Subject: [PATCH] Revert "Remove FrameBuffer2Proxy" This reverts commit d5ddad4cdb7348292cf9deedca16b8f818f3a811. Reason for revert: Impacts perf tests at low BW Original change's description: > Remove FrameBuffer2Proxy > > FrameBuffer3Proxy and sync decoding has been shown to work. First step of cleaning up is to remove the FrameBuffer2Proxy. > > Change-Id: Ic96303c2d4f9111cfeed9927e8826ea7ffe7ee17 > Bug: webrtc:14003 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264126 > Reviewed-by: Ilya Nikolaevskiy > Commit-Queue: Evan Shrubsole > Cr-Commit-Position: refs/heads/main@{#37086} Bug: webrtc:14003 Change-Id: Ia858f4d8d0f9c90bed91a17b2bcfb477d1919b26 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265020 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Evan Shrubsole Auto-Submit: Evan Shrubsole Commit-Queue: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/main@{#37119} --- video/frame_buffer_proxy.cc | 110 ++++++++++++++++++++++++++- video/frame_buffer_proxy_unittest.cc | 5 +- 2 files changed, 112 insertions(+), 3 deletions(-) diff --git a/video/frame_buffer_proxy.cc b/video/frame_buffer_proxy.cc index 87370c3b4e..85ad274f92 100644 --- a/video/frame_buffer_proxy.cc +++ b/video/frame_buffer_proxy.cc @@ -36,6 +36,104 @@ namespace webrtc { namespace { +class FrameBuffer2Proxy : public FrameBufferProxy { + public: + FrameBuffer2Proxy(Clock* clock, + VCMTiming* timing, + VCMReceiveStatisticsCallback* stats_proxy, + rtc::TaskQueue* decode_queue, + FrameSchedulingReceiver* receiver, + TimeDelta max_wait_for_keyframe, + TimeDelta max_wait_for_frame, + const FieldTrialsView& field_trials) + : max_wait_for_keyframe_(max_wait_for_keyframe), + max_wait_for_frame_(max_wait_for_frame), + frame_buffer_(clock, timing, stats_proxy, field_trials), + decode_queue_(decode_queue), + stats_proxy_(stats_proxy), + receiver_(receiver) { + RTC_DCHECK(decode_queue_); + RTC_DCHECK(stats_proxy_); + RTC_DCHECK(receiver_); + } + + void StopOnWorker() override { + RTC_DCHECK_RUN_ON(&worker_sequence_checker_); + decode_queue_->PostTask([this] { + frame_buffer_.Stop(); + decode_safety_->SetNotAlive(); + }); + } + + void SetProtectionMode(VCMVideoProtection protection_mode) override { + RTC_DCHECK_RUN_ON(&worker_sequence_checker_); + frame_buffer_.SetProtectionMode(kProtectionNackFEC); + } + + void Clear() override { + RTC_DCHECK_RUN_ON(&worker_sequence_checker_); + frame_buffer_.Clear(); + } + + absl::optional InsertFrame( + std::unique_ptr frame) override { + RTC_DCHECK_RUN_ON(&worker_sequence_checker_); + int64_t last_continuous_pid = frame_buffer_.InsertFrame(std::move(frame)); + if (last_continuous_pid != -1) + return last_continuous_pid; + return absl::nullopt; + } + + void UpdateRtt(int64_t max_rtt_ms) override { + RTC_DCHECK_RUN_ON(&worker_sequence_checker_); + frame_buffer_.UpdateRtt(max_rtt_ms); + } + + void StartNextDecode(bool keyframe_required) override { + if (!decode_queue_->IsCurrent()) { + decode_queue_->PostTask(ToQueuedTask( + decode_safety_, + [this, keyframe_required] { StartNextDecode(keyframe_required); })); + return; + } + RTC_DCHECK_RUN_ON(decode_queue_); + + frame_buffer_.NextFrame( + MaxWait(keyframe_required).ms(), keyframe_required, decode_queue_, + /* encoded frame handler */ + [this, keyframe_required](std::unique_ptr frame) { + RTC_DCHECK_RUN_ON(decode_queue_); + if (!decode_safety_->alive()) + return; + if (frame) { + receiver_->OnEncodedFrame(std::move(frame)); + } else { + receiver_->OnDecodableFrameTimeout(MaxWait(keyframe_required)); + } + }); + } + + int Size() override { + RTC_DCHECK_RUN_ON(&worker_sequence_checker_); + return frame_buffer_.Size(); + } + + private: + TimeDelta MaxWait(bool keyframe_required) const { + return keyframe_required ? max_wait_for_keyframe_ : max_wait_for_frame_; + } + + RTC_NO_UNIQUE_ADDRESS SequenceChecker worker_sequence_checker_; + const TimeDelta max_wait_for_keyframe_; + const TimeDelta max_wait_for_frame_; + video_coding::FrameBuffer frame_buffer_; + rtc::TaskQueue* const decode_queue_; + VCMReceiveStatisticsCallback* const stats_proxy_; + FrameSchedulingReceiver* const receiver_; + rtc::scoped_refptr decode_safety_ = + PendingTaskSafetyFlag::CreateDetached(); +}; + // Max number of frames the buffer will hold. static constexpr size_t kMaxFramesBuffered = 800; // Max number of decoded frame info that will be saved. @@ -71,7 +169,9 @@ Timestamp ReceiveTime(const EncodedFrame& frame) { } // Encapsulates use of the new frame buffer for use in -// VideoReceiveStreamInterface. +// VideoReceiveStreamInterface. This behaves the same as the FrameBuffer2Proxy +// but uses frame_buffer instead. Responsibilities from frame_buffer2, like +// stats, jitter and frame timing accounting are moved into this pro class FrameBuffer3Proxy : public FrameBufferProxy { public: FrameBuffer3Proxy( @@ -434,6 +534,7 @@ class FrameBuffer3Proxy : public FrameBufferProxy { }; enum class FrameBufferArm { + kFrameBuffer2, kFrameBuffer3, kSyncDecode, }; @@ -442,8 +543,9 @@ constexpr const char* kFrameBufferFieldTrial = "WebRTC-FrameBuffer3"; FrameBufferArm ParseFrameBufferFieldTrial(const FieldTrialsView& field_trials) { webrtc::FieldTrialEnum arm( - "arm", FrameBufferArm::kFrameBuffer3, + "arm", FrameBufferArm::kFrameBuffer2, { + {"FrameBuffer2", FrameBufferArm::kFrameBuffer2}, {"FrameBuffer3", FrameBufferArm::kFrameBuffer3}, {"SyncDecoding", FrameBufferArm::kSyncDecode}, }); @@ -465,6 +567,10 @@ std::unique_ptr FrameBufferProxy::CreateFromFieldTrial( DecodeSynchronizer* decode_sync, const FieldTrialsView& field_trials) { switch (ParseFrameBufferFieldTrial(field_trials)) { + case FrameBufferArm::kFrameBuffer2: + return std::make_unique( + clock, timing, stats_proxy, decode_queue, receiver, + max_wait_for_keyframe, max_wait_for_frame, field_trials); case FrameBufferArm::kSyncDecode: { std::unique_ptr scheduler; if (decode_sync) { diff --git a/video/frame_buffer_proxy_unittest.cc b/video/frame_buffer_proxy_unittest.cc index 80712e3c6a..6babcdada9 100644 --- a/video/frame_buffer_proxy_unittest.cc +++ b/video/frame_buffer_proxy_unittest.cc @@ -752,7 +752,8 @@ TEST_P(FrameBufferProxyTest, NextFrameWithOldTimestamp) { INSTANTIATE_TEST_SUITE_P( FrameBufferProxy, FrameBufferProxyTest, - ::testing::Values("WebRTC-FrameBuffer3/arm:FrameBuffer3/", + ::testing::Values("WebRTC-FrameBuffer3/arm:FrameBuffer2/", + "WebRTC-FrameBuffer3/arm:FrameBuffer3/", "WebRTC-FrameBuffer3/arm:SyncDecoding/")); class LowLatencyFrameBufferProxyTest : public ::testing::Test, @@ -833,6 +834,8 @@ INSTANTIATE_TEST_SUITE_P( FrameBufferProxy, LowLatencyFrameBufferProxyTest, ::testing::Values( + "WebRTC-FrameBuffer3/arm:FrameBuffer2/" + "WebRTC-ZeroPlayoutDelay/min_pacing:16ms,max_decode_queue_size:5/", "WebRTC-FrameBuffer3/arm:FrameBuffer3/" "WebRTC-ZeroPlayoutDelay/min_pacing:16ms,max_decode_queue_size:5/", "WebRTC-FrameBuffer3/arm:SyncDecoding/"