[FFmpeg-devel] [PATCH] lavfi: add helper macro for OpenCL error handling.

[Song, Ruiling]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of
> Mark Thompson
> Sent: Monday, June 18, 2018 2:17 AM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi: add helper macro for OpenCL error
> handling.
> 
> On 15/06/18 03:36, Dan Yaschenko wrote:
> > On 12 June 2018 at 10:20, Ruiling Song <ruiling.song at intel.com> wrote:
> >
> >> Signed-off-by: Ruiling Song <ruiling.song at intel.com>
> >> ---
> >> I am not sure whether do you think this would be useful?
> >> the main purpose is to make OpenCL error check code simpler.
> >> If we think this is good, I can go to replace current
> >> OpenCL filters to use this macro.
> >
> >
> >> for example:
> >>         if (cle != CL_SUCCESS) {
> >>             av_log(avctx, AV_LOG_ERROR, "Failed to enqueue kernel: %d.\n",
> >>                    cle);
> >>             err = AVERROR(EIO);
> >>             goto fail;
> >>         }
> >> can be replaced with:
> >> OCL_FAIL_ON_ERR(avctx, cle, AVERROR(EIO), "Failed to enqueue kernel:
> >> %d.\n", cle);
> >>
> >> Thanks!
> >> Ruiling
> >>  libavfilter/opencl.h | 11 +++++++++++
> >>  1 file changed, 11 insertions(+)
> >>
> >> diff --git a/libavfilter/opencl.h b/libavfilter/opencl.h
> >> index c0a4519..c33df1c 100644
> >> --- a/libavfilter/opencl.h
> >> +++ b/libavfilter/opencl.h
> >> @@ -97,5 +97,16 @@ int
> ff_opencl_filter_work_size_from_image(AVFilterContext
> >> *avctx,
> >>                                            size_t *work_size,
> >>                                            AVFrame *frame, int plane,
> >>                                            int block_alignment);
> >> +/**
> >> + * A helper macro to handle OpenCL error. It will assign errcode to
> >> + * variable err, log error msg, and jump to fail label on error.
> >> + */
> >> +#define OCL_FAIL_ON_ERR(logctx, cle, errcode, ...) do {\
> >> +    if (cle != CL_SUCCESS) {\
> >> +        av_log(logctx, AV_LOG_ERROR, __VA_ARGS__);\
> >> +        err = errcode;\
> >> +        goto fail;\
> >> +    }\
> >> +} while(0)
> >>
> >>  #endif /* AVFILTER_OPENCL_H */
> >> --
> >> 2.7.4
> >>
> >
> > Hi!
> > I like your idea, but still don't know which one is better.
> > My idea relies on fact, that there are only few OpenCL functions which are
> > used multiple times in filters:
> > setKernelArg, clCreateKernel(in case when there are multiple kernels) and
> > maybe
> > clEnqueueNDRangeKernel. So that is why my purpose is totally wrap them and
> > significantly reduce code,
> > but yes, there are some restrictions, like you can't use kernel_arg++ when
> > setting kernel arguments.
> > And still most of cl-error checking statements appear after using
> > cl-functions listed above.
> 
> The specific one for kernel args is the largest source of extra boilerplate, so I've
> applied it already.  (Even with the more general setup it would still make sense
> for a separate macro to exist to do the kernel arguments.)
> 
> This more general case seems reasonable to me if you'd like to implement it.  To
> make it slightly simpler, it's probably ok to assume that the first two arguments
> have the fixed names used in all current filters ("avctx" for the AVFilterContext,
> "cle" for the CL error code we want to check).  I'd probably also make it
> "CL_FAIL_ON_ERROR" rather than "OCL_FAIL_ON_ERR" to better match
> existing style.
Ok, I will submit a new patch later.

Ruiling
> 
> Thanks,
> 
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel