[FFmpeg-devel] [PATCH] lavfi: add erosion_opencl, dilation_opencl filters

[Mark Thompson]

On 09/08/18 03:41, Danil Iashchenko wrote:
> Add erosion_opencl, dilation_opencl filters. Behave like existing erosion and dilation filters.
> ---
> 
> Thanks! Fixed.
> 
>  configure                        |   2 +
>  libavfilter/Makefile             |   4 +
>  libavfilter/allfilters.c         |   2 +
>  libavfilter/opencl/neighbor.cl   |  87 +++++++++++
>  libavfilter/opencl_source.h      |   1 +
>  libavfilter/vf_neighbor_opencl.c | 327 +++++++++++++++++++++++++++++++++++++++
>  6 files changed, 423 insertions(+)
>  create mode 100644 libavfilter/opencl/neighbor.cl
>  create mode 100644 libavfilter/vf_neighbor_opencl.c
> 
> ...
> diff --git a/libavfilter/vf_neighbor_opencl.c b/libavfilter/vf_neighbor_opencl.c
> new file mode 100644
> index 0000000..7ba759b
> --- /dev/null
> +++ b/libavfilter/vf_neighbor_opencl.c
> ...
> +static int neighbor_opencl_make_filter_params(AVFilterContext *avctx)
> +{
> +    NeighborOpenCLContext *ctx = avctx->priv;
> +    int *matrix = NULL;
> +    size_t matrix_bytes;
> +    cl_mem buffer;
> +    cl_int cle;
> +    int i;
> +
> +    for (i = 0; i < 4; i++) {
> +        ctx->threshold[i] /= 255.0;
> +    }
> +    for (i = 0; i < 9; i++) {
> +        matrix_bytes = sizeof(float)*9;

sizeof(int)?

> +        matrix = av_malloc(matrix_bytes);
> +    }

I don't think you want a loop around this?  It allocates extra instances of matrix which never get freed.

Since it's small it might be easier to just put "cl_int matrix[9]" on the stack.

> +    if (!matrix) {
> +        av_freep(&matrix);
> +        return AVERROR(ENOMEM);
> +    }
> +    matrix[4] = 0;
> +    for (i = 0; i < 8; i++) {
> +        if (ctx->coordinates & (1 << i)) {
> +            matrix[i > 3 ? i + 1: i] = 1;
> +        }
> +    }
> +    buffer = clCreateBuffer(ctx->ocf.hwctx->context,
> +                            CL_MEM_READ_ONLY |
> +                            CL_MEM_COPY_HOST_PTR |
> +                            CL_MEM_HOST_NO_ACCESS,
> +                            matrix_bytes, matrix, &cle);

This buffer is never released.

> +    if (!buffer) {
> +        av_log(avctx, AV_LOG_ERROR, "Failed to create matrix buffer: "
> +               "%d.\n", cle);
> +        av_freep(&matrix);
> +        return AVERROR(EIO);
> +    }
> +    ctx->coord = buffer;
> +    av_freep(&matrix);
> +
> +    return 0;
> +}
> ...

Tested and has identical output to existing dilation/erosion filters, so otherwise LGTM.

Thanks,

- Mark