[Bug 722127] v4l2: Add NV12_64Z32 support

[GStreamer (bugzilla.gnome.org)]

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

Sebastian Dröge (slomo) <slomo> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #266189|none                        |reviewed
             status|                            |

--- Comment #2 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2014-01-14 09:48:46 UTC ---
Review of attachment 266189:
 --> (https://bugzilla.gnome.org/review?bug=722127&attachment=266189)

Generally looks good but some weird things...

::: sys/v4l2/gstv4l2bufferpool.c
@@ +58,3 @@

+#ifndef ALIGN
+#define ALIGN(value,n) ((((value) + ((n) - 1)) & ~((n) - 1)))

Maybe we should get an utility macro for that in gstutils.h

@@ +448,3 @@
+            tile_height = 1 << hs;
+
+            x_tiles = obj->bytesperline[i] >> ws;

Isn't the tilesize in components (or pixels?) while bytesperline is in bytes?
You mix units here then without conversion

@@ +450,3 @@
+            x_tiles = obj->bytesperline[i] >> ws;
+            y_tiles = GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT (finfo, i,
+                ALIGN (height, tile_height) >> hs);

But not here


But maybe all this needs rounding up instead of the rounding down division with
>> ?

@@ +568,3 @@
+      if (GST_VIDEO_FORMAT_INFO_IS_TILED (obj->info.finfo)) {
+        stride = GST_VIDEO_TILE_X_TILES (stride) <<
+            GST_VIDEO_FORMAT_INFO_TILE_WS ((obj->info.finfo));

And here any rounding needed or not?

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