[Bug 706083] v4l2src: UVC Allocated buffers wrapped in GstBuffer get orphaned by GstBuffer API

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=706083
  GStreamer | gst-plugins-good | git

--- Comment #15 from Robert Krakora <rob.krakora at messagenetsystems.com> 2013-09-18 18:41:37 UTC ---
Review of attachment 252279:
 --> (https://bugzilla.gnome.org/review?bug=706083&attachment=252279)

::: sys/v4l2/gstv4l2bufferpool.c
@@ +666,2 @@
   index = meta->vbuffer.index;
+  meta->vbuffer.bytesused = 0;

You are correct.  Don't know why I did that.

@@ +675,3 @@
+  if (pool->buffers[index] != NULL) {
+      goto already_queued;
+  }

The {} can be removed.

@@ +686,3 @@
   pool->buffers[index] = buf;
   pool->num_queued++;
+  GST_V4L2_BUFFER_POOL_UNLOCK (pool);

GST_V4L2_BUFFER_POOL_UNLOCK (pool) not needed.

@@ +736,3 @@
+  }
+
+  GST_V4L2_BUFFER_POOL_LOCK (pool);

GST_V4L2_BUFFER_POOL_LOCK (pool) not needed.

@@ +743,3 @@
   outbuf = pool->buffers[vbuffer.index];
+  if (outbuf == NULL) {
+    GST_V4L2_BUFFER_POOL_UNLOCK (pool);

GST_V4L2_BUFFER_POOL_UNLOCK (pool) not needed and {} can be removed.

@@ +775,3 @@
+        gst_memory_new_wrapped (GST_MEMORY_FLAG_NO_SHARE,
+            meta->mem, vbuffer.bytesused, 0, vbuffer.bytesused, NULL,
+            NULL));

This resizes without replacing the dequeued buffer as does gst_buffer_resize().
 I believe that there is work underway to prevent the replacement of wrapped
memory by the GstBuffer class.  This is a temporary workaround.

@@ +784,1 @@


GST_V4L2_BUFFER_POOL_UNLOCK (pool) not needed and {} can be removed.

@@ +894,3 @@

+            /* copy the buffer with memory, but not the meta data */
+            copy = gst_buffer_copy_region (*buffer, GST_BUFFER_COPY_MEMORY, 0,
-1);

Trying to be efficient...gst_buffer_copy() can be used instead as it was in the
original source.

* To efficiently create a smaller buffer out of an existing one, you can
 * use gst_buffer_copy_region(). This method tries to share the memory objects
 * between the two buffers.

@@ +1006,3 @@
+                gst_memory_new_wrapped (GST_MEMORY_FLAG_NO_SHARE,
+                    meta->mem, meta->vbuffer.length, 0, meta->vbuffer.length,
NULL,
+                    NULL));

This resizes without replacing the dequeued buffer as does gst_buffer_resize().
 I believe that there is work underway to prevent the replacement of wrapped
memory by the GstBuffer class.  This is a temporary workaround.

@@ +1054,3 @@
 gst_v4l2_buffer_pool_init (GstV4l2BufferPool * pool)
 {
+  g_mutex_init(&pool->lock);

g_mutex_init (&pool->lock) not needed.

::: sys/v4l2/gstv4l2bufferpool.h
@@ +34,2 @@
 #include "gstv4l2object.h"
+#include <gst/glib-compat-private.h>

For the added lock which is not needed.

@@ +67,2 @@
   GstBuffer **buffers;
+  GMutex lock;

It is not needed.

@@ +93,3 @@

+#define GST_V4L2_BUFFER_POOL_LOCK(pool)     g_mutex_lock (&(pool)->lock)
+#define GST_V4L2_BUFFER_POOL_UNLOCK(pool)   g_mutex_unlock (&(pool)->lock)

These are not needed.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.