[Bug 719791] v4l2sink: enable zero-copy path between mfcdec and v4l2sink io-mode=mmap

[GStreamer (bugzilla.gnome.org)]

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

Nicolas Dufresne <nicolas.dufresne> changed:

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

--- Comment #10 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2013-12-03 20:07:32 UTC ---
Review of attachment 263414:
 --> (https://bugzilla.gnome.org/review?bug=719791&attachment=263414)

::: sys/v4l2/gstv4l2bufferpool.c
@@ +478,3 @@
+      /* do padding and alignment */
+      /* FIXME: should call it but it set wrong values
+       * gst_video_info_align (&obj->info, &pool->align); */

I don't think we need to call this, but it should not return wrong value
(unless the alignment value are wrong).

Also we don't check if V4L2 respect the stride alignment. We can't decide it,
but we should at least fail the pool activation if we are not aligned.

@@ +660,3 @@
   gst_poll_set_flushing (obj->poll, FALSE);

+  GST_V4L2_SET_ACTIVE (obj);

Why and is it related ?

::: sys/v4l2/gstv4l2object.c
@@ +2461,3 @@
+
+    v4l2object->update_crop = TRUE;
+  }

I think it would be a bit cleaner to set the width and height for what is
written in the info, and then += what's missing ?

@@ +2702,2 @@
   /* now configure ther pools */
+  if (!v4l2object->pool && !gst_v4l2_object_setup_pool (v4l2object, caps))

Hmm, that seems wrong. If caps have changed you won't update the caps in the
pool parameter.

::: sys/v4l2/gstv4l2sink.c
@@ +702,3 @@
+    obj->update_crop = FALSE;
+  }
+

You should not change what has been set in the property. Buffer pools cropping
is mandatory and relative to the configured size, while the property cropping
is optional and relative to the caps size. Both cropping need to be combined,
right now the property cropping is overridden.

We also ignore extra cropping set in the GstVideoCropMeta on the buffer. I
think the extra cropping there should be overridden by the value set in the
properties.

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