[FFmpeg-devel] [PATCH] avfilter/vf_avgblur_opencl: fix error when clSetKernelArg fails

[Mark Thompson]

On 25/03/18 13:42, dylanf123 at gmail.com wrote:
> From: drfer3 <drfer3 at student.monash.edu>
> 
> Fixes Coverity CID 1430382
> ---> Following is a patch attempting to fix the err issue. It returns -1 if any
> clSetKernelArg() fails. Is this good, or should I be using a different
> return value for this error?

-1 shouldn't be used (since it maps to errno values, it looks like EPERM on Linux, and different things on other systems).

Given that would be quite unexpected if it failed, I think AVERROR_UNKNOWN is appropriate here - see <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavutil/error.h#l71>.

Patch looks fine with that changed.

Thanks,

- Mark


>  libavfilter/vf_avgblur_opencl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libavfilter/vf_avgblur_opencl.c b/libavfilter/vf_avgblur_opencl.c
> index 5ee66c0ba2..09caa1fd4f 100644
> --- a/libavfilter/vf_avgblur_opencl.c
> +++ b/libavfilter/vf_avgblur_opencl.c
> @@ -155,18 +155,21 @@ static int avgblur_opencl_filter_frame(AVFilterLink *inlink, AVFrame *input)
>          if (cle != CL_SUCCESS) {
>              av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
>                     "destination image argument: %d.\n", cle);
> +            err = -1;
>              goto fail;
>          }
>          cle = clSetKernelArg(ctx->kernel_horiz, 1, sizeof(cl_mem), &src);
>          if (cle != CL_SUCCESS) {
>              av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
>                     "source image argument: %d.\n", cle);
> +            err = -1;
>              goto fail;
>          }
>          cle = clSetKernelArg(ctx->kernel_horiz, 2, sizeof(cl_int), &radius_x);
>          if (cle != CL_SUCCESS) {
>              av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
>                     "sizeX argument: %d.\n", cle);
> +            err = -1;
>              goto fail;
>          }
>  
> @@ -191,18 +194,21 @@ static int avgblur_opencl_filter_frame(AVFilterLink *inlink, AVFrame *input)
>          if (cle != CL_SUCCESS) {
>              av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
>                     "destination image argument: %d.\n", cle);
> +            err = -1;
>              goto fail;
>          }
>          cle = clSetKernelArg(ctx->kernel_vert, 1, sizeof(cl_mem), &inter);
>          if (cle != CL_SUCCESS) {
>              av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
>                     "source image argument: %d.\n", cle);
> +            err = -1;
>              goto fail;
>          }
>          cle = clSetKernelArg(ctx->kernel_vert, 2, sizeof(cl_int), &radius_y);
>          if (cle != CL_SUCCESS) {
>              av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
>                     "sizeY argument: %d.\n", cle);
> +            err = -1;
>              goto fail;
>          }
>  
>