[Bug 687964] videocrop: Add NV12/NV21 support

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=687964
  GStreamer | gst-plugins-good | 1.x

--- Comment #3 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2012-11-09 12:20:46 UTC ---
(In reply to comment #2)
> Review of attachment 228528 [details]:
> 
> ::: gst/videocrop/gstvideocrop.c
> @@ +392,3 @@
> +  /* UV plane */
> +  uv_in = y_in + GST_VIDEO_FRAME_COMP_OFFSET (in_frame, 1);
> +  uv_out = y_out + GST_VIDEO_FRAME_COMP_OFFSET (out_frame, 1);
> 
> Use GST_VIDEO_FRAME_PLANE_DATA() here too. The components are always in order
> Y, U, V, A while what you want is the offset to the second plane (no matter if
> U or V is the first component in memory).
> 
> Also the Y and UV planes could be in different memory areas.

Good point, this is a porting error.

> 
> @@ +404,3 @@
> +  }
> +
> +  uv_in += (vcrop->crop_top / 2) * GST_VIDEO_FRAME_PLANE_STRIDE (in_frame, 1);
> 
> Round down this one too maybe?

Dividing by two already round down, also you don't need (vcrop->crop_top / 2)
to be even. Unless I don't understand what you mean ?

> 
> @@ +408,3 @@
> +  dx = GST_ROUND_UP_2 (width);
> +
> +  for (i = 0; i < GST_ROUND_UP_2 (height) / 2; i++) {
> 
> Maybe put this round-divide calculation out of the loop
> 
> ::: gst/videocrop/gstvideocrop.h
> @@ +40,3 @@
>    VIDEO_CROP_PIXEL_FORMAT_PACKED_COMPLEX,     /* UYVY, YVYU */
> +  VIDEO_CROP_PIXEL_FORMAT_PLANAR,             /* I420, YV12 */
> +  VIDEO_CROP_PIXEL_FORMAT_BIPLANAR            /* NV12, NV21 */
> 
> Maybe call this semi-planar instead... first read bipolar here ;)

Biplanar was from VLC doc I have read, but semi-planar seems more common. I'll
change that.

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