> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of > Mark Thompson > Sent: Monday, June 18, 2018 2:17 AM > To: ffmpeg-devel@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.s...@intel.com> wrote: > > > >> Signed-off-by: Ruiling Song <ruiling.s...@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@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
