[Bug 745151] videofilter: add sharpening filter

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=745151

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

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

--- Comment #17 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 309131:
 --> (https://bugzilla.gnome.org/review?bug=745151&attachment=309131)

Can you split this into two patches, one to add the base class and one for the
sharpen filter?

::: gst/videofilters/gstsharpen.c
@@ +89,3 @@
+    -1, -1, -1,
+    -1, 18, -1,
+    -1, -1, -1

Should the "strength" of the filter be configurable by property?

::: gst/videofilters/gstvideobaseconvolvefilter.c
@@ +28,3 @@
+
+#define ALLOWED_CAPS \
+    "{NV12, NV21, RGB}"

You can easily support all other RGB/xRGB/ARGB (in all orders) formats, and
also I420/Y444/Y42B/Y41B/YV12. Can you add support for these?

@@ +62,3 @@
+GstFlowReturn
+gst_video_base_convolve (GstVideoFilter * video_filter, GstVideoFrame *
pInput,
+    GstVideoFrame * pOutput)

We use a different style to name variables: lower-case, underscore separated
words and the type (p / pointer) is not part of the name

@@ +109,3 @@
+  }
+
+  switch (pInput->info.finfo->format) {

Try to move the format specific processing code into separate functions

@@ +112,3 @@
+      /* Gray Scale, YUV Data */
+    case GST_VIDEO_FORMAT_NV12:
+    case GST_VIDEO_FORMAT_NV21:

The two planes might have different strides

@@ +143,3 @@
+      /* Get Pointer for UV plane and memcpy remaining data */
+      pData = (guint8 *) pInput->data[1];
+      pOutp = (guint8 *) pOutput->data[1];

Use the accessor macros (here and elsewhere) to access the fields of the
GstVideoFrame

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.