From 686aa37382428bc667adff8cd6895674cae4e5fd Mon Sep 17 00:00:00 2001 From: tommi Date: Mon, 27 Feb 2017 05:10:37 -0800 Subject: [PATCH] Revert of Use TaskQueue in IncomingVideoStream (patchset #10 id:320001 of https://codereview.webrtc.org/2716473002/ ) Reason for revert: Reverting while fixing build issue in Chromium. Original issue's description: > Use TaskQueue in IncomingVideoStream instead of the PlatformThread + event timer approach. > > BUG=webrtc:7219, webrtc:7253 > > Review-Url: https://codereview.webrtc.org/2716473002 > Cr-Commit-Position: refs/heads/master@{#16860} > Committed: https://chromium.googlesource.com/external/webrtc/+/e2d1d6429557af4560a97581a5e282b54c742173 TBR=mflodman@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:7219, webrtc:7253 Review-Url: https://codereview.webrtc.org/2714393003 Cr-Commit-Position: refs/heads/master@{#16863} --- webrtc/base/task_queue_unittest.cc | 9 -- .../include/incoming_video_stream.h | 27 +++-- webrtc/common_video/incoming_video_stream.cc | 111 +++++++++++------- webrtc/common_video/video_render_frames.cc | 32 +---- webrtc/common_video/video_render_frames.h | 13 +- 5 files changed, 99 insertions(+), 93 deletions(-) diff --git a/webrtc/base/task_queue_unittest.cc b/webrtc/base/task_queue_unittest.cc index c5faf8f5dc..4389d35f59 100644 --- a/webrtc/base/task_queue_unittest.cc +++ b/webrtc/base/task_queue_unittest.cc @@ -80,15 +80,6 @@ TEST(TaskQueueTest, PostLambda) { EXPECT_TRUE(event.Wait(1000)); } -TEST(TaskQueueTest, PostDelayedZero) { - static const char kQueueName[] = "PostDelayedZero"; - Event event(false, false); - TaskQueue queue(kQueueName); - - queue.PostDelayedTask([&event]() { event.Set(); }, 0); - EXPECT_TRUE(event.Wait(1000)); -} - TEST(TaskQueueTest, PostFromQueue) { static const char kQueueName[] = "PostFromQueue"; Event event(false, false); diff --git a/webrtc/common_video/include/incoming_video_stream.h b/webrtc/common_video/include/incoming_video_stream.h index ff407c56d8..2ea80eace7 100644 --- a/webrtc/common_video/include/incoming_video_stream.h +++ b/webrtc/common_video/include/incoming_video_stream.h @@ -11,12 +11,18 @@ #ifndef WEBRTC_COMMON_VIDEO_INCLUDE_INCOMING_VIDEO_STREAM_H_ #define WEBRTC_COMMON_VIDEO_INCLUDE_INCOMING_VIDEO_STREAM_H_ +#include + +#include "webrtc/base/criticalsection.h" +#include "webrtc/base/platform_thread.h" #include "webrtc/base/race_checker.h" -#include "webrtc/base/task_queue.h" +#include "webrtc/base/thread_annotations.h" +#include "webrtc/base/thread_checker.h" #include "webrtc/common_video/video_render_frames.h" #include "webrtc/media/base/videosinkinterface.h" namespace webrtc { +class EventTimerWrapper; class IncomingVideoStream : public rtc::VideoSinkInterface { public: @@ -24,19 +30,24 @@ class IncomingVideoStream : public rtc::VideoSinkInterface { rtc::VideoSinkInterface* callback); ~IncomingVideoStream() override; + protected: + static void IncomingVideoStreamThreadFun(void* obj); + void IncomingVideoStreamProcess(); + private: void OnFrame(const VideoFrame& video_frame) override; - void Dequeue(); - - // Fwd decl of a QueuedTask implementation for carrying frames over to the TQ. - class NewFrameTask; rtc::ThreadChecker main_thread_checker_; + rtc::ThreadChecker render_thread_checker_; rtc::RaceChecker decoder_race_checker_; - VideoRenderFrames render_buffers_; // Only touched on the TaskQueue. - rtc::VideoSinkInterface* const callback_; - rtc::TaskQueue incoming_render_queue_; + rtc::CriticalSection buffer_critsect_; + rtc::PlatformThread incoming_render_thread_; + std::unique_ptr deliver_buffer_event_; + + rtc::VideoSinkInterface* const external_callback_; + std::unique_ptr render_buffers_ + GUARDED_BY(buffer_critsect_); }; } // namespace webrtc diff --git a/webrtc/common_video/incoming_video_stream.cc b/webrtc/common_video/incoming_video_stream.cc index b88892d967..c1f61d1cfb 100644 --- a/webrtc/common_video/incoming_video_stream.cc +++ b/webrtc/common_video/incoming_video_stream.cc @@ -10,8 +10,6 @@ #include "webrtc/common_video/include/incoming_video_stream.h" -#include - #include "webrtc/base/timeutils.h" #include "webrtc/common_video/video_render_frames.h" #include "webrtc/system_wrappers/include/critical_section_wrapper.h" @@ -19,62 +17,87 @@ namespace webrtc { namespace { -const char kIncomingQueueName[] = "IncomingVideoStream"; -} - -// Capture by moving (std::move) into a lambda isn't possible in C++11 -// (supported in C++14). This class provides the functionality of what would be -// something like (inside OnFrame): -// VideoFrame frame(video_frame); -// incoming_render_queue_.PostTask([this, frame = std::move(frame)](){ -// if (render_buffers_.AddFrame(std::move(frame)) == 1) -// Dequeue(); -// }); -class IncomingVideoStream::NewFrameTask : public rtc::QueuedTask { - public: - NewFrameTask(IncomingVideoStream* stream, VideoFrame frame) - : stream_(stream), frame_(std::move(frame)) {} - - private: - bool Run() override { - RTC_DCHECK(rtc::TaskQueue::IsCurrent(kIncomingQueueName)); - if (stream_->render_buffers_.AddFrame(std::move(frame_)) == 1) - stream_->Dequeue(); - return true; - } - - IncomingVideoStream* stream_; - VideoFrame frame_; -}; +const int kEventStartupTimeMs = 10; +const int kEventMaxWaitTimeMs = 100; +} // namespace IncomingVideoStream::IncomingVideoStream( int32_t delay_ms, rtc::VideoSinkInterface* callback) - : render_buffers_(delay_ms), - callback_(callback), - incoming_render_queue_(kIncomingQueueName, - rtc::TaskQueue::Priority::HIGH) {} + : incoming_render_thread_(&IncomingVideoStreamThreadFun, + this, + "IncomingVideoStreamThread", + rtc::kRealtimePriority), + deliver_buffer_event_(EventTimerWrapper::Create()), + external_callback_(callback), + render_buffers_(new VideoRenderFrames(delay_ms)) { + RTC_DCHECK(external_callback_); + + render_thread_checker_.DetachFromThread(); + + deliver_buffer_event_->StartTimer(false, kEventStartupTimeMs); + incoming_render_thread_.Start(); +} IncomingVideoStream::~IncomingVideoStream() { RTC_DCHECK(main_thread_checker_.CalledOnValidThread()); + + { + rtc::CritScope cs(&buffer_critsect_); + render_buffers_.reset(); + } + + deliver_buffer_event_->Set(); + incoming_render_thread_.Stop(); + deliver_buffer_event_->StopTimer(); } void IncomingVideoStream::OnFrame(const VideoFrame& video_frame) { RTC_CHECK_RUNS_SERIALIZED(&decoder_race_checker_); - RTC_DCHECK(!incoming_render_queue_.IsCurrent()); - incoming_render_queue_.PostTask( - std::unique_ptr(new NewFrameTask(this, video_frame))); + // Hand over or insert frame. + rtc::CritScope csB(&buffer_critsect_); + if (render_buffers_->AddFrame(video_frame) == 1) { + deliver_buffer_event_->Set(); + } } -void IncomingVideoStream::Dequeue() { - RTC_DCHECK(incoming_render_queue_.IsCurrent()); - rtc::Optional frame_to_render = render_buffers_.FrameToRender(); - if (frame_to_render) - callback_->OnFrame(*frame_to_render); +// static +void IncomingVideoStream::IncomingVideoStreamThreadFun(void* obj) { + static_cast(obj)->IncomingVideoStreamProcess(); +} - if (render_buffers_.HasPendingFrames()) { - uint32_t wait_time = render_buffers_.TimeToNextFrameRelease(); - incoming_render_queue_.PostDelayedTask([this]() { Dequeue(); }, wait_time); +void IncomingVideoStream::IncomingVideoStreamProcess() { + RTC_DCHECK_RUN_ON(&render_thread_checker_); + + while (true) { + if (kEventError != deliver_buffer_event_->Wait(kEventMaxWaitTimeMs)) { + // Get a new frame to render and the time for the frame after this one. + rtc::Optional frame_to_render; + uint32_t wait_time; + { + rtc::CritScope cs(&buffer_critsect_); + if (!render_buffers_.get()) { + // Terminating + return; + } + + frame_to_render = render_buffers_->FrameToRender(); + wait_time = render_buffers_->TimeToNextFrameRelease(); + } + + // Set timer for next frame to render. + if (wait_time > kEventMaxWaitTimeMs) { + wait_time = kEventMaxWaitTimeMs; + } + + deliver_buffer_event_->StartTimer(false, wait_time); + + if (frame_to_render) { + external_callback_->OnFrame(*frame_to_render); + } + } else { + RTC_NOTREACHED(); + } } } diff --git a/webrtc/common_video/video_render_frames.cc b/webrtc/common_video/video_render_frames.cc index 444347d205..c6b109c2e9 100644 --- a/webrtc/common_video/video_render_frames.cc +++ b/webrtc/common_video/video_render_frames.cc @@ -10,8 +10,6 @@ #include "webrtc/common_video/video_render_frames.h" -#include - #include "webrtc/base/logging.h" #include "webrtc/base/timeutils.h" #include "webrtc/modules/include/module_common_types.h" @@ -19,10 +17,6 @@ namespace webrtc { namespace { -// Don't render frames with timestamp older than 500ms from now. -const int kOldRenderTimestampMS = 500; -// Don't render frames with timestamp more than 10s into the future. -const int kFutureRenderTimestampMS = 10000; const uint32_t kEventMaxWaitTimeMs = 200; const uint32_t kMinRenderDelayMs = 10; @@ -39,13 +33,13 @@ uint32_t EnsureValidRenderDelay(uint32_t render_delay) { VideoRenderFrames::VideoRenderFrames(uint32_t render_delay_ms) : render_delay_ms_(EnsureValidRenderDelay(render_delay_ms)) {} -int32_t VideoRenderFrames::AddFrame(VideoFrame&& new_frame) { +int32_t VideoRenderFrames::AddFrame(const VideoFrame& new_frame) { const int64_t time_now = rtc::TimeMillis(); // Drop old frames only when there are other frames in the queue, otherwise, a // really slow system never renders any frames. if (!incoming_frames_.empty() && - new_frame.render_time_ms() + kOldRenderTimestampMS < time_now) { + new_frame.render_time_ms() + KOldRenderTimestampMS < time_now) { WEBRTC_TRACE(kTraceWarning, kTraceVideoRenderer, -1, @@ -55,25 +49,14 @@ int32_t VideoRenderFrames::AddFrame(VideoFrame&& new_frame) { return -1; } - if (new_frame.render_time_ms() > time_now + kFutureRenderTimestampMS) { + if (new_frame.render_time_ms() > time_now + KFutureRenderTimestampMS) { WEBRTC_TRACE(kTraceWarning, kTraceVideoRenderer, -1, "%s: frame too long into the future, timestamp=%u.", __FUNCTION__, new_frame.timestamp()); return -1; } - if (new_frame.render_time_ms() < last_render_time_ms_) { - WEBRTC_TRACE(kTraceWarning, kTraceVideoRenderer, -1, - "%s: frame scheduled out of order, render_time=%u, latest=%u.", - __FUNCTION__, new_frame.render_time_ms(), - last_render_time_ms_); - // TODO(mflodman): Decide what to do when this happens. - // See bug: https://bugs.chromium.org/p/webrtc/issues/detail?id=7253 - } - - last_render_time_ms_ = new_frame.render_time_ms(); - incoming_frames_.emplace_back(std::move(new_frame)); - + incoming_frames_.push_back(new_frame); if (incoming_frames_.size() > kMaxIncomingFramesBeforeLogged) LOG(LS_WARNING) << "Stored incoming frames: " << incoming_frames_.size(); return static_cast(incoming_frames_.size()); @@ -83,8 +66,7 @@ rtc::Optional VideoRenderFrames::FrameToRender() { rtc::Optional render_frame; // Get the newest frame that can be released for rendering. while (!incoming_frames_.empty() && TimeToNextFrameRelease() <= 0) { - render_frame = - rtc::Optional(std::move(incoming_frames_.front())); + render_frame = rtc::Optional(incoming_frames_.front()); incoming_frames_.pop_front(); } return render_frame; @@ -100,8 +82,4 @@ uint32_t VideoRenderFrames::TimeToNextFrameRelease() { return time_to_release < 0 ? 0u : static_cast(time_to_release); } -bool VideoRenderFrames::HasPendingFrames() const { - return !incoming_frames_.empty(); -} - } // namespace webrtc diff --git a/webrtc/common_video/video_render_frames.h b/webrtc/common_video/video_render_frames.h index 5ed076069c..38bc0e9e19 100644 --- a/webrtc/common_video/video_render_frames.h +++ b/webrtc/common_video/video_render_frames.h @@ -27,7 +27,7 @@ class VideoRenderFrames { VideoRenderFrames(const VideoRenderFrames&) = delete; // Add a frame to the render queue - int32_t AddFrame(VideoFrame&& new_frame); + int32_t AddFrame(const VideoFrame& new_frame); // Get a frame for rendering, or false if it's not time to render. rtc::Optional FrameToRender(); @@ -35,16 +35,19 @@ class VideoRenderFrames { // Returns the number of ms to next frame to render uint32_t TimeToNextFrameRelease(); - bool HasPendingFrames() const; - private: + // 10 seconds for 30 fps. + enum { KMaxNumberOfFrames = 300 }; + // Don't render frames with timestamp older than 500ms from now. + enum { KOldRenderTimestampMS = 500 }; + // Don't render frames with timestamp more than 10s into the future. + enum { KFutureRenderTimestampMS = 10000 }; + // Sorted list with framed to be rendered, oldest first. std::list incoming_frames_; // Estimated delay from a frame is released until it's rendered. const uint32_t render_delay_ms_; - - int64_t last_render_time_ms_ = 0; }; } // namespace webrtc