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

[Michael Niedermayer]

On Mon, Apr 22, 2013 at 05:48:19PM +0800, Wei Gao wrote:
> 2013/4/22 Michael Niedermayer <michaelni at gmx.at>
> 
> > On Mon, Apr 22, 2013 at 10:21:59AM +0200, Stefano Sabatini wrote:
> > > On date Sunday 2013-04-21 17:10:38 +0800, Wei Gao encoded:
> > [...]
> > > > >
> > > > > > +        return AVERROR_EXTERNAL;
> > > > >                                     \
> > > > > > +    }
> > > > > > +
> > > > > > +#define UNSHARP_OPENCL_SET_KERNEL_ARG(arg_ptr)
> > > > >                                     \
> > > > > > +    status =
> > > > >
> > clSetKernelArg((kernel),(arg_no++),(sizeof(arg_ptr)),(void*)(&(arg_ptr)));
> > > > >                    \
> > > > > > +    if (status != CL_SUCCESS) {
> > > > >                                    \
> > > > > > +        av_log(ctx, AV_LOG_ERROR, "cannot set kernel argument:
> > %d\n",
> > > > > status );                            \
> > > > > > +        return AVERROR_EXTERNAL;
> > > > >                                     \
> > > > > > +    }
> > > > >
> > > > > These macros are mostly duplicated from deshake_opencl.c, so we think
> > > > > to factorize them to a common file (libavfilter/opencl_internal.h
> > > > > could be fine).
> > > > >
> > > > Yes, I agree with you, I defined them in the previous patch but there
> > seems
> > > > some problem because the parameter status, kernel, arg_no must be
> >  declared
> > > > in the function which use the macro. Can you give me some advice?
> > Thanks.
> > >
> > > Don't know, in case the macro is going to be used in other parts of
> > > the code (as I suspect), then maybe it makes sense to define them in a
> > > public header in libavutil. The problem with that approach is than we
> > > can't change it later, so another approach would be to use an internal
> > > header (libavutil/opencl_internal.h) which is not published and used
> > > only internally. Michael can probably give a better advice.
> >
> > an internal header could be used but iam not sure if the macro itself
> > is a good idea.
> > Using a function instead could be more readable.
> > The input to the function could be an array of all the parameters to
> > set
> 
> not only the parameter, but also the parameters type size, if set them to a
> array, it seems not convenient to set the value.

a array of a structure containing the size & value is a possibility.
For the value an union can be used

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130422/ee163c1c/attachment.asc>