diff mbox

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

Message ID 20180325124253.36091-1-dylanf123@gmail.com
State Superseded
Headers show

Commit Message

Dylan Fernando March 25, 2018, 12:42 p.m. UTC 
From: drfer3 <drfer3@student.monash.edu>

Fixes Coverity CID 1430382
---
 libavfilter/vf_avgblur_opencl.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Mark Thompson March 25, 2018, 6:04 p.m. UTC | #1 
On 25/03/18 13:42, dylanf123@gmail.com wrote:
> From: drfer3 <drfer3@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;
>          }
>  
>
Dylan Fernando March 26, 2018, 12:08 a.m. UTC | #2 
Thanks, fixed

- Dylan
diff mbox

Patch

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;
         }