Make it possible for the user to select Rec. 601/709 for each input from the UI.

diff --git a/glwidget.cpp b/glwidget.cpp
index a9ad359ac11a09ecc463d895d5ad4a50427ccb7b..73d8b7fd3bcb72a19e6cd10313f3482eb6a5f85e 100644 (file)
--- a/glwidget.cpp
+++ b/glwidget.cpp
@@ -32,6 +32,8 @@ class QMouseEvent;
 #include <movit/util.h>
 #include <string>
 
+
+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, &current_ycbcr_coefficients_auto,
+               &current_ycbcr_coefficients, &current_full_range);
+       {
+               QAction *action = new QAction("Auto", &interpretation_group);
+               action->setCheckable(true);
+               if (current_ycbcr_coefficients_auto) {
+                       action->setChecked(true);
+               }
+               action->setData(QList<QVariant>{"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<QVariant>{"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 ca14739a1813727b3400da73abdd4ab4b3cab925..2f33654537b9cd9bd6ff78a3b6bd86977af1d64f 100644 (file)
--- a/input_state.h
+++ b/input_state.h
@@ -1,6 +1,8 @@
 #ifndef _INPUT_STATE_H
 #define _INPUT_STATE_H 1
 
+#include <movit/image_format.h>
+
 #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 31328212a24dc8a571a758794a82305504d5719c..6439e58cfaf845614357314c7f7f82fdfcd9b206 100644 (file)
--- 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<mutex> 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<mutex> 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<mutex> 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 6828bc66364d27d20d9f0a264816d29d42293882..84ef32db0efac732975c1a5e7d86e94c0db13d4e 100644 (file)
--- a/mixer.h
+++ b/mixer.h
@@ -23,6 +23,8 @@
 #include <thread>
 #include <vector>
 
+#include <movit/image_format.h>
+
 #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 <card_mutex>.
        AudioMixer audio_mixer;  // Same as global_audio_mixer (see audio_mixer.h).

diff --git a/theme.cpp b/theme.cpp
index 9cfcc87b7c10783c87cc36755fc6899e3a023d61..dd564196e3c8abcf1e033e3981812a40f73e27d7 100644 (file)
--- 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 51107d6a5e088e3f6806c3785b6d1d8e1fdad85a..cc8765c8e5e055d6e77eb9eced8da43b8e3e44e7 100644 (file)
--- 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<movit::YCbCrInput *> ycbcr_inputs;  // Multiple ones if deinterlacing. Owned by the chain.
        std::vector<movit::FlatInput *> rgba_inputs;  // Multiple ones if deinterlacing. Owned by the chain.
        movit::Effect *deinterlace_effect = nullptr;  // Owned by the chain.