[Bug 737400] videoscale: Lanczos resizing for NV image format

[GStreamer (bugzilla.gnome.org)]

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

--- Comment #6 from Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> 2014-09-26 13:30:37 UTC ---
Review of attachment 287133:
 --> (https://bugzilla.gnome.org/review?bug=737400&attachment=287133)

This is a lot of copy paste to my taste but seems to respect the existing code
style. Ravi or Luis, could you please provide a patch to fix the review left
over.

::: gst/videoscale/vs_lanczos.c
@@ +220,3 @@
+vs_image_scale_lanczos_NV_int16 (const VSImage * dest, const VSImage * src,
+    uint8_t * tmpbuf, double sharpness, gboolean dither, double a,
+    double sharpen);

Style: type and declaration should be on same line.

@@ +753,3 @@
+  const tap_type *tapsline; \
+  int offset; \
+  if (_shift > 0) offset = (1<<_shift)>>1; \

Style: Space before after operators

@@ +756,3 @@
+  else offset = 0; \
+  for (i = 0; i < n; i++) { \
+    srcline = src + 2*offsets[i]; \

Style: Space before after operators

@@ +765,3 @@
+    } \
+    dest[i*2+0] = (sum1 + offset) >> _shift; \
+    dest[i*2+1] = (sum2 + offset) >> _shift; \

I know this is copy pasted, but doing multiplication in index is far from ideal
for performance. Aslo, you compute k*2 and i*2 twice. (Note, k * 2 == k << 1).
There is also style issues with operators missing space in between.

@@ +898,2 @@
   } \
 }

Normally we try and split the unrelated style fix from our patches.

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