Refcount the input frames directly instead of trying to free them after-the-fact...

diff --git a/Makefile b/Makefile
index e57af7d70f7c3f9a87090a676884e2fc69a5af72..624ddcdef19c65214a1a4a02db074c3f35f0e6e1 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -8,7 +8,7 @@ OBJS=glwidget.o main.o mainwindow.o window.o
 OBJS += glwidget.moc.o mainwindow.moc.o window.moc.o
 
 # Mixer objects
-OBJS += h264encode.o mixer.o bmusb.o pbo_frame_allocator.o context.o
+OBJS += h264encode.o mixer.o bmusb.o pbo_frame_allocator.o context.o ref_counted_frame.o
 
 %.o: %.cpp
        $(CXX) -MMD -MP $(CPPFLAGS) $(CXXFLAGS) -o $@ -c $<

diff --git a/h264encode.cpp b/h264encode.cpp
index 6eb11d1ef1204b0efc7d03b67bde09a3cb603733..19cd2fc3f7ba99dad675e8575326e597df44e907 100644 (file)
--- a/h264encode.cpp
+++ b/h264encode.cpp
@@ -1868,11 +1868,11 @@ bool H264Encoder::begin_frame(GLuint *y_tex, GLuint *cbcr_tex)
        return true;
 }
 
-void H264Encoder::end_frame(RefCountedGLsync fence, const std::vector<FrameAllocator::Frame> &input_frames_to_release)
+void H264Encoder::end_frame(RefCountedGLsync fence, const std::vector<RefCountedFrame> &input_frames)
 {
        {
                unique_lock<mutex> lock(frame_queue_mutex);
-               pending_frames[current_storage_frame++] = PendingFrame{ fence, input_frames_to_release };
+               pending_frames[current_storage_frame++] = PendingFrame{ fence, input_frames };
        }
        frame_queue_nonempty.notify_one();
 }
@@ -1901,10 +1901,7 @@ void H264Encoder::copy_thread_func()
                glClientWaitSync(frame.fence.get(), 0, 0);
 
                // Release back any input frames we needed to render this frame.
-               // (Actually, those that were needed one output frame ago.)
-               for (FrameAllocator::Frame input_frame : frame.input_frames_to_release) {
-                       input_frame.owner->release_frame(input_frame);
-               }
+               frame.input_frames.clear();
 
                // Unmap the image.
                GLSurface *surf = &gl_surfaces[current_frame_display % SURFACE_NUM];

diff --git a/h264encode.h b/h264encode.h
index b9effa76d86eadcd0b466762196392d1d9adef05..9525d802c8875a8908e30dfa319b246087ac825c 100644 (file)
--- a/h264encode.h
+++ b/h264encode.h
@@ -42,6 +42,7 @@ extern "C" {
 #include "bmusb.h"
 #include "context.h"
 #include "pbo_frame_allocator.h"
+#include "ref_counted_frame.h"
 #include "ref_counted_gl_sync.h"
 
 class QSurface;
@@ -66,7 +67,7 @@ public:
        void 
 #endif
        bool begin_frame(GLuint *y_tex, GLuint *cbcr_tex);
-       void end_frame(RefCountedGLsync fence, const std::vector<FrameAllocator::Frame> &input_frames_to_release);
+       void end_frame(RefCountedGLsync fence, const std::vector<RefCountedFrame> &input_frames);
 
 private:
        struct storage_task {
@@ -98,7 +99,7 @@ private:
 
        struct PendingFrame {
                RefCountedGLsync fence;
-               std::vector<FrameAllocator::Frame> input_frames_to_release;
+               std::vector<RefCountedFrame> input_frames;
        };
        std::map<int, PendingFrame> pending_frames;
        QSurface *surface;

diff --git a/mixer.cpp b/mixer.cpp
index ad0c38dc2eb67ea47ae1d55df1a861280cb79658..2ae8276ae7ef3780885c54ccc5b1f390778390a4 100644 (file)
--- a/mixer.cpp
+++ b/mixer.cpp
@@ -228,12 +228,12 @@ void Mixer::bm_frame(int card_index, uint16_t timecode,
        {
                std::unique_lock<std::mutex> lock(bmusb_mutex);
                card->new_data_ready = true;
-               card->new_frame = video_frame;
+               card->new_frame = RefCountedFrame(video_frame);
                card->new_data_ready_fence = fence;
                card->new_data_ready_changed.notify_all();
        }
 
-       // Video frame will be released later.
+       // Video frame will be released when last user of card->new_frame goes out of scope.
         card->usb->get_audio_frame_allocator()->release_frame(audio_frame);
 }
        
@@ -393,20 +393,11 @@ void Mixer::thread_func()
                        }
                }
 
