v4l2: rework sink buffer refcounting

This is a followup patch for #700781, which is not quite correct.
The buffer handling is quite complicated here.
The original code intended to the the following:

- gst_v4l2_buffer_pool_process() calls QBUF and adds the buffer to the
  local list.
- The sink calls gst_buffer_unref() which returns the buffer to the pool
  but not the 'free list'.
- Some time later DQBUF returns the buffer and
  gst_v4l2_buffer_pool_release_buffer() puts in on the 'free list'.

If the buffer must be copied then (parent_class)->acquire_buffer() is
called directly to keep the buffer in the pool.

This has two problems:
1. If gst_v4l2_buffer_pool_release_buffer() is called before the buffer is
   returned to the pool, then the buffer is put on the 'free list' twice.
   This can happen if a reference to the buffer is kept outside the sink,
   of if DQBUF returns the buffer, that was just queued with QBUF.
2. If buffers are copied, then all buffers are in the pool at all times. As
   a result gst_v4l2_buffer_pool_stop() and gst_v4l2_buffer_pool_dqbuf()
   can access pool->buffers at the same time, which can lead to memory
   corruption.

The patch for #700781 fixes those problems, but with the side effect that
there are always buffers outside the pool (because they are queued) and
the pool is never stopped.
This patch fixes this by releasing the reference to the buffer after
handling it (to avoid problem 2.) so it can be returned to the pool.
gst_v4l2_buffer_pool_release_buffer() is only called if the buffer is
already in the pool (to avoid problem 1.).

Fixes https://bugzilla.gnome.org/show_bug.cgi?id=701375

1 files changed, 7 insertions, 3 deletions

diff --git a/sys/v4l2/gstv4l2bufferpool.c b/sys/v4l2/gstv4l2bufferpool.c
index 74dd3ff3b..35be4ed67 100644
--- a/sys/v4l2/gstv4l2bufferpool.c
+++ b/sys/v4l2/gstv4l2bufferpool.c
@@ -1248,16 +1248,20 @@ gst_v4l2_buffer_pool_process (GstV4l2BufferPool * pool, GstBuffer * buf)
               goto start_failed;
 
           if (pool->num_queued == pool->num_allocated) {
+            GstBuffer *out;
             /* all buffers are queued, try to dequeue one and release it back
              * into the pool so that _acquire can get to it again. */
-            ret = gst_v4l2_buffer_pool_dqbuf (pool, &to_queue);
+            ret = gst_v4l2_buffer_pool_dqbuf (pool, &out);
             if (ret != GST_FLOW_OK)
               goto done;
 
             /* release the rendered buffer back into the pool. This wakes up any
-             * thread waiting for a buffer in _acquire() */
-            gst_buffer_unref (to_queue);
+             * thread waiting for a buffer in _acquire(). If the buffer still has
+             * a pool then this will happen when the refcount reaches 0 */
+            if (!out->pool)
+              gst_v4l2_buffer_pool_release_buffer (bpool, out);
           }
+          gst_buffer_unref (to_queue);
           break;
         }