[FFmpeg-devel] [PATCH] libavfilter/unsharp: add opencl unsharp filter

[Wei Gao]

Hi, some explanations are as follows:

2013/4/25 Stefano Sabatini <stefasab at gmail.com>

> On date Wednesday 2013-04-24 12:48:23 +0800, Wei Gao encoded:
> > Hi
> >
> > Thanks for reviewing, the patches is modified according the comments
> >
> > Thanks
> >
> >
> > 2013/4/24 Stefano Sabatini <stefasab at gmail.com>
> >
> > > On date Tuesday 2013-04-23 14:30:41 +0800, Wei Gao encoded:
> > > [...]
> > > > From 7eaeb25facbfae38cf6e13d074be8eecdb669df7 Mon Sep 17 00:00:00
> 2001
> > > > From: highgod0401 <highgod0401 at gmail.com>
> > > > Date: Tue, 23 Apr 2013 14:26:23 +0800
> > > > Subject: [PATCH 1/2] lavu/opencl:add opencl set param function
> > > >
> > >
> > > Overall, very nice work.
> > > --
> > > FFmpeg = Forgiving & Formidable Moronic Ponderous Enhanced Gadget
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
>
> > From a4cac216966100ed594a93ef6d2a544be8dcad0e Mon Sep 17 00:00:00 2001
> > From: highgod0401 <highgod0401 at gmail.com>
> > Date: Wed, 24 Apr 2013 12:41:08 +0800
> > Subject: [PATCH 1/2] lavu/opencl:add opencl set param function
> >
> > ---
> > 1.7.11.msysgit.1
>
> LGTM, thanks.
>
> > From d0460e4218739d1f2bac6d6cd5bdfcdfd8a4f877 Mon Sep 17 00:00:00 2001
> > From: highgod0401 <highgod0401 at gmail.com>
> > Date: Wed, 24 Apr 2013 12:45:10 +0800
> > Subject: [PATCH 2/2] lavfi/unsharp: add opencl unsharp filter
> >
> > ---
> >  doc/filters.texi                |   5 +
> >  libavfilter/Makefile            |   2 +-
> >  libavfilter/opencl_allkernels.c |   3 +
> >  libavfilter/unsharp.h           |  78 +++++++++++
> >  libavfilter/unsharp_kernel.h    | 132 +++++++++++++++++++
> >  libavfilter/unsharp_opencl.c    | 281
> ++++++++++++++++++++++++++++++++++++++++
> >  libavfilter/unsharp_opencl.h    |  34 +++++
> >  libavfilter/vf_unsharp.c        |  88 ++++++++-----
>
> A micro bump in libavfilter/version.h may be nice for library users.
>
> LIBAVFILTER_VERSION_MINOR + 1?

>
> >  #endif
> >
> >  #define OPENCL_REGISTER_KERNEL_CODE(X, x)
>                \
>

> > +        if (res & (~0xFF))
> > +            temp_dst[x + y * temp_dst_stride] = (-res) >> 31;
> > +        else
> > +            temp_dst[x + y * temp_dst_stride] = res;
>
> I still find this a bit obfuscated. Why not a simple:
> if (res > 255)
>    res = 255;
> temp_dst[x + y * temp_dst_stride] = res;
>
> or
> temp_dst[x + y * temp_dst_stride] = res > 255 ? 255 : res;
> ?
>
> if I use this, the kernel md5 code is not the same as the C code, but I
think the original code is not complicate.

> > +    } else {
> > +        temp_dst[x + y * temp_dst_stride] = temp_src[x + y *
> temp_src_stride];
> > +    }
>
>
> [...]
>
> LGTM otherwise, thanks.
> --
> FFmpeg = Free & Freak Mysterious Peaceful Experimenting Gangster
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>