-               vector<FrameAllocator::Frame> input_frames_to_release;
-       
                for (int card_index = 0; card_index < NUM_CARDS; ++card_index) {
                        CaptureCard *card = &card_copy[card_index];
                        if (!card->new_data_ready)
                                continue;
 
-                       // Now we're done with the previous frame, so we can definitely
-                       // release it when this is done rendering. (Actually, we could do
-                       // it one frame earlier, but before we have a new one, there's no
-                       // knowing when the current one is released.)
-                       if (bmusb_current_rendering_frame[card_index].owner != nullptr) {
-                               input_frames_to_release.push_back(bmusb_current_rendering_frame[card_index]);
-                       }
                        bmusb_current_rendering_frame[card_index] = card->new_frame;
                        check_error();
 
@@ -417,7 +408,7 @@ void Mixer::thread_func()
                        check_error();
                        glDeleteSync(card->new_data_ready_fence);
                        check_error();
-                       const PBOFrameAllocator::Userdata *userdata = (const PBOFrameAllocator::Userdata *)card->new_frame.userdata;
+                       const PBOFrameAllocator::Userdata *userdata = (const PBOFrameAllocator::Userdata *)card->new_frame->userdata;
                        input[card_index]->set_texture_num(0, userdata->tex_y);
                        input[card_index]->set_texture_num(1, userdata->tex_cbcr);
 
@@ -455,7 +446,15 @@ void Mixer::thread_func()
 
                RefCountedGLsync fence(GL_SYNC_GPU_COMMANDS_COMPLETE, /*flags=*/0);
                check_error();
-               h264_encoder->end_frame(fence, input_frames_to_release);
+
+               // Make sure the H.264 gets a reference to all the
+               // input frames needed, so that they are not released back
+               // until the rendering is done.
+               vector<RefCountedFrame> input_frames;
+               for (int card_index = 0; card_index < NUM_CARDS; ++card_index) {
+                       input_frames.push_back(bmusb_current_rendering_frame[card_index]);
+               }
+               h264_encoder->end_frame(fence, input_frames);
 
                output_channel[OUTPUT_LIVE].output_frame(rgba_tex, fence);
                output_channel[OUTPUT_PREVIEW].output_frame(preview_rgba_tex, fence);

diff --git a/mixer.h b/mixer.h
index 92395a1b4c8e78ed8b44aca05a1d29fb28af5c42..c4697310ec95757fecbfacea767ed887ba821e03 100644 (file)
--- a/mixer.h
+++ b/mixer.h
@@ -11,6 +11,7 @@
 #include "bmusb.h"
 #include "h264encode.h"
 #include "pbo_frame_allocator.h"
+#include "ref_counted_frame.h"
 #include "ref_counted_gl_sync.h"
 
 #define NUM_CARDS 2
@@ -101,13 +102,13 @@ private:
                QOpenGLContext *context;
 
                bool new_data_ready = false;  // Whether new_frame contains anything.
-               FrameAllocator::Frame new_frame;
+               RefCountedFrame new_frame;
                GLsync new_data_ready_fence;  // Whether new_frame is ready for rendering.
                std::condition_variable new_data_ready_changed;  // Set whenever new_data_ready is changed.
        };
        CaptureCard cards[NUM_CARDS];  // protected by <bmusb_mutex>
 
-       FrameAllocator::Frame bmusb_current_rendering_frame[NUM_CARDS];
+       RefCountedFrame bmusb_current_rendering_frame[NUM_CARDS];
 
        class OutputChannel {
        public:

diff --git a/ref_counted_frame.cpp b/ref_counted_frame.cpp
new file mode 100644 (file)
index 0000000..725291e
--- /dev/null
+++ b/ref_counted_frame.cpp
@@ -0,0 +1,7 @@
+#include "ref_counted_frame.h"
+
+void release_refcounted_frame(FrameAllocator::Frame *frame)
+{
+       frame->owner->release_frame(*frame);
+       delete frame;
+}

diff --git a/ref_counted_frame.h b/ref_counted_frame.h
new file mode 100644 (file)
index 0000000..5912ce8
--- /dev/null
+++ b/ref_counted_frame.h
@@ -0,0 +1,24 @@
+#ifndef _REF_COUNTED_FRAME_H
+#define _REF_COUNTED_FRAME_H 1
+
+// A wrapper around FrameAllocator::Frame that is automatically refcounted;
+// when the refcount goes to zero, the frame is given back to the allocator.
+//
+// Note that the important point isn't really the pointer to the Frame itself,
+// it's the resources it's representing that need to go back to the allocator.
+
+#include "bmusb.h"
+
+void release_refcounted_frame(FrameAllocator::Frame *frame);
+
+typedef std::shared_ptr<FrameAllocator::Frame> RefCountedFrameBase;
+
+class RefCountedFrame : public RefCountedFrameBase {
+public:
+       RefCountedFrame() {}
+
+       RefCountedFrame(const FrameAllocator::Frame &frame)
+               : RefCountedFrameBase(new FrameAllocator::Frame(frame), release_refcounted_frame) {}
+};
+
+#endif  // !defined(_REF_COUNTED_FRAME_H)