v4l2bufferpool: Don't stop streaming when pool is flushing

The purpose of being able to flush the buffer pool is only to
unlock any blocked operation. Doing streamoff/streamon had the
side effect of turning off and on the camera. As we do a flush_start
/ flush_stop sequence when shutting down, that would cause a really
quick sequence of streamoff/streamon/streamoff/close which was
causing some cameras to stop working.

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

5 files changed, 97 insertions, 110 deletions

diff --git a/sys/v4l2/gstv4l2bufferpool.c b/sys/v4l2/gstv4l2bufferpool.c
index 56a8279f0..b5a9f47b5 100644
--- a/sys/v4l2/gstv4l2bufferpool.c
+++ b/sys/v4l2/gstv4l2bufferpool.c
@@ -610,24 +610,60 @@ wrong_config:
   }
 }
 
+static GstFlowReturn
+gst_v4l2_buffer_pool_resurect_buffer (GstV4l2BufferPool * pool)
+{
+  GstBufferPoolAcquireParams params = { 0 };
+  GstBuffer *buffer = NULL;
+  GstFlowReturn ret;
+
+  GST_DEBUG_OBJECT (pool, "A buffer was lost, reallocating it");
+
+  /* block recursive calls to this function */
+  g_signal_handler_block (pool->vallocator, pool->group_released_handler);
+
+  params.flags =
+      (GstBufferPoolAcquireFlags) GST_V4L2_BUFFER_POOL_ACQUIRE_FLAG_RESURRECT |
+      GST_BUFFER_POOL_ACQUIRE_FLAG_DONTWAIT;
+  ret =
+      gst_buffer_pool_acquire_buffer (GST_BUFFER_POOL (pool), &buffer, &params);
+
+  if (ret == GST_FLOW_OK)
+    gst_buffer_unref (buffer);
+
+  g_signal_handler_unblock (pool->vallocator, pool->group_released_handler);
+
+  return ret;
+}
+
 static gboolean
 gst_v4l2_buffer_pool_streamon (GstV4l2BufferPool * pool)
 {
   GstV4l2Object *obj = pool->obj;
 
+  if (pool->streaming)
+    return TRUE;
+
   switch (obj->mode) {
     case GST_V4L2_IO_MMAP:
     case GST_V4L2_IO_USERPTR:
     case GST_V4L2_IO_DMABUF:
     case GST_V4L2_IO_DMABUF_IMPORT:
-      if (!pool->streaming) {
-        if (obj->ioctl (pool->video_fd, VIDIOC_STREAMON, &obj->type) < 0)
-          goto streamon_failed;
+      if (!V4L2_TYPE_IS_OUTPUT (pool->obj->type)) {
+        /* For captures, we need to enqueue buffers before we start streaming,
+         * so the driver don't underflow immediatly. As we have put then back
+         * into the base class queue, resurect them, then releasing will queue
+         * them back. */
+        while (gst_v4l2_buffer_pool_resurect_buffer (pool) == GST_FLOW_OK)
+          continue;
+      }
 
-        pool->streaming = TRUE;
+      if (obj->ioctl (pool->video_fd, VIDIOC_STREAMON, &obj->type) < 0)
+        goto streamon_failed;
 
-        GST_DEBUG_OBJECT (pool, "Started streaming");
-      }
+      pool->streaming = TRUE;
+
+      GST_DEBUG_OBJECT (pool, "Started streaming");
       break;
     default:
       break;
@@ -643,58 +679,53 @@ streamon_failed:
   }
 }
 
