From 7de0f46028e964bd7b7979835ef0a80cf8cb01b7 Mon Sep 17 00:00:00 2001
From: Stanislaw Halik <sthalik@misaki.pl>
Date: Fri, 22 Feb 2019 15:20:52 +0100
Subject: 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.
---
 video/video-widget.cpp | 64 +++++++++++++++++++++++++++++---------------------
 video/video-widget.hpp | 31 +++++++++++++-----------
 2 files changed, 54 insertions(+), 41 deletions(-)

(limited to 'video')

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);
 };
-- 
cgit v1.2.3