[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

Olivier Crete (Tester) <olivier.crete> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #252279|none                        |needs-work
             status|                            |

--- Comment #14 from Olivier Crete (Tester) <olivier.crete at ocrete.ca> 2013-09-18 17:14:43 UTC ---
Review of attachment 252279:
 --> (https://bugzilla.gnome.org/review?bug=706083&attachment=252279)

I think we should put something along those lines in 1.2.

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

This will break v4l2sink I think, it needs to know how much of the buffer is
valid when it is queued.

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

Why the extra {} ?

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

Didn't you already replace the memory when doing the qbuf ?

@@ +894,3 @@

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

Why not copy the metadata also ? Don't you want GST_BUFFER_COPY_ALL |
GST_BUFFER_COPY_DEEP ?

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

What are you using from glib-compat-private ?

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

Why do you add a new lock here? Any reason to not use the GstObject lock ?

-- 
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.