+/* Call with streamlock held, or when streaming threads are down */
 static void
 gst_v4l2_buffer_pool_streamoff (GstV4l2BufferPool * pool)
 {
+  GstBufferPoolClass *pclass = GST_BUFFER_POOL_CLASS (parent_class);
   GstV4l2Object *obj = pool->obj;
+  gint i;
+
+  if (!pool->streaming)
+    return;
 
   switch (obj->mode) {
     case GST_V4L2_IO_MMAP:
     case GST_V4L2_IO_USERPTR:
     case GST_V4L2_IO_DMABUF:
     case GST_V4L2_IO_DMABUF_IMPORT:
-      if (pool->streaming) {
-        if (obj->ioctl (pool->video_fd, VIDIOC_STREAMOFF, &obj->type) < 0)
-          GST_WARNING_OBJECT (pool, "STREAMOFF failed with errno %d (%s)",
-              errno, g_strerror (errno));
 
-        pool->streaming = FALSE;
+      if (obj->ioctl (pool->video_fd, VIDIOC_STREAMOFF, &obj->type) < 0)
+        GST_WARNING_OBJECT (pool, "STREAMOFF failed with errno %d (%s)",
+            errno, g_strerror (errno));
 
-        GST_DEBUG_OBJECT (pool, "Stopped streaming");
+      pool->streaming = FALSE;
 
-        if (pool->vallocator)
-          gst_v4l2_allocator_flush (pool->vallocator);
-      }
+      GST_DEBUG_OBJECT (pool, "Stopped streaming");
+
+      if (pool->vallocator)
+        gst_v4l2_allocator_flush (pool->vallocator);
       break;
     default:
       break;
   }
-}
-
-static GstFlowReturn
-gst_v4l2_buffer_pool_resurect_buffer (GstV4l2BufferPool * pool)
-{
-  GstBufferPoolAcquireParams params = { 0 };
-  GstBuffer *buffer = NULL;
-  GstFlowReturn ret;
-
-  GST_DEBUG_OBJECT (pool, "A buffer was lost, reallocating it");
-
-  /* block recursive calls to this function */
-  g_signal_handler_block (pool->vallocator, pool->group_released_handler);
 
-  params.flags =
-      (GstBufferPoolAcquireFlags) GST_V4L2_BUFFER_POOL_ACQUIRE_FLAG_RESURRECT |
-      GST_BUFFER_POOL_ACQUIRE_FLAG_DONTWAIT;
-  ret =
-      gst_buffer_pool_acquire_buffer (GST_BUFFER_POOL (pool), &buffer, &params);
+  for (i = 0; i < VIDEO_MAX_FRAME; i++) {
+    if (pool->buffers[i]) {
+      GstBuffer *buffer = pool->buffers[i];
+      GstBufferPool *bpool = GST_BUFFER_POOL (pool);
 
-  if (ret == GST_FLOW_OK)
-    gst_buffer_unref (buffer);
+      pool->buffers[i] = NULL;
 
-  g_signal_handler_unblock (pool->vallocator, pool->group_released_handler);
+      if (V4L2_TYPE_IS_OUTPUT (pool->obj->type))
+        gst_v4l2_buffer_pool_release_buffer (bpool, buffer);
+      else                      /* Don't re-enqueue capture buffer on stop */
+        pclass->release_buffer (bpool, buffer);
 
-  return ret;
+      g_atomic_int_add (&pool->num_queued, -1);
+    }
+  }
 }
 
 static gboolean
@@ -707,7 +738,7 @@ gst_v4l2_buffer_pool_start (GstBufferPool * bpool)
   GstCaps *caps;
   guint size, min_buffers, max_buffers;
   guint max_latency, min_latency, copy_threshold = 0;
-  gboolean can_allocate = FALSE;
+  gboolean can_allocate = FALSE, ret = TRUE;
 
   GST_DEBUG_OBJECT (pool, "activating pool");
 
@@ -838,12 +869,14 @@ gst_v4l2_buffer_pool_start (GstBufferPool * bpool)
   if (!pclass->start (bpool))
     goto start_failed;
 
-  if (!V4L2_TYPE_IS_OUTPUT (obj->type))
+  if (!V4L2_TYPE_IS_OUTPUT (obj->type)) {
     pool->group_released_handler =
         g_signal_connect_swapped (pool->vallocator, "group-released",
         G_CALLBACK (gst_v4l2_buffer_pool_resurect_buffer), pool);
+    ret = gst_v4l2_buffer_pool_streamon (pool);
+  }
 
