Make the white balance picker more robust.

Earlier the change to neutral white balance could go out of sync
with when we wanted to sample, so we could come too early or too
late and get an already-corrected value.

This is also helped by the previous theme fixes for storing the
neutral_colors array as an upvalue in prepare_chain().

diff --git a/glwidget.cpp b/glwidget.cpp
index ca9416bc4f38a14e26566696642a1ba7491fd8f9..574bf61a6b3e116fa219a5df1245fddb780f37e2 100644 (file)
--- a/glwidget.cpp
+++ b/glwidget.cpp
@@ -33,11 +33,23 @@ class QMouseEvent;
 #include <movit/util.h>
 #include <string>
 
-
 using namespace movit;
 using namespace std;
 using namespace std::placeholders;
 
+namespace {
+
+double srgb_to_linear(double x)
+{
+       if (x < 0.04045) {
+               return x / 12.92;
+       } else {
+               return pow((x + 0.055) / 1.055, 2.4);
+       }
+}
+
+}  // namespace
+
 GLWidget::GLWidget(QWidget *parent)
     : QGLWidget(parent, global_share_widget)
 {
@@ -56,6 +68,22 @@ void GLWidget::shutdown()
        global_mixer->remove_frame_ready_callback(output, this);
 }
 
+void GLWidget::grab_white_balance(unsigned channel, unsigned x, unsigned y)
+{
+       // Set the white balance to neutral for the grab. It's probably going to
+       // flicker a bit, but hopefully this display is not live anyway.
+       global_mixer->set_wb(output, 0.5, 0.5, 0.5);
+       global_mixer->wait_for_next_frame();
+
+       // Mark that the next paintGL() should grab the given pixel.
+       grab_x = x;
+       grab_y = y;
+       grab_output = Mixer::Output(Mixer::OUTPUT_INPUT0 + channel);
+       should_grab = true;
+
+       updateGL();
+}
+
 void GLWidget::initializeGL()
 {
        static once_flag flag;
@@ -122,6 +150,16 @@ void GLWidget::paintGL()
        } else {
                assert(resource_pool == frame.chain->get_resource_pool());
        }
+
+       if (should_grab) {
+               QRgb reference_color = grabFrameBuffer().pixel(grab_x, grab_y);
+
+               double r = srgb_to_linear(qRed(reference_color) / 255.0);
+               double g = srgb_to_linear(qGreen(reference_color) / 255.0);
+               double b = srgb_to_linear(qBlue(reference_color) / 255.0);
+               global_mixer->set_wb(grab_output, r, g, b);
+               should_grab = false;
+       }
 }
 
 void GLWidget::mousePressEvent(QMouseEvent *event)

diff --git a/glwidget.h b/glwidget.h
index ed67ae8621169fd7852c9bc9bccd5de2aa0797b7..9b554d078d5280e1ae441bd345485f97112156a8 100644 (file)
--- a/glwidget.h
+++ b/glwidget.h
@@ -39,6 +39,9 @@ public:
 
        void shutdown();
 
+       // NOTE: Will make the white balance flicker for a frame.
+       void grab_white_balance(unsigned channel, unsigned x, unsigned y);
+
 protected:
        void initializeGL() override;
        void resizeGL(int width, int height) override;
@@ -63,6 +66,9 @@ private:
        GLuint position_vbo, texcoord_vbo;
        movit::ResourcePool *resource_pool = nullptr;
        int current_width = 1, current_height = 1;
+       bool should_grab = false;
+       unsigned grab_x, grab_y;
+       Mixer::Output grab_output;  // Should nominally be the same as output.
 };
 
 #endif

diff --git a/mainwindow.cpp b/mainwindow.cpp
index bbafc25d05ecf275bba006d4e14fe58c96bc7b2d..313097de4de63807b5f17fa2637a31a99d2e88d0 100644 (file)
--- a/mainwindow.cpp
+++ b/mainwindow.cpp
@@ -1468,7 +1468,9 @@ bool MainWindow::eventFilter(QObject *watched, QEvent *event)
                QApplication::restoreOverrideCursor();
                if (watched == previews[current_wb_pick_display]->display) {
                        const QMouseEvent *mouse_event = (QMouseEvent *)event;
-                       set_white_balance(current_wb_pick_display, mouse_event->x(), mouse_event->y());
+                       previews[current_wb_pick_display]->display->grab_white_balance(
+                               current_wb_pick_display,
+                               mouse_event->x(), mouse_event->y());
                } else {
                        // The user clicked on something else, give up.
                        // (The click goes through, which might not be ideal, but, yes.)
@@ -1494,34 +1496,6 @@ void MainWindow::closeEvent(QCloseEvent *event)
        event->accept();
 }
 
