Fix some memory leaks in the VA-API upload code (also make it more RAII-ish to be...

diff --git a/nageru/mixer.cpp b/nageru/mixer.cpp
index e0aa8b1fe27783c685dde0c142a8be586dcb89cf..32c1984fa4c3ee89dd214695a26113c5729f554d 100644 (file)
--- a/nageru/mixer.cpp
+++ b/nageru/mixer.cpp
@@ -1083,9 +1083,7 @@ void Mixer::thread_func()
 
                        if (new_frame->frame->data_copy != nullptr) {
                                int mjpeg_card_index = mjpeg_encoder->get_mjpeg_stream_for_card(card_index);
-                               if (mjpeg_card_index == -1) {
-                                       mjpeg_encoder->finish_frame(new_frame->frame);
-                               } else {
+                               if (mjpeg_card_index != -1) {
                                        mjpeg_encoder->upload_frame(pts_int, mjpeg_card_index, new_frame->frame, new_frame->video_format, new_frame->y_offset, new_frame->cbcr_offset);
                                }
                        }

diff --git a/nageru/mjpeg_encoder.cpp b/nageru/mjpeg_encoder.cpp
index d66ef1438a1ae24a973add5defcde6c386ec0fcc..24aa0ebabbdb3d04dbed9ce8542d059c62d96881 100644 (file)
--- a/nageru/mjpeg_encoder.cpp
+++ b/nageru/mjpeg_encoder.cpp
@@ -304,19 +304,6 @@ void MJPEGEncoder::upload_frame(int64_t pts, unsigned card_index, RefCountedFram
        any_frames_to_be_encoded.notify_all();
 }
 
-void MJPEGEncoder::finish_frame(RefCountedFrame frame)
-{
-       PBOFrameAllocator::Userdata *userdata = (PBOFrameAllocator::Userdata *)frame->userdata;
-
-       if (userdata->data_copy_current_src == PBOFrameAllocator::Userdata::FROM_VA_API) {
-               VAResources resources __attribute__((unused)) = move(userdata->va_resources);
-               ReleaseVAResources release = move(userdata->va_resources_release);
-
-               VAStatus va_status = vaUnmapBuffer(va_dpy->va_dpy, resources.image.buf);
-               CHECK_VASTATUS(va_status, "vaUnmapBuffer");
-       }
-}
-
 int MJPEGEncoder::get_mjpeg_stream_for_card(unsigned card_index)
 {
        // Only bother doing MJPEG encoding if there are any connected clients
@@ -456,6 +443,9 @@ void MJPEGEncoder::release_va_resources(MJPEGEncoder::VAResources resources)
                va_status = vaDestroySurfaces(va_dpy->va_dpy, &it->surface, 1);
                CHECK_VASTATUS(va_status, "vaDestroySurfaces");
 
+               va_status = vaDestroyImage(va_dpy->va_dpy, it->image.image_id);
+               CHECK_VASTATUS(va_status, "vaDestroyImage");
+
                va_resources_freelist.erase(it);
        }
 
@@ -677,9 +667,9 @@ void MJPEGEncoder::encode_jpeg_va(QueuedFrame &&qf)
        VABufferDestroyer destroy_slice_param(va_dpy->va_dpy, slice_param_buffer);
 
        if (userdata->data_copy_current_src == PBOFrameAllocator::Userdata::FROM_VA_API) {
+               // The pixel data is already put into the image by the caller.
                va_status = vaUnmapBuffer(va_dpy->va_dpy, resources.image.buf);
                CHECK_VASTATUS(va_status, "vaUnmapBuffer");
-               // The pixel data is already put into the image by the caller.
        } else {
                assert(userdata->data_copy_current_src == PBOFrameAllocator::Userdata::FROM_MALLOC);
 
@@ -700,6 +690,8 @@ void MJPEGEncoder::encode_jpeg_va(QueuedFrame &&qf)
                CHECK_VASTATUS(va_status, "vaUnmapBuffer");
        }
 
+       qf.frame->data_copy = nullptr;
+
        // Seemingly vaPutImage() (which triggers a GPU copy) is much nicer to the
        // CPU than vaDeriveImage() and copying directly into the GPU's buffers.
        // Exactly why is unclear, but it seems to involve L3 cache usage when there

diff --git a/nageru/mjpeg_encoder.h b/nageru/mjpeg_encoder.h
index aee1b9bf52f0da0891df85313a878635d2bb6bf3..6e0357f0c0f32bd4a862ece103bb74d3bb7b50e5 100644 (file)
--- a/nageru/mjpeg_encoder.h
+++ b/nageru/mjpeg_encoder.h
@@ -39,12 +39,6 @@ public:
        ~MJPEGEncoder();
        void stop();
        void upload_frame(int64_t pts, unsigned card_index, RefCountedFrame frame, const bmusb::VideoFormat &video_format, size_t y_offset, size_t cbcr_offset);
-
-       // If the frame was started (data_copy != nullptr) but will not be finished
-       // (MJPEG decoding was turned off in the meantime), you'll need to call finish_frame()
-       // to release any VA-API resources.
-       void finish_frame(RefCountedFrame frame);
-
        bool using_vaapi() const { return va_dpy != nullptr; }
 
        // Returns -1 for inactive (ie., don't encode frames for this card right now).

diff --git a/nageru/pbo_frame_allocator.cpp b/nageru/pbo_frame_allocator.cpp
index 54182a83a899fa3ca9994974a76316c90e812d4d..7fb06f849b1bff09c78be68ad9b388ff1cd8c7e9 100644 (file)
--- a/nageru/pbo_frame_allocator.cpp
+++ b/nageru/pbo_frame_allocator.cpp
@@ -370,6 +370,21 @@ void PBOFrameAllocator::release_frame(Frame frame)
        }
 #endif
 
+       {
+               // In case we never got to upload the frame to MJPEGEncoder.
+               Userdata *userdata = (Userdata *)frame.userdata;
+               MJPEGEncoder::VAResources resources __attribute__((unused)) = move(userdata->va_resources);
+               MJPEGEncoder::ReleaseVAResources release = move(userdata->va_resources_release);
+
+               if (frame.data_copy != nullptr && userdata->data_copy_current_src == Userdata::FROM_VA_API) {
+                       VADisplay va_dpy = mjpeg_encoder->va_dpy->va_dpy;
+                       VAStatus va_status = vaUnmapBuffer(va_dpy, resources.image.buf);
+                       CHECK_VASTATUS(va_status, "vaUnmapBuffer");
+
+                       frame.data_copy = nullptr;
+               }
+       }
+
        lock_guard<mutex> lock(freelist_mutex);
        freelist.push(frame);
        //--sumsum;