v4l2sink: change where buffers get dequeued

It seems to cause strange occasional high latencies (almost 200ms) when dequeuing buffers from _buffer_alloc().  It is simpler and seems to work much better to dqbuf from the same thread that is queuing the next buffer.

sys/v4l2/gstv4l2bufferpool.c

@@ -61,13 +61,6 @@ gst_v4l2_buffer_finalize (GstV4l2Buffer * buffer)
   GST_LOG_OBJECT (pool->v4l2elem, "finalizing buffer %p %d", buffer, index);
 
   GST_V4L2_BUFFER_POOL_LOCK (pool);
-  if (GST_BUFFER_SIZE (buffer) != 0) {
-    /* BUFFER_SIZE is only set if the frame was dequeued */
-    pool->num_live_buffers--;
-    GST_DEBUG_OBJECT (pool->v4l2elem, "num_live_buffers--: %d",
-        pool->num_live_buffers);
-  }
-
   if (pool->running) {
     if (pool->requeuebuf) {
       if (!gst_v4l2_buffer_pool_qbuf (pool, buffer)) {
@@ -368,6 +361,7 @@ gst_v4l2_buffer_pool_new (GstElement * v4l2elem, gint fd, gint num_buffers,
     pool->buffers[n] = gst_v4l2_buffer_new (pool, n, caps);
     if (!pool->buffers[n])
       goto buffer_new_failed;
+    pool->num_live_buffers++;
     g_async_queue_push (pool->avail_buffers, pool->buffers[n]);
   }
 
@@ -452,46 +446,17 @@ gst_v4l2_buffer_pool_destroy (GstV4l2BufferPool * pool)
  * Get an available buffer in the pool
  *
  * @pool   the "this" object
- * @blocking   if <code>TRUE</code>, then suspend until a buffer is available
  */
 GstV4l2Buffer *
-gst_v4l2_buffer_pool_get (GstV4l2BufferPool * pool, gboolean blocking)
+gst_v4l2_buffer_pool_get (GstV4l2BufferPool * pool)
 {
-  GstV4l2Buffer *buf = NULL;
+  GstV4l2Buffer *buf = g_async_queue_try_pop (pool->avail_buffers);
 
-  do {
-    buf = g_async_queue_try_pop (pool->avail_buffers);
-
-    /* if there isn't a buffer avail, let's try to dequeue one:
-     */
-    if (blocking && !buf) {
-      GST_DEBUG_OBJECT (pool->v4l2elem, "No buffers available.. need to dqbuf");
-      buf = gst_v4l2_buffer_pool_dqbuf (pool);
-
-      /* note: if we get a buf, we don't want to use it directly (because
-       * someone else could still hold a ref).. but instead we release our
-       * reference to it, and if no one else holds a ref it will be returned
-       * to the pool of available buffers..  and if not, we keep looping.
-       */
-      if (buf) {
-        gst_buffer_unref (GST_BUFFER (buf));
-        buf = NULL;
-      }
-    } else {
-      break;
-    }
-  } while (1);
-
-  if (buf) {
-    pool->num_live_buffers++;
-    GST_DEBUG_OBJECT (pool->v4l2elem, "num_live_buffers++: %d",
-        pool->num_live_buffers);
+  if (buf)
     GST_BUFFER_SIZE (buf) = buf->vbuffer.length;
-  }
 
   pool->running = TRUE;
 
-
   return buf;
 }
 
@@ -623,6 +588,11 @@ gst_v4l2_buffer_pool_dqbuf (GstV4l2BufferPool * pool)
   return NULL;
 }
 
+/**
+ * gst_v4l2_buffer_pool_available_buffers:
+ * Returns the number of buffers available to the driver, ie. buffers that
+ * have been QBUF'd but not yet DQBUF'd.
+ */
 gint
 gst_v4l2_buffer_pool_available_buffers (GstV4l2BufferPool * pool)
 {

sys/v4l2/gstv4l2bufferpool.h

@@ -59,7 +59,7 @@ struct _GstV4l2BufferPool
 
   GMutex *lock;
   gboolean running;          /* with lock */
-  gint num_live_buffers;     /* number of buffers not with driver (capture) or not in avail buffer pool (display) */
+  gint num_live_buffers;     /* number of buffers not with driver */
   GAsyncQueue* avail_buffers;/* pool of available buffers, not with the driver and which aren't held outside the bufferpool */
   gint video_fd;             /* a dup(2) of the v4l2object's video_fd */
   guint buffer_count;
@@ -84,7 +84,7 @@ void gst_v4l2_buffer_pool_destroy (GstV4l2BufferPool * pool);
 GstV4l2BufferPool *gst_v4l2_buffer_pool_new (GstElement *v4l2elem, gint fd, gint num_buffers, GstCaps * caps, gboolean requeuebuf, enum v4l2_buf_type type);
 
 
-GstV4l2Buffer *gst_v4l2_buffer_pool_get (GstV4l2BufferPool *pool, gboolean blocking);
+GstV4l2Buffer *gst_v4l2_buffer_pool_get (GstV4l2BufferPool *pool);
 gboolean gst_v4l2_buffer_pool_qbuf (GstV4l2BufferPool *pool, GstV4l2Buffer *buf);
 GstV4l2Buffer *gst_v4l2_buffer_pool_dqbuf (GstV4l2BufferPool *pool);
 

sys/v4l2/gstv4l2sink.c

@@ -641,18 +641,17 @@ gst_v4l2sink_buffer_alloc (GstBaseSink * bsink, guint64 offset, guint size,
       }
     }
 
-    v4l2buf = gst_v4l2_buffer_pool_get (v4l2sink->pool, TRUE);
+    v4l2buf = gst_v4l2_buffer_pool_get (v4l2sink->pool);
 
-    GST_DEBUG_OBJECT (v4l2sink, "allocated buffer: %p\n", v4l2buf);
-
-    if (G_UNLIKELY (!v4l2buf)) {
+    if (G_LIKELY (v4l2buf)) {
+      GST_DEBUG_OBJECT (v4l2sink, "allocated buffer: %p", v4l2buf);
+      *buf = GST_BUFFER (v4l2buf);
+      return GST_FLOW_OK;
+    } else {
+      GST_DEBUG_OBJECT (v4l2sink, "failed to allocate buffer");
       return GST_FLOW_ERROR;
     }
 
-    *buf = GST_BUFFER (v4l2buf);
-
-    return GST_FLOW_OK;
-
   } else {
     GST_ERROR_OBJECT (v4l2sink, "only supporting streaming mode for now...");
     return GST_FLOW_ERROR;
@@ -666,7 +665,7 @@ gst_v4l2sink_show_frame (GstBaseSink * bsink, GstBuffer * buf)
   GstV4l2Sink *v4l2sink = GST_V4L2SINK (bsink);
   GstBuffer *newbuf = NULL;
 
-  GST_DEBUG_OBJECT (v4l2sink, "render buffer: %p\n", buf);
+  GST_DEBUG_OBJECT (v4l2sink, "render buffer: %p", buf);
 
   if (!GST_IS_V4L2_BUFFER (buf)) {
     GstFlowReturn ret;
@@ -686,7 +685,7 @@ gst_v4l2sink_show_frame (GstBaseSink * bsink, GstBuffer * buf)
         GST_BUFFER_DATA (buf),
         MIN (GST_BUFFER_SIZE (newbuf), GST_BUFFER_SIZE (buf)));
 
-    GST_DEBUG_OBJECT (v4l2sink, "render copied buffer: %p\n", newbuf);
+    GST_DEBUG_OBJECT (v4l2sink, "render copied buffer: %p", newbuf);
 
     buf = newbuf;
   }
@@ -707,5 +706,22 @@ gst_v4l2sink_show_frame (GstBaseSink * bsink, GstBuffer * buf)
     gst_buffer_ref (buf);
   }
 
+  /* if the driver has more than one buffer, ie. more than just the one we
+   * just queued, then dequeue one immediately to make it available via
+   * _buffer_alloc():
+   */
+  if (gst_v4l2_buffer_pool_available_buffers (v4l2sink->pool) > 1) {
+    GstV4l2Buffer *v4l2buf = gst_v4l2_buffer_pool_dqbuf (v4l2sink->pool);
+
+    /* note: if we get a buf, we don't want to use it directly (because
+     * someone else could still hold a ref).. but instead we release our
+     * reference to it, and if no one else holds a ref it will be returned
+     * to the pool of available buffers..  and if not, we keep looping.
+     */
+    if (v4l2buf) {
+      gst_buffer_unref (GST_BUFFER (v4l2buf));
+    }
+  }
+
   return GST_FLOW_OK;
 }

sys/v4l2/v4l2src_calls.c

@@ -68,7 +68,7 @@ gst_v4l2src_buffer_pool_activate (GstV4l2BufferPool * pool,
 {
   GstV4l2Buffer *buf;
 
-  while ((buf = gst_v4l2_buffer_pool_get (pool, FALSE)) != NULL)
+  while ((buf = gst_v4l2_buffer_pool_get (pool)) != NULL)
     if (!gst_v4l2_buffer_pool_qbuf (pool, buf))
       goto queue_failed;