-namespace {
-
-double srgb_to_linear(double x)
-{
-       if (x < 0.04045) {
-               return x / 12.92;
-       } else {
-               return pow((x + 0.055) / 1.055, 2.4);
-       }
-}
-
-}  // namespace
-
-void MainWindow::set_white_balance(int channel_number, int x, int y)
-{
-       // Set the white balance to neutral for the grab. It's probably going to
-       // flicker a bit, but hopefully this display is not live anyway.
-       global_mixer->set_wb(Mixer::OUTPUT_INPUT0 + channel_number, 0.5, 0.5, 0.5);
-       previews[channel_number]->display->updateGL();
-       QRgb reference_color = previews[channel_number]->display->grabFrameBuffer().pixel(x, y);
-
-       double r = srgb_to_linear(qRed(reference_color) / 255.0);
-       double g = srgb_to_linear(qGreen(reference_color) / 255.0);
-       double b = srgb_to_linear(qBlue(reference_color) / 255.0);
-       global_mixer->set_wb(Mixer::OUTPUT_INPUT0 + channel_number, r, g, b);
-       previews[channel_number]->display->updateGL();
-}
-
 void MainWindow::audio_state_changed()
 {
        post_to_main_thread([this]{

diff --git a/mainwindow.h b/mainwindow.h
index 62551d9bd3d908bcc4af03d5670c513a24d3db1d..8b896890cc1aaaa73c71e29c3f8c0df6a2fcec24 100644 (file)
--- a/mainwindow.h
+++ b/mainwindow.h
@@ -125,7 +125,6 @@ private:
        void setup_audio_expanded_view();
        bool eventFilter(QObject *watched, QEvent *event) override;
        void closeEvent(QCloseEvent *event) override;
-       void set_white_balance(int channel_number, int x, int y);
        void update_cutoff_labels(float cutoff_hz);
        void update_eq_label(unsigned bus_index, EQBand band, float gain_db);
        void setup_theme_menu();

diff --git a/mixer.cpp b/mixer.cpp
index 5cee30005faee349e8023396b9cd5f98d5072316..a168b66bcdd41fbbf7e575edf761551fa717b33e 100644 (file)
--- a/mixer.cpp
+++ b/mixer.cpp
@@ -1066,7 +1066,11 @@ void Mixer::thread_func()
 
                int64_t frame_duration = output_frame_info.frame_duration;
                render_one_frame(frame_duration);
-               ++frame_num;
+               {
+                       lock_guard<mutex> lock(frame_num_mutex);
+                       ++frame_num;
+               }
+               frame_num_updated.notify_all();
                pts_int += frame_duration;
 
                basic_stats.update(frame_num, stats_dropped_frames);
@@ -1598,6 +1602,14 @@ void Mixer::set_ffmpeg_filename(unsigned card_index, const string &filename) {
        ((FFmpegCapture *)(cards[card_index].capture.get()))->change_filename(filename);
 }
 
+void Mixer::wait_for_next_frame()
+{
+       unique_lock<mutex> lock(frame_num_mutex);
+       unsigned old_frame_num = frame_num;
+       frame_num_updated.wait_for(lock, seconds(1),  // Timeout is just in case.
+               [old_frame_num, this]{ return this->frame_num > old_frame_num; });
+}
+
 Mixer::OutputChannel::~OutputChannel()
 {
        if (has_current_frame) {

diff --git a/mixer.h b/mixer.h
index 5a3e203dc9dc0dd5f02c6eb2c100364313521291..d1bf79519452113a49183e7539596f6c96600010 100644 (file)
--- a/mixer.h
+++ b/mixer.h
@@ -421,6 +421,8 @@ public:
                theme->set_theme_menu_callback(callback);
        }
 
+       void wait_for_next_frame();
+
 private:
        struct CaptureCard;
 
@@ -482,7 +484,10 @@ private:
        movit::YCbCrInput *display_input;
 
        int64_t pts_int = 0;  // In TIMEBASE units.
-       unsigned frame_num = 0;
+
+       mutable std::mutex frame_num_mutex;
+       std::condition_variable frame_num_updated;
+       unsigned frame_num = 0;  // Under <frame_num_mutex>.
 
        // Accumulated errors in number of 1/TIMEBASE audio samples. If OUTPUT_FREQUENCY divided by
        // frame rate is integer, will always stay zero.