From 14dc6f7ef9dd76cba75ffe7499d9a81d66c7b152 Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Sat, 13 May 2017 19:22:44 +0200 Subject: [PATCH] Make it possible for the user to select Rec. 601/709 for each input from the UI. --- glwidget.cpp | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++ input_state.h | 10 ++++++++++ mixer.cpp | 31 ++++++++++++++++++++++++++++++ mixer.h | 11 +++++++++++ theme.cpp | 37 +++++++++++++++++++++++++++-------- theme.h | 1 + 6 files changed, 135 insertions(+), 8 deletions(-) diff --git a/glwidget.cpp b/glwidget.cpp index a9ad359..73d8b7f 100644 --- a/glwidget.cpp +++ b/glwidget.cpp @@ -32,6 +32,8 @@ class QMouseEvent; #include #include + +using namespace movit; using namespace std; using namespace std::placeholders; @@ -185,6 +187,52 @@ void GLWidget::show_preview_context_menu(unsigned signal_num, const QPoint &pos) card_submenu.setTitle("Input source"); menu.addMenu(&card_submenu); + // Note that this setting depends on which card is active. + // TODO: Consider hiding this for BGRA sources. + + QMenu interpretation_submenu; + QActionGroup interpretation_group(&interpretation_submenu); + + bool current_ycbcr_coefficients_auto, current_full_range; + YCbCrLumaCoefficients current_ycbcr_coefficients; + global_mixer->get_input_ycbcr_interpretation( + current_card, ¤t_ycbcr_coefficients_auto, + ¤t_ycbcr_coefficients, ¤t_full_range); + { + QAction *action = new QAction("Auto", &interpretation_group); + action->setCheckable(true); + if (current_ycbcr_coefficients_auto) { + action->setChecked(true); + } + action->setData(QList{"interpretation", true, YCBCR_REC_709, false}); + interpretation_submenu.addAction(action); + } + for (YCbCrLumaCoefficients ycbcr_coefficients : { YCBCR_REC_709, YCBCR_REC_601 }) { + for (bool full_range : { false, true }) { + std::string description; + if (ycbcr_coefficients == YCBCR_REC_709) { + description = "Rec. 709 (HD)"; + } else { + description = "Rec. 601 (SD)"; + } + if (full_range) { + description += ", full range (nonstandard)"; + } + QAction *action = new QAction(QString::fromStdString(description), &interpretation_group); + action->setCheckable(true); + if (!current_ycbcr_coefficients_auto && + ycbcr_coefficients == current_ycbcr_coefficients && + full_range == current_full_range) { + action->setChecked(true); + } + action->setData(QList{"interpretation", false, ycbcr_coefficients, full_range}); + interpretation_submenu.addAction(action); + } + } + + interpretation_submenu.setTitle("Input interpretation"); + menu.addMenu(&interpretation_submenu); + // --- The choices in the next few options depend a lot on which card is active --- // Add a submenu for selecting video input, with an action for each input. @@ -308,6 +356,11 @@ void GLWidget::show_preview_context_menu(unsigned signal_num, const QPoint &pos) } else if (selected[0].toString() == "card") { unsigned card_index = selected[1].toUInt(nullptr); global_mixer->set_signal_mapping(signal_num, card_index); + } else if (selected[0].toString() == "interpretation") { + bool ycbcr_coefficients_auto = selected[1].toBool(); + YCbCrLumaCoefficients ycbcr_coefficients = YCbCrLumaCoefficients(selected[2].toUInt(nullptr)); + bool full_range = selected[3].toBool(); + global_mixer->set_input_ycbcr_interpretation(current_card, ycbcr_coefficients_auto, ycbcr_coefficients, full_range); } else { assert(false); } diff --git a/input_state.h b/input_state.h index ca14739..2f33654 100644 --- a/input_state.h +++ b/input_state.h @@ -1,6 +1,8 @@ #ifndef _INPUT_STATE_H #define _INPUT_STATE_H 1 +#include + #include "defs.h" #include "ref_counted_frame.h" @@ -19,6 +21,14 @@ struct InputState { // we immediately clear out all history and all entries will point to the same // frame. BufferedFrame buffered_frames[MAX_VIDEO_CARDS][FRAME_HISTORY_LENGTH]; + + // For each card, the current Y'CbCr input settings. Ignored for BGRA inputs. + // If ycbcr_coefficients_auto = true for a given card, the others are ignored + // for that card (SD is taken to be Rec. 601, HD is taken to be Rec. 709, + // both limited range). + bool ycbcr_coefficients_auto[MAX_VIDEO_CARDS]; + movit::YCbCrLumaCoefficients ycbcr_coefficients[MAX_VIDEO_CARDS]; + bool full_range[MAX_VIDEO_CARDS]; }; #endif // !defined(_INPUT_STATE_H) diff --git a/mixer.cpp b/mixer.cpp index 3132821..6439e58 100644 --- a/mixer.cpp +++ b/mixer.cpp @@ -1081,6 +1081,17 @@ void Mixer::render_one_frame(int64_t duration) printf("Timecode: '%s'\n", timecode_text.c_str()); } + // Update Y'CbCr settings for all cards. + { + unique_lock lock(card_mutex); + for (unsigned card_index = 0; card_index < num_cards; ++card_index) { + CaptureCard *card = &cards[card_index]; + input_state.ycbcr_coefficients_auto[card_index] = card->ycbcr_coefficients_auto; + input_state.ycbcr_coefficients[card_index] = card->ycbcr_coefficients; + input_state.full_range[card_index] = card->full_range; + } + } + // Get the main chain from the theme, and set its state immediately. Theme::Chain theme_main_chain = theme->get_chain(0, pts(), global_flags.width, global_flags.height, input_state); EffectChain *chain = theme_main_chain.chain; @@ -1282,6 +1293,26 @@ void Mixer::channel_clicked(int preview_num) theme->channel_clicked(preview_num); } +void Mixer::get_input_ycbcr_interpretation(unsigned card_index, bool *ycbcr_coefficients_auto, + movit::YCbCrLumaCoefficients *ycbcr_coefficients, bool *full_range) +{ + unique_lock lock(card_mutex); + CaptureCard *card = &cards[card_index]; + *ycbcr_coefficients_auto = card->ycbcr_coefficients_auto; + *ycbcr_coefficients = card->ycbcr_coefficients; + *full_range = card->full_range; +} + +void Mixer::set_input_ycbcr_interpretation(unsigned card_index, bool ycbcr_coefficients_auto, + movit::YCbCrLumaCoefficients ycbcr_coefficients, bool full_range) +{ + unique_lock lock(card_mutex); + CaptureCard *card = &cards[card_index]; + card->ycbcr_coefficients_auto = ycbcr_coefficients_auto; + card->ycbcr_coefficients = ycbcr_coefficients; + card->full_range = full_range; +} + void Mixer::start_mode_scanning(unsigned card_index) { assert(card_index < num_cards); diff --git a/mixer.h b/mixer.h index 6828bc6..84ef32d 100644 --- a/mixer.h +++ b/mixer.h @@ -23,6 +23,8 @@ #include #include +#include + #include "audio_mixer.h" #include "bmusb/bmusb.h" #include "defs.h" @@ -216,6 +218,11 @@ public: return theme->set_signal_mapping(signal, card); } + void get_input_ycbcr_interpretation(unsigned card_index, bool *ycbcr_coefficients_auto, + movit::YCbCrLumaCoefficients *ycbcr_coefficients, bool *full_range); + void set_input_ycbcr_interpretation(unsigned card_index, bool ycbcr_coefficients_auto, + movit::YCbCrLumaCoefficients ycbcr_coefficients, bool full_range); + bool get_supports_set_wb(unsigned channel) const { return theme->get_supports_set_wb(channel); @@ -439,6 +446,10 @@ private: QueueLengthPolicy queue_length_policy; // Refers to the "new_frames" queue. int last_timecode = -1; // Unwrapped. + + bool ycbcr_coefficients_auto = true; + movit::YCbCrLumaCoefficients ycbcr_coefficients = movit::YCBCR_REC_709; + bool full_range = false; }; CaptureCard cards[MAX_VIDEO_CARDS]; // Protected by . AudioMixer audio_mixer; // Same as global_audio_mixer (see audio_mixer.h). diff --git a/theme.cpp b/theme.cpp index 9cfcc87..dd56419 100644 --- a/theme.cpp +++ b/theme.cpp @@ -719,12 +719,6 @@ LiveInputWrapper::LiveInputWrapper(Theme *theme, EffectChain *chain, bmusb::Pixe } } else { assert(pixel_format == bmusb::PixelFormat_8BitYCbCr || pixel_format == bmusb::PixelFormat_10BitYCbCr); - // The Blackmagic driver docs claim that the device outputs Y'CbCr - // according to Rec. 601, but practical testing indicates it definitely - // is Rec. 709 (at least up to errors attributable to rounding errors). - // Perhaps 601 was only to indicate the subsampling positions, not the - // colorspace itself? Tested with a Lenovo X1 gen 3 as input. - YCbCrFormat input_ycbcr_format; input_ycbcr_format.chroma_subsampling_x = (pixel_format == bmusb::PixelFormat_10BitYCbCr) ? 1 : 2; input_ycbcr_format.chroma_subsampling_y = 1; input_ycbcr_format.num_levels = (pixel_format == bmusb::PixelFormat_10BitYCbCr) ? 1024 : 256; @@ -732,8 +726,8 @@ LiveInputWrapper::LiveInputWrapper(Theme *theme, EffectChain *chain, bmusb::Pixe input_ycbcr_format.cr_x_position = 0.0; input_ycbcr_format.cb_y_position = 0.5; input_ycbcr_format.cr_y_position = 0.5; - input_ycbcr_format.luma_coefficients = YCBCR_REC_709; - input_ycbcr_format.full_range = false; + input_ycbcr_format.luma_coefficients = YCBCR_REC_709; // Will be overridden later. + input_ycbcr_format.full_range = false; // Will be overridden later. for (unsigned i = 0; i < num_inputs; ++i) { // When using 10-bit input, we're converting to interleaved through v210Converter. @@ -778,6 +772,31 @@ void LiveInputWrapper::connect_signal_raw(int signal_num) height = userdata->last_height[first_frame.field_number]; } + movit::YCbCrLumaCoefficients ycbcr_coefficients = theme->input_state->ycbcr_coefficients[signal_num]; + bool full_range = theme->input_state->full_range[signal_num]; + + if (theme->input_state->ycbcr_coefficients_auto[signal_num]) { + full_range = false; + + // The Blackmagic driver docs claim that the device outputs Y'CbCr + // according to Rec. 601, but this seems to indicate the subsampling + // positions only, as they publish Y'CbCr → RGB formulas that are + // different for HD and SD (corresponding to Rec. 709 and 601, respectively), + // and a Lenovo X1 gen 3 I used to test definitely outputs Rec. 709 + // (at least up to rounding error). Other devices seem to use Rec. 601 + // even on HD resolutions. Nevertheless, Rec. 709 _is_ the right choice + // for HD, so we default to that if the user hasn't set anything. + if (height >= 720) { + ycbcr_coefficients = YCBCR_REC_709; + } else { + ycbcr_coefficients = YCBCR_REC_601; + } + } + + // This is a global, but it doesn't really matter. + input_ycbcr_format.luma_coefficients = ycbcr_coefficients; + input_ycbcr_format.full_range = full_range; + BufferedFrame last_good_frame = first_frame; for (unsigned i = 0; i < max(ycbcr_inputs.size(), rgba_inputs.size()); ++i) { BufferedFrame frame = theme->input_state->buffered_frames[signal_num][i]; @@ -801,11 +820,13 @@ void LiveInputWrapper::connect_signal_raw(int signal_num) case bmusb::PixelFormat_8BitYCbCr: ycbcr_inputs[i]->set_texture_num(0, userdata->tex_y[frame.field_number]); ycbcr_inputs[i]->set_texture_num(1, userdata->tex_cbcr[frame.field_number]); + ycbcr_inputs[i]->change_ycbcr_format(input_ycbcr_format); ycbcr_inputs[i]->set_width(width); ycbcr_inputs[i]->set_height(height); break; case bmusb::PixelFormat_10BitYCbCr: ycbcr_inputs[i]->set_texture_num(0, userdata->tex_444[frame.field_number]); + ycbcr_inputs[i]->change_ycbcr_format(input_ycbcr_format); ycbcr_inputs[i]->set_width(width); ycbcr_inputs[i]->set_height(height); break; diff --git a/theme.h b/theme.h index 51107d6..cc8765c 100644 --- a/theme.h +++ b/theme.h @@ -122,6 +122,7 @@ public: private: Theme *theme; // Not owned by us. bmusb::PixelFormat pixel_format; + movit::YCbCrFormat input_ycbcr_format; std::vector ycbcr_inputs; // Multiple ones if deinterlacing. Owned by the chain. std::vector rgba_inputs; // Multiple ones if deinterlacing. Owned by the chain. movit::Effect *deinterlace_effect = nullptr; // Owned by the chain. -- 2.39.2