From 36ae902913f91a6e4d3d6a1f5d16a0ab1b92c3ae Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Sun, 10 Mar 2019 23:29:24 +0100 Subject: [PATCH] Move to one JPEG decoder thread per view. This is, surprisingly, the most useful for VA-API decodes; they can have long latency at 1080p, and Futatabi's dropping scheme sometimes caused massive unfairness. Our system doesn't pipeline all that nicely, so just having multiple threads was the simplest solution. The risk is that we now access VA-API from multiple threads, which has a tendency to tickle bugs, but we'll see. Of course, for CPU decoding, you will also benefit. --- futatabi/jpeg_frame_view.cpp | 45 +++++++----------------------------- futatabi/jpeg_frame_view.h | 24 +++++++++++++++---- futatabi/main.cpp | 1 - 3 files changed, 28 insertions(+), 42 deletions(-) diff --git a/futatabi/jpeg_frame_view.cpp b/futatabi/jpeg_frame_view.cpp index c9b8090..d35964c 100644 --- a/futatabi/jpeg_frame_view.cpp +++ b/futatabi/jpeg_frame_view.cpp @@ -59,19 +59,6 @@ struct LRUFrame { size_t last_used; }; -struct PendingDecode { - JPEGFrameView *destination; - - // For actual decodes (only if frame below is nullptr). - FrameOnDisk primary, secondary; - float fade_alpha; // Irrelevant if secondary.stream_idx == -1. - - // Already-decoded frames are also sent through PendingDecode, - // so that they get drawn in the right order. If frame is nullptr, - // it's a real decode. - shared_ptr frame; -}; - // There can be multiple JPEGFrameView instances, so make all the metrics static. once_flag jpeg_metrics_inited; atomic metric_jpeg_cache_used_bytes{ 0 }; // Same value as cache_bytes_used. @@ -86,12 +73,9 @@ atomic metric_jpeg_vaapi_fail_frames{ 0 }; } // namespace -thread JPEGFrameView::jpeg_decoder_thread; mutex cache_mu; map cache; // Under cache_mu. size_t cache_bytes_used = 0; // Under cache_mu. -condition_variable any_pending_decodes; -deque pending_decodes; // Under cache_mu. atomic event_counter{ 0 }; extern QGLWidget *global_share_widget; extern atomic should_quit; @@ -277,7 +261,7 @@ void JPEGFrameView::jpeg_decoder_thread_func() CacheMissBehavior cache_miss_behavior = DECODE_IF_NOT_IN_CACHE; { unique_lock lock(cache_mu); // TODO: Perhaps under another lock? - any_pending_decodes.wait(lock, [] { + any_pending_decodes.wait(lock, [this] { return !pending_decodes.empty() || should_quit.load(); }); if (should_quit.load()) @@ -285,20 +269,14 @@ void JPEGFrameView::jpeg_decoder_thread_func() decode = pending_decodes.front(); pending_decodes.pop_front(); - size_t num_pending = 0; - for (const PendingDecode &other_decode : pending_decodes) { - if (other_decode.destination == decode.destination) { - ++num_pending; - } - } - if (num_pending > 3) { + if (pending_decodes.size() > 3) { cache_miss_behavior = RETURN_NULLPTR_IF_NOT_IN_CACHE; } } if (decode.frame != nullptr) { // Already decoded, so just show it. - decode.destination->setDecodedFrame(decode.frame, nullptr, 1.0f); + setDecodedFrame(decode.frame, nullptr, 1.0f); continue; } @@ -312,7 +290,7 @@ void JPEGFrameView::jpeg_decoder_thread_func() } bool found_in_cache; - shared_ptr frame = decode_jpeg_with_cache(frame_spec, cache_miss_behavior, &decode.destination->frame_reader, &found_in_cache); + shared_ptr frame = decode_jpeg_with_cache(frame_spec, cache_miss_behavior, &frame_reader, &found_in_cache); if (frame == nullptr) { assert(cache_miss_behavior == RETURN_NULLPTR_IF_NOT_IN_CACHE); @@ -339,11 +317,11 @@ void JPEGFrameView::jpeg_decoder_thread_func() } // TODO: Could we get jitter between non-interpolated and interpolated frames here? - decode.destination->setDecodedFrame(primary_frame, secondary_frame, decode.fade_alpha); + setDecodedFrame(primary_frame, secondary_frame, decode.fade_alpha); } } -void JPEGFrameView::shutdown() +JPEGFrameView::~JPEGFrameView() { any_pending_decodes.notify_all(); jpeg_decoder_thread.join(); @@ -374,7 +352,6 @@ void JPEGFrameView::setFrame(unsigned stream_idx, FrameOnDisk frame, FrameOnDisk decode.primary = frame; decode.secondary = secondary_frame; decode.fade_alpha = fade_alpha; - decode.destination = this; pending_decodes.push_back(decode); any_pending_decodes.notify_all(); } @@ -384,24 +361,18 @@ void JPEGFrameView::setFrame(shared_ptr frame) lock_guard lock(cache_mu); PendingDecode decode; decode.frame = std::move(frame); - decode.destination = this; pending_decodes.push_back(decode); any_pending_decodes.notify_all(); } -ResourcePool *resource_pool = nullptr; - void JPEGFrameView::initializeGL() { glDisable(GL_BLEND); glDisable(GL_DEPTH_TEST); check_error(); - static once_flag once; - call_once(once, [] { - resource_pool = new ResourcePool; - jpeg_decoder_thread = std::thread(jpeg_decoder_thread_func); - }); + resource_pool = new ResourcePool; + jpeg_decoder_thread = std::thread(&JPEGFrameView::jpeg_decoder_thread_func, this); ycbcr_converter.reset(new YCbCrConverter(YCbCrConverter::OUTPUT_TO_RGBA, resource_pool)); diff --git a/futatabi/jpeg_frame_view.h b/futatabi/jpeg_frame_view.h index 9af7d2c..b66e265 100644 --- a/futatabi/jpeg_frame_view.h +++ b/futatabi/jpeg_frame_view.h @@ -13,6 +13,8 @@ #include #include #include +#include +#include #include enum CacheMissBehavior { @@ -29,6 +31,7 @@ class JPEGFrameView : public QGLWidget { public: JPEGFrameView(QWidget *parent); + ~JPEGFrameView(); void setFrame(unsigned stream_idx, FrameOnDisk frame, FrameOnDisk secondary_frame = {}, float fade_alpha = 0.0f); void setFrame(std::shared_ptr frame); @@ -40,8 +43,6 @@ public: void setDecodedFrame(std::shared_ptr frame, std::shared_ptr secondary_frame, float fade_alpha); void set_overlay(const std::string &text); // Blank for none. - static void shutdown(); - signals: void clicked(); @@ -51,7 +52,7 @@ protected: void paintGL() override; private: - static void jpeg_decoder_thread_func(); + void jpeg_decoder_thread_func(); FrameReader frame_reader; @@ -73,7 +74,22 @@ private: int gl_width, gl_height; - static std::thread jpeg_decoder_thread; + std::thread jpeg_decoder_thread; + movit::ResourcePool *resource_pool = nullptr; + + struct PendingDecode { + // For actual decodes (only if frame below is nullptr). + FrameOnDisk primary, secondary; + float fade_alpha; // Irrelevant if secondary.stream_idx == -1. + + // Already-decoded frames are also sent through PendingDecode, + // so that they get drawn in the right order. If frame is nullptr, + // it's a real decode. + std::shared_ptr frame; + }; + + std::condition_variable any_pending_decodes; + std::deque pending_decodes; // Under cache_mu. }; #endif // !defined(_JPEG_FRAME_VIEW_H) diff --git a/futatabi/main.cpp b/futatabi/main.cpp index 7c5f660..a3a550e 100644 --- a/futatabi/main.cpp +++ b/futatabi/main.cpp @@ -275,7 +275,6 @@ int main(int argc, char **argv) should_quit = true; record_thread.join(); - JPEGFrameView::shutdown(); return ret; } -- 2.39.2