-  return TRUE;
+  return ret;
 
   /* ERRORS */
 wrong_config:
@@ -877,9 +910,7 @@ static gboolean
 gst_v4l2_buffer_pool_stop (GstBufferPool * bpool)
 {
   GstV4l2BufferPool *pool = GST_V4L2_BUFFER_POOL (bpool);
-  GstBufferPoolClass *pclass = GST_BUFFER_POOL_CLASS (parent_class);
   gboolean ret;
-  gint i;
 
   GST_DEBUG_OBJECT (pool, "stopping pool");
 
@@ -897,21 +928,6 @@ gst_v4l2_buffer_pool_stop (GstBufferPool * bpool)
 
   gst_v4l2_buffer_pool_streamoff (pool);
 
-  for (i = 0; i < VIDEO_MAX_FRAME; i++) {
-    if (pool->buffers[i]) {
-      GstBuffer *buffer = pool->buffers[i];
-
-      pool->buffers[i] = NULL;
-
-      if (V4L2_TYPE_IS_OUTPUT (pool->obj->type))
-        gst_v4l2_buffer_pool_release_buffer (bpool, buffer);
-      else                      /* Don't re-enqueue capture buffer on stop */
-        pclass->release_buffer (bpool, buffer);
-
-      g_atomic_int_add (&pool->num_queued, -1);
-    }
-  }
-
   ret = GST_BUFFER_POOL_CLASS (parent_class)->stop (bpool);
 
   if (ret && pool->vallocator) {
@@ -950,65 +966,12 @@ static void
 gst_v4l2_buffer_pool_flush_stop (GstBufferPool * bpool)
 {
   GstV4l2BufferPool *pool = GST_V4L2_BUFFER_POOL (bpool);
-  GstV4l2Object *obj = pool->obj;
-  GstBuffer *buffers[VIDEO_MAX_FRAME];
-  gint i;
 
   GST_DEBUG_OBJECT (pool, "stop flushing");
 
-  /* If we haven't started streaming yet, simply call streamon */
-  if (!pool->streaming)
-    goto streamon;
-
   if (pool->other_pool)
     gst_buffer_pool_set_flushing (pool->other_pool, FALSE);
 
-  GST_OBJECT_LOCK (pool);
-  gst_v4l2_buffer_pool_streamoff (pool);
-  /* Remember buffers to re-enqueue */
-  memcpy (buffers, pool->buffers, sizeof (buffers));
-  memset (pool->buffers, 0, sizeof (pool->buffers));
-  GST_OBJECT_UNLOCK (pool);
-
-  /* Reset our state */
-  switch (obj->mode) {
-    case GST_V4L2_IO_RW:
-      break;
-    case GST_V4L2_IO_MMAP:
-    case GST_V4L2_IO_USERPTR:
-    case GST_V4L2_IO_DMABUF:
-    case GST_V4L2_IO_DMABUF_IMPORT:
-    {
-      for (i = 0; i < VIDEO_MAX_FRAME; i++) {
-        /* Re-enqueue buffers */
-        if (buffers[i]) {
-          GstBufferPool *bpool = (GstBufferPool *) pool;
-          GstBuffer *buffer = buffers[i];
-
-          /* Remove qdata, this will unmap any map data in
-           * userptr/dmabuf-import */
-          gst_mini_object_set_qdata (GST_MINI_OBJECT (buffer),
-              GST_V4L2_IMPORT_QUARK, NULL, NULL);
-
-          if (buffer->pool == NULL)
-            gst_v4l2_buffer_pool_release_buffer (bpool, buffer);
-
-          g_atomic_int_add (&pool->num_queued, -1);
-        }
-      }
-
-      break;
-    }
-    default:
-      g_assert_not_reached ();
-      break;
-  }
-
-streamon:
-  /* Start streaming on capture device only */
-  if (!V4L2_TYPE_IS_OUTPUT (obj->type))
-    gst_v4l2_buffer_pool_streamon (pool);
-
   gst_poll_set_flushing (pool->poll, FALSE);
 }
 
@@ -2037,3 +2000,17 @@ gst_v4l2_buffer_pool_copy_at_threshold (GstV4l2BufferPool * pool, gboolean copy)
   pool->enable_copy_threshold = copy;
   GST_OBJECT_UNLOCK (pool);
 }
+
+gboolean
+gst_v4l2_buffer_pool_flush (GstBufferPool * bpool)
+{
+  GstV4l2BufferPool *pool = GST_V4L2_BUFFER_POOL (bpool);
+  gboolean ret = TRUE;
+
+  gst_v4l2_buffer_pool_streamoff (pool);
+
+  if (!V4L2_TYPE_IS_OUTPUT (pool->obj->type))
+    ret = gst_v4l2_buffer_pool_streamon (pool);
+
+  return ret;
+}
diff --git a/sys/v4l2/gstv4l2bufferpool.h b/sys/v4l2/gstv4l2bufferpool.h
index eff7737fc..0fffc71c0 100644
--- a/sys/v4l2/gstv4l2bufferpool.h
+++ b/sys/v4l2/gstv4l2bufferpool.h
@@ -107,6 +107,8 @@ void                gst_v4l2_buffer_pool_set_other_pool (GstV4l2BufferPool * poo
 void                gst_v4l2_buffer_pool_copy_at_threshold (GstV4l2BufferPool * pool,
                                                             gboolean copy);
 
+gboolean            gst_v4l2_buffer_pool_flush   (GstBufferPool *pool);
+
 G_END_DECLS
 
 #endif /*__GST_V4L2_BUFFER_POOL_H__ */
diff --git a/sys/v4l2/gstv4l2transform.c b/sys/v4l2/gstv4l2transform.c
index feed7bf57..0c29f91ce 100644
--- a/sys/v4l2/gstv4l2transform.c
+++ b/sys/v4l2/gstv4l2transform.c
@@ -989,6 +989,8 @@ gst_v4l2_transform_sink_event (GstBaseTransform * trans, GstEvent * event)
       GST_DEBUG_OBJECT (self, "flush stop");
       gst_v4l2_object_unlock_stop (self->v4l2capture);
       gst_v4l2_object_unlock_stop (self->v4l2output);
+      gst_v4l2_buffer_pool_flush (self->v4l2output->pool);
+      gst_v4l2_buffer_pool_flush (self->v4l2capture->pool);
       break;
     default:
       break;
diff --git a/sys/v4l2/gstv4l2videodec.c b/sys/v4l2/gstv4l2videodec.c
index 8e369fae7..0f19cc691 100644
--- a/sys/v4l2/gstv4l2videodec.c
+++ b/sys/v4l2/gstv4l2videodec.c
@@ -282,6 +282,9 @@ gst_v4l2_video_dec_flush (GstVideoDecoder * decoder)
   gst_v4l2_object_unlock_stop (self->v4l2output);
   gst_v4l2_object_unlock_stop (self->v4l2capture);
 
+  gst_v4l2_buffer_pool_flush (self->v4l2output->pool);
+  gst_v4l2_buffer_pool_flush (self->v4l2capture->pool);
+
   return TRUE;
 }
 
diff --git a/sys/v4l2/gstv4l2videoenc.c b/sys/v4l2/gstv4l2videoenc.c
index ee89cec0c..c322f5dfd 100644
--- a/sys/v4l2/gstv4l2videoenc.c
+++ b/sys/v4l2/gstv4l2videoenc.c
@@ -218,6 +218,9 @@ gst_v4l2_video_enc_stop (GstVideoEncoder * encoder)
   gst_v4l2_object_stop (self->v4l2output);
   gst_v4l2_object_stop (self->v4l2capture);
 
+  gst_v4l2_buffer_pool_flush (self->v4l2output->pool);
+  gst_v4l2_buffer_pool_flush (self->v4l2capture->pool);
+
   if (self->input_state) {
     gst_video_codec_state_unref (self->input_state);
     self->input_state = NULL;