From 86520183fc1bb7aafb65b766e6f31b722c77a640 Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Thu, 26 Apr 2018 22:20:12 +0200 Subject: [PATCH] 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(). --- glwidget.cpp | 40 +++++++++++++++++++++++++++++++++++++++- glwidget.h | 6 ++++++ mainwindow.cpp | 32 +++----------------------------- mainwindow.h | 1 - mixer.cpp | 14 +++++++++++++- mixer.h | 7 ++++++- 6 files changed, 67 insertions(+), 33 deletions(-) diff --git a/glwidget.cpp b/glwidget.cpp index ca9416b..574bf61 100644 --- a/glwidget.cpp +++ b/glwidget.cpp @@ -33,11 +33,23 @@ class QMouseEvent; #include #include - 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 ed67ae8..9b554d0 100644 --- 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 bbafc25..313097d 100644 --- 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 62551d9..8b89689 100644 --- 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 5cee300..a168b66 100644 --- 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 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 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 5a3e203..d1bf795 100644 --- 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 . // Accumulated errors in number of 1/TIMEBASE audio samples. If OUTPUT_FREQUENCY divided by // frame rate is integer, will always stay zero. -- 2.39.2