video/widget: fix mutex starvation

Get rid of contention in `preview_size' and `set_image'.

After switching the Qt mutex to non-recursive, turns out the writer
thread preempts the UI thread to the point of freezing the entire thing.
Mutex fairness is an implementation detail and we must assume unfair
mutexes in the worst case.

5 files changed, 72 insertions, 66 deletions

diff --git a/cv/video-widget.cpp b/cv/video-widget.cpp
index 7240814f..8ff252c9 100644
--- a/cv/video-widget.cpp
+++ b/cv/video-widget.cpp
@@ -4,15 +4,15 @@
 
 void cv_video_widget::update_image(const cv::Mat& frame)
 {
-    QMutexLocker l(&mtx);
-
-    if (freshp)
+    if (fresh())
         return;
 
+    auto [ W, H ] = preview_size();
+
     if (W < 1 || H < 1 || frame.rows < 1 || frame.cols < 1)
         return;
 
-    cv::Mat const* __restrict frame_scaled = nullptr;
+    cv::Mat const* __restrict scaled = nullptr;
 
     if (frame3.cols != W || frame3.rows != H)
     {
@@ -26,22 +26,20 @@ void cv_video_widget::update_image(const cv::Mat& frame)
     if (frame.cols != W || frame.rows != H)
     {
         cv::resize(frame, frame3, { W, H }, 0, 0, cv::INTER_NEAREST);
-        frame_scaled = &frame3;
+        scaled = &frame3;
     }
     else if (!frame.isContinuous())
     {
         frame.copyTo(frame3);
-        frame_scaled = &frame3;
+        scaled = &frame3;
     }
     else
-        frame_scaled = &frame;
-
-    freshp = true;
+        scaled = &frame;
 
     int color_cvt = 0;
     constexpr int nchannels = 4;
 
-    switch (frame_scaled->channels())
+    switch (scaled->channels())
     {
     case 1:
         color_cvt = cv::COLOR_GRAY2BGRA;
@@ -56,23 +54,19 @@ void cv_video_widget::update_image(const cv::Mat& frame)
         break;
     }
 
-    cv::Mat const* frame_color;
+    cv::Mat const* color;
 
     if (color_cvt != cv::COLOR_COLORCVT_MAX)
     {
-        cv::cvtColor(*frame_scaled, frame2, color_cvt);
-        frame_color = &frame2;
+        cv::cvtColor(*scaled, frame2, color_cvt);
+        color = &frame2;
     }
     else
-        frame_color = frame_scaled;
+        color = scaled;
 
-    int stride = frame_color->step.p[0], rows = frame_color->rows;
-    unsigned nbytes = (unsigned)(rows * stride);
-    vec.resize(nbytes); vec.shrink_to_fit();
-    std::memcpy(vec.data(), frame_color->data, nbytes);
+    int width = color->cols, height = color->rows;
+    unsigned stride = color->step.p[0];
+    set_image(color->data, width, height, stride, QImage::Format_ARGB32);
 
-    texture = QImage((const unsigned char*) vec.data(), W, H, stride, QImage::Format_ARGB32);
-    texture.setDevicePixelRatio(devicePixelRatioF());
+    set_fresh(true);
 }
-
-cv_video_widget::cv_video_widget(QWidget* parent) : video_widget(parent) {}
diff --git a/cv/video-widget.hpp b/cv/video-widget.hpp
index 9d62f19e..54316d32 100644
--- a/cv/video-widget.hpp
+++ b/cv/video-widget.hpp
@@ -12,7 +12,7 @@
 
 struct cv_video_widget final : video_widget
 {
-    cv_video_widget(QWidget* parent = nullptr);
+    using video_widget::video_widget;
     void update_image(const cv::Mat& frame);
 
 private:
diff --git a/tracker-pt/ftnoir_tracker_pt.cpp b/tracker-pt/ftnoir_tracker_pt.cpp
index a1a0dc1c..a2272d1c 100644
--- a/tracker-pt/ftnoir_tracker_pt.cpp
+++ b/tracker-pt/ftnoir_tracker_pt.cpp
@@ -101,8 +101,7 @@ void Tracker_PT::run()
             widget->update_image(preview_frame->get_bitmap());
 
             {
-                int w = -1, h = -1;
-                widget->get_preview_size(w, h);
+                auto [ w, h ] = widget->preview_size();
                 if (w != preview_width || h != preview_height)
                 {
                     preview_width = w; preview_height = h;
diff --git a/video/video-widget.cpp b/video/video-widget.cpp
index fb380fc4..4394fec7 100644
--- a/video/video-widget.cpp
+++ b/video/video-widget.cpp
@@ -3,76 +3,86 @@
 #include "compat/check-visible.hpp"
 #include "compat/math.hpp"
 
-#include <cstddef>
 #include <cstring>
 
 #include <QPainter>
+#include <QtAlgorithms>
+#include <QDebug>
 
 void video_widget::init_image_nolock()
 {
-    texture = QImage(W, H, QImage::Format_ARGB32);
-    texture.setDevicePixelRatio(devicePixelRatioF());
+    double dpr = devicePixelRatioF();
+    size_.store({ iround(width() * dpr), iround(height() * dpr) },
+                std::memory_order_release);
 }
 
 video_widget::video_widget(QWidget* parent) : QWidget(parent)
 {
-    W = width(); H = height();
-    init_image_nolock(); texture.fill(Qt::gray);
-
-    connect(&timer, &QTimer::timeout, this, &video_widget::update_and_repaint, Qt::DirectConnection);
+    init_image_nolock();
+    connect(&timer, &QTimer::timeout, this, &video_widget::draw_image, Qt::DirectConnection);
     timer.start(65);
 }
 
 void video_widget::update_image(const QImage& img)
 {
-    if (freshp.load(std::memory_order_relaxed))
+    if (fresh())
         return;
 
+    set_image(img.constBits(), img.width(), img.height(),
+              img.bytesPerLine(), img.format());
+    set_fresh(true);
+}
+
+void video_widget::set_image(const unsigned char* src, int width, int height,
+                             unsigned stride, QImage::Format fmt)
+{
     QMutexLocker l(&mtx);
 
-    unsigned nbytes = (unsigned)(img.bytesPerLine() * img.height());
+    texture = QImage();
+    unsigned nbytes = stride * height;
     vec.resize(nbytes); vec.shrink_to_fit();
-    std::memcpy(vec.data(), img.constBits(), nbytes);
-
-    texture = QImage((const unsigned char*) vec.data(), img.width(), img.height(), img.bytesPerLine(), img.format());
-    texture.setDevicePixelRatio(devicePixelRatioF());
-
-    freshp.store(true, std::memory_order_relaxed);
+    std::memcpy(vec.data(), src, nbytes);
+    texture = QImage((const unsigned char*)vec.data(), width, height, stride, fmt);
 }
 
 void video_widget::paintEvent(QPaintEvent*)
 {
-    QMutexLocker foo(&mtx);
-
     QPainter painter(this);
+
+    QMutexLocker l(&mtx);
     painter.drawImage(rect(), texture);
 }
 
-void video_widget::update_and_repaint()
+void video_widget::draw_image()
 {
-    if (!freshp.load(std::memory_order_relaxed))
+    if (!fresh())
         return;
 
     if (!check_is_visible())
         return;
 
-    QMutexLocker l(&mtx);
     repaint();
-    freshp.store(false, std::memory_order_relaxed);
+    set_fresh(false);
 }
 
 void video_widget::resizeEvent(QResizeEvent*)
 {
     QMutexLocker l(&mtx);
-    double dpr = devicePixelRatioF();
-    W = iround(width() * dpr);
-    H = iround(height() * dpr);
     init_image_nolock();
 }
 
-void video_widget::get_preview_size(int& w, int& h)
+std::tuple<int, int> video_widget::preview_size() const
 {
-    QMutexLocker l(&mtx);
-    w = W; h = H;
+    QSize sz = size_.load(std::memory_order_acquire);
+    return { sz.width(), sz.height() };
 }
 
+bool video_widget::fresh() const
+{
+    return fresh_.load(std::memory_order_acquire);
+}
+
+void video_widget::set_fresh(bool x)
+{
+    fresh_.store(x, std::memory_order_release);
+}
diff --git a/video/video-widget.hpp b/video/video-widget.hpp
index 563f468c..ab5d42d2 100644
--- a/video/video-widget.hpp
+++ b/video/video-widget.hpp
@@ -11,36 +11,39 @@
 #include "export.hpp"
 
 #include <vector>
+#include <atomic>
+#include <tuple>
 
 #include <QWidget>
+#include <QImage>
 #include <QTimer>
+
 #include <QMutex>
 
-class OTR_VIDEO_EXPORT video_widget : public QWidget
+struct OTR_VIDEO_EXPORT video_widget : QWidget
 {
-    Q_OBJECT
-
-public:
     video_widget(QWidget* parent = nullptr);
 
     void update_image(const QImage& image);
-    void get_preview_size(int& w, int& h);
+    std::tuple<int, int> preview_size() const;
     void resizeEvent(QResizeEvent*) override;
-protected slots:
     void paintEvent(QPaintEvent*) override;
-    void update_and_repaint();
-private:
-    QTimer timer;
+    void draw_image();
 
 protected:
-    QMutex mtx { QMutex::Recursive };
+    mutable QMutex mtx { QMutex::NonRecursive };
     QImage texture;
     std::vector<unsigned char> vec;
+    bool fresh() const;
+    void set_fresh(bool x);
+    void set_image(const unsigned char* src, int width, int height, unsigned stride, QImage::Format fmt);
 
-    bool freshp = false;
+private:
+    void init_image_nolock();
+    QTimer timer;
 
-    int W = iround(QWidget::width() * devicePixelRatioF());
-    int H = iround(QWidget::height() * devicePixelRatioF());
+    std::atomic<QSize> size_ = QSize(320, 240);
+    std::atomic<bool> fresh_ { false };
 
-    void init_image_nolock();
+    static_assert(decltype(fresh_)::is_always_lock_free);
 };