Fix some issues with cards changing pixel format on-the-fly.

If a card came back with another pixel format than before,
we would still allocate frames with the same pixel format,
which obviously didn't work.

This is very unlikely to happen in the current situation
(or even impossible?), but needs to be fixed for upcoming patches.

diff --git a/nageru/mixer.cpp b/nageru/mixer.cpp
index 465e96708c716c80302f40a7d3ad8987a7d5214e..76ab1bab21c4c47d136160f96e93fbe40b21b3d1 100644 (file)
--- a/nageru/mixer.cpp
+++ b/nageru/mixer.cpp
@@ -568,6 +568,15 @@ void Mixer::configure_card(unsigned card_index, CaptureInterface *capture, CardT
        card->capture->set_frame_callback(bind(&Mixer::bm_frame, this, card_index, _1, _2, _3, _4, _5, _6, _7));
        if (card->frame_allocator == nullptr) {
                card->frame_allocator.reset(new PBOFrameAllocator(pixel_format, 8 << 20, global_flags.width, global_flags.height, card_index, mjpeg_encoder.get()));  // 8 MB.
+       } else {
+               // The format could have changed, but we cannot reset the allocator
+               // and create a new one from scratch, since there may be allocated
+               // frames from it that expect to call release_frame() on it.
+               // Instead, ask the allocator to create new frames for us and discard
+               // any old ones as they come back. This takes the mutex while
+               // allocating, but nothing should really be sending frames in there
+               // right now anyway (start_bm_capture() has not been called yet).
+               card->frame_allocator->reconfigure(pixel_format, 8 << 20, global_flags.width, global_flags.height, card_index, mjpeg_encoder.get());
        }
        card->capture->set_video_frame_allocator(card->frame_allocator.get());
        if (card->surface == nullptr) {

diff --git a/nageru/pbo_frame_allocator.cpp b/nageru/pbo_frame_allocator.cpp
index 133b65fe3310c2b0c69b27330098286e9f6cb994..f9b4601ec3653da0e23fd349a54ad2475291d085 100644 (file)
--- a/nageru/pbo_frame_allocator.cpp
+++ b/nageru/pbo_frame_allocator.cpp
@@ -29,11 +29,20 @@ void set_clamp_to_edge()
 }  // namespace
 
 PBOFrameAllocator::PBOFrameAllocator(bmusb::PixelFormat pixel_format, size_t frame_size, GLuint width, GLuint height, unsigned card_index, MJPEGEncoder *mjpeg_encoder, size_t num_queued_frames, GLenum buffer, GLenum permissions, GLenum map_bits)
-        : card_index(card_index), mjpeg_encoder(mjpeg_encoder), pixel_format(pixel_format), buffer(buffer)
+        : card_index(card_index),
+         mjpeg_encoder(mjpeg_encoder),
+         pixel_format(pixel_format),
+         buffer(buffer),
+         frame_size(frame_size),
+         num_queued_frames(num_queued_frames),
+         width(width),
+         height(height),
+         permissions(permissions),
+         map_bits(map_bits)
 {
        userdata.reset(new Userdata[num_queued_frames]);
        for (size_t i = 0; i < num_queued_frames; ++i) {
-               init_frame(i, frame_size, width, height, permissions, map_bits);
+               init_frame(i, frame_size, width, height, permissions, map_bits, generation);
        }
        glBindBuffer(buffer, 0);
        check_error();
@@ -41,7 +50,7 @@ PBOFrameAllocator::PBOFrameAllocator(bmusb::PixelFormat pixel_format, size_t fra
        check_error();
 }
 
-void PBOFrameAllocator::init_frame(size_t frame_idx, size_t frame_size, GLuint width, GLuint height, GLenum permissions, GLenum map_bits)
+void PBOFrameAllocator::init_frame(size_t frame_idx, size_t frame_size, GLuint width, GLuint height, GLenum permissions, GLenum map_bits, int generation)
 {
        GLuint pbo;
        glGenBuffers(1, &pbo);
@@ -58,6 +67,7 @@ void PBOFrameAllocator::init_frame(size_t frame_idx, size_t frame_size, GLuint w
        frame.size = frame_size;
        Userdata *ud = &userdata[frame_idx];
        frame.userdata = ud;
+       ud->generation = generation;
        ud->pbo = pbo;
        ud->pixel_format = pixel_format;
        ud->data_copy_malloc = new uint8_t[frame_size];
@@ -388,6 +398,60 @@ void PBOFrameAllocator::release_frame(Frame frame)
        }
 
        lock_guard<mutex> lock(freelist_mutex);
-       freelist.push(frame);
+       Userdata *userdata = (Userdata *)frame.userdata;
+       if (userdata->generation == generation) {
+               freelist.push(frame);
+       } else {
+               destroy_frame(&frame);
+       }
        //--sumsum;
 }
+
+void PBOFrameAllocator::reconfigure(bmusb::PixelFormat pixel_format,
+                        size_t frame_size,
+                        GLuint width, GLuint height,
+                        unsigned card_index,
+                        MJPEGEncoder *mjpeg_encoder,
+                        size_t num_queued_frames,
+                        GLenum buffer,
+                        GLenum permissions,
+                        GLenum map_bits)
+{
+       if (pixel_format == this->pixel_format &&
+           frame_size == this->frame_size &&
+           width == this->width && height == this->height &&
+           card_index == this->card_index &&
+           mjpeg_encoder == this->mjpeg_encoder &&
+           num_queued_frames == this->num_queued_frames &&
+           buffer == this->buffer &&
+           permissions == this->permissions &&
+           map_bits == this->map_bits) {
+               return;
+       }
+
+       this->pixel_format = pixel_format;
+       this->frame_size = frame_size;
+       this->width = width;
+       this->height = height;
+       this->card_index = card_index;
+       this->mjpeg_encoder = mjpeg_encoder;
+       this->num_queued_frames = num_queued_frames;
+       this->buffer = buffer;
+       this->permissions = permissions;
+       this->map_bits = map_bits;
+
+       lock_guard<mutex> lock(freelist_mutex);
+       while (!freelist.empty()) {
+               Frame frame = freelist.front();
+               freelist.pop();
+               destroy_frame(&frame);
+       }
+       ++generation;
+       for (size_t i = 0; i < num_queued_frames; ++i) {
+               init_frame(i, frame_size, width, height, permissions, map_bits, generation);
+       }
+
+       // There may still be frames out with the old configuration
+       // (for instance, living in GLWidget); they will be destroyed
+       // when they come back in release_frame().
+}

diff --git a/nageru/pbo_frame_allocator.h b/nageru/pbo_frame_allocator.h
index 0080dd0e97fe3ce729d37aa960895d04ffedc12e..5ea0ffee8f43f72a0abc4b0dcf3be01d8e998112 100644 (file)
--- a/nageru/pbo_frame_allocator.h
+++ b/nageru/pbo_frame_allocator.h
@@ -36,6 +36,17 @@ public:
        Frame create_frame(size_t width, size_t height, size_t stride) override;
        void release_frame(Frame frame) override;
 
+       // NOTE: Does not check the buffer types; they are just assumed to be compatible.
+       void reconfigure(bmusb::PixelFormat pixel_format,
+                        size_t frame_size,
+                        GLuint width, GLuint height,
+                        unsigned card_index,
+                        MJPEGEncoder *mjpeg_encoder = nullptr,
+                        size_t num_queued_frames = 16,
+                        GLenum buffer = GL_PIXEL_UNPACK_BUFFER_ARB,
+                        GLenum permissions = GL_MAP_WRITE_BIT,
+                        GLenum map_bits = GL_MAP_FLUSH_EXPLICIT_BIT);
+
        struct Userdata {
                GLuint pbo;
 
@@ -80,10 +91,12 @@ public:
                uint8_t *data_copy_malloc;
                MJPEGEncoder::VAResources va_resources;
                MJPEGEncoder::ReleaseVAResources va_resources_release;
+
+               int generation;
        };
 
 private:
-       void init_frame(size_t frame_idx, size_t frame_size, GLuint width, GLuint height, GLenum permissions, GLenum map_bits);
+       void init_frame(size_t frame_idx, size_t frame_size, GLuint width, GLuint height, GLenum permissions, GLenum map_bits, int generation);
        void destroy_frame(Frame *frame);
 
        unsigned card_index;
@@ -93,6 +106,14 @@ private:
        std::queue<Frame> freelist;
        GLenum buffer;
        std::unique_ptr<Userdata[]> userdata;
+
+       // Used only for reconfigure(), to check whether we can do without.
+       size_t frame_size;
+       size_t num_queued_frames;
+       GLuint width, height;
+       GLenum permissions;
+       GLenum map_bits;
+       int generation = 0;  // Under freelist_mutex.
 };
 
 #endif  // !defined(_PBO_FRAME_ALLOCATOR)