When switching output cards, do it from the mixer thread.

This is a _lot_ easier to reason about (and also much more stable
in practice), but we're still having some issues with delays
on disabling video input.

diff --git a/decklink_output.cpp b/decklink_output.cpp
index 21baf618f7aba57a5bab928ee334c33cff40c6db..b3c4c85457dedef75fd1a115fa9b82689ac27599 100644 (file)
--- a/decklink_output.cpp
+++ b/decklink_output.cpp
@@ -181,6 +181,8 @@ void DeckLinkOutput::end_output()
 
 void DeckLinkOutput::send_frame(GLuint y_tex, GLuint cbcr_tex, const vector<RefCountedFrame> &input_frames, int64_t pts, int64_t duration)
 {
+       assert(!should_quit);
+
        unique_ptr<Frame> frame = move(get_frame());
        chroma_subsampler->create_uyvy(y_tex, cbcr_tex, width, height, frame->uyvy_tex);
 
@@ -242,6 +244,8 @@ void DeckLinkOutput::send_audio(int64_t pts, const std::vector<float> &samples)
 
 void DeckLinkOutput::wait_for_frame(int64_t pts, int *dropped_frames, int64_t *frame_duration)
 {
+       assert(!should_quit);
+
        *dropped_frames = 0;
        *frame_duration = this->frame_duration;
 

diff --git a/mixer.cpp b/mixer.cpp
index 457734d29578d80696cb2dfdb55f22b669a5e625..a83d66ef7194a9ff21b01dbe4843c73bc1d852b9 100644 (file)
--- a/mixer.cpp
+++ b/mixer.cpp
@@ -203,7 +203,7 @@ Mixer::Mixer(const QSurfaceFormat &format, unsigned num_cards)
                alsa.reset(new ALSAOutput(OUTPUT_FREQUENCY, /*num_channels=*/2));
        }
        if (global_flags.output_card != -1) {
-               set_output_card(global_flags.output_card);
+               set_output_card_internal(global_flags.output_card);
        }
 }
 
@@ -260,11 +260,11 @@ void Mixer::configure_card(unsigned card_index, CaptureInterface *capture, bool
        audio_mixer.trigger_state_changed_callback();
 }
 
-void Mixer::set_output_card(int card_index)
+void Mixer::set_output_card_internal(int card_index)
 {
-       if (card_index == output_card_index) {
-               return;
-       }
+       // We don't really need to take card_mutex, since we're in the mixer
+       // thread and don't mess with any queues (which is the only thing that happens
+       // from other threads), but it's probably the safest in the long run.
        unique_lock<mutex> lock(card_mutex);
        if (output_card_index != -1) {
                // Switch the old card from output to input.
@@ -285,6 +285,9 @@ void Mixer::set_output_card(int card_index)
        if (card_index != -1) {
                CaptureCard *card = &cards[card_index];
                bmusb::CaptureInterface *capture = card->capture.get();
+               // TODO: DeckLinkCapture::stop_dequeue_thread can actually take
+               // several seconds to complete (blocking on DisableVideoInput);
+               // see if we can maybe do it asynchronously.
                lock.unlock();
                capture->stop_dequeue_thread();
                lock.lock();
@@ -590,6 +593,10 @@ void Mixer::thread_func()
        int stats_dropped_frames = 0;
 
        while (!should_quit) {
+               if (desired_output_card_index != output_card_index) {
+                       set_output_card_internal(desired_output_card_index);
+               }
+
                CaptureCard::NewFrame new_frames[MAX_VIDEO_CARDS];
                bool has_new_frame[MAX_VIDEO_CARDS] = { false };
 

diff --git a/mixer.h b/mixer.h
index b7c5ef790761fcab8fae0d9b4659d4ee63e53fc9..9e4c4d52a222e0d16a38693479f6e91859061269 100644 (file)
--- a/mixer.h
+++ b/mixer.h
@@ -234,6 +234,25 @@ public:
                return cards[card_index].capture->get_description();
        }
 
+       // The difference between this and the previous function is that if a card
+       // is used as the current output, get_card_description() will return the
+       // fake card that's replacing it for input, whereas this function will return
+       // the card's actual name.
+       std::string get_output_card_description(unsigned card_index) const {
+               assert(card_can_be_used_as_output(card_index));
+               assert(card_index < num_cards);
+               if (cards[card_index].parked_capture) {
+                       return cards[card_index].parked_capture->get_description();
+               } else {
+                       return cards[card_index].capture->get_description();
+               }
+       }
+
+       bool card_can_be_used_as_output(unsigned card_index) const {
+               assert(card_index < num_cards);
+               return cards[card_index].output != nullptr;
+       }
+
        std::map<uint32_t, bmusb::VideoMode> get_available_video_modes(unsigned card_index) const {
                assert(card_index < num_cards);
                return cards[card_index].capture->get_available_video_modes();
@@ -285,9 +304,17 @@ public:
                video_encoder->change_x264_bitrate(rate_kbit);
        }
 
+       int get_output_card_index() const {  // -1 = no output, just stream.
+               return desired_output_card_index;
+       }
+
+       void set_output_card(int card_index) { // -1 = no output, just stream.
+               desired_output_card_index = card_index;
+       }
+
 private:
        void configure_card(unsigned card_index, bmusb::CaptureInterface *capture, bool is_fake_capture, DeckLinkOutput *output);
-       void set_output_card(int card_index); // -1 = no output, just stream.
+       void set_output_card_internal(int card_index);  // Should only be called from the mixer thread.
        void bm_frame(unsigned card_index, uint16_t timecode,
                bmusb::FrameAllocator::Frame video_frame, size_t video_offset, bmusb::VideoFormat video_format,
                bmusb::FrameAllocator::Frame audio_frame, size_t audio_offset, bmusb::AudioFormat audio_format);
@@ -310,7 +337,15 @@ private:
        std::unique_ptr<Theme> theme;
        std::atomic<unsigned> audio_source_channel{0};
        std::atomic<int> master_clock_channel{0};  // Gets overridden by <output_card_index> if set.
-       std::atomic<int> output_card_index{-1};  // -1 for none.
+       int output_card_index = -1;  // -1 for none.
+
+       // The mechanics of changing the output card are so intricately connected
+       // with the work the mixer thread is doing. Thus, we don't change it directly,
+       // we just set this variable instead, which signals to the mixer thread that
+       // it should do the change before the next frame. This simplifies locking
+       // considerations immensely.
+       std::atomic<int> desired_output_card_index{-1};
+
        std::unique_ptr<movit::EffectChain> display_chain;
        std::unique_ptr<ChromaSubsampler> chroma_subsampler;
        std::unique_ptr<VideoEncoder> video_encoder;