GNOME Bugzilla – Bug 735032
zebrastripe: Refactoring of common code
Last modified: 2014-09-01 14:11:02 UTC
Created attachment 283859 [details] [review] Refactored ZebraStripe element In GstZebraStripe element, there are three functions gst_zebra_stripe_transform_frame_ip_planarY gst_zebra_stripe_transform_frame_ip_YUY2 gst_zebra_stripe_transform_frame_ip_AYUV which does the same functionality except for the offset values The code is duplicated in all 3 functions. Hence moving the code to a common place and just changing the offset values based on the CAPS.
Review of attachment 283859 [details] [review]: Nice catch, some comments for more readability below. ::: gst/videofilters/gstzebrastripe.c @@ +221,3 @@ case GST_VIDEO_FORMAT_Y444: case GST_VIDEO_FORMAT_Y42B: + data_a = 1; GST_VIDEO_FORMAT_INFO_PSTRIDE can be used to get those values for data_a @@ +241,3 @@ + (guint8 *) frame->data[0] + frame->info.stride[0] * j + offset; + for (i = 0; i < width; i++) { + if (data[data_a * i + data_b] >= threshold) { How about using more significative names for data_a and data_b. data_a seems to be the pixel stride and data_b the index of the Y inside the pixel. Maybe something like pixel_stride and y_position or similar.
Created attachment 284475 [details] [review] Made changes as per the review comments. Hi Thiago, Thanks for the review Have made changes as per the review comments. Please review the same.
Review of attachment 284475 [details] [review]: Code looks good now, could you please elaborate a better commit message? If you look at other commits you will figure out the pattern used in gstreamer. It is something like: elementname: short title long description (multiple lines) bugzilla bug url (on the last line)
Created attachment 284968 [details] [review] Patch with proper commit comment Edited the commit message as per the norm followed for Gstreamer.
Thanks for the update. commit 114ee3355ad6db075c230f26ed92da84953a6def Author: Vineeth T M <vineeth.tm@samsung.com> Date: Mon Sep 1 08:24:57 2014 +0530 zebrastripe: Refactor to remove duplicate code gst_zebra_stripe_transform_frame_ip_planarY gst_zebra_stripe_transform_frame_ip_YUY2 gst_zebra_stripe_transform_frame_ip_AYUV all above 3 functions do the same functionality except for offset and pixel stride. Hence moving the functionality to a single funtion. https://bugzilla.gnome.org/show_bug.cgi?id=735032