[FFmpeg-devel] [PATCH] lavfi/unsharp: fix opencl crushed on 64bit linux

[Stefano Sabatini]

On date Friday 2013-05-03 15:50:59 +0800, Wei Gao encoded:
> 

Replace "crushed" with "crash" in the patch subject line.

> From 767ac68cb1734b9320b615b2a8e112ebe7add102 Mon Sep 17 00:00:00 2001
> From: highgod0401 <highgod0401 at gmail.com>
> Date: Fri, 3 May 2013 15:46:57 +0800
> Subject: [PATCH] lavfi/unsharp: fix opencl crushed on 64bit linux
> 
> ---
>  libavfilter/unsharp_opencl.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/libavfilter/unsharp_opencl.c b/libavfilter/unsharp_opencl.c
> index e9a4c93..3de878e 100644
> --- a/libavfilter/unsharp_opencl.c
> +++ b/libavfilter/unsharp_opencl.c
> @@ -51,7 +51,7 @@ static int compute_mask(int step, uint32_t *mask)
>          ret = AVERROR(ENOMEM);
>          goto end;
>      }
> -    counter = av_mallocz(counter_size);
> +    counter = av_mallocz(sizeof(uint32_t *) * (2 * step + 1));
>      if (!counter) {
>          ret = AVERROR(ENOMEM);
>          goto end;

This hunk seems correct (uh embarassing bug I overlooked).

> @@ -88,13 +88,15 @@ static int compute_mask_matrix(cl_mem cl_mask_matrix, int step_x, int step_y)
>  {
>      int i, j, ret = 0;
>      uint32_t *mask_matrix, *mask_x, *mask_y;
> -    size_t size_matrix = sizeof(uint32_t) * (2 * step_x + 1) * (2 * step_y + 1);
> -    mask_x = av_mallocz(sizeof(uint32_t) * (2 * step_x + 1));
> +    size_t size_x = sizeof(uint32_t) * (2 * step_x + 1);
> +    size_t size_y = sizeof(uint32_t) * (2 * step_y + 1);
> +    size_t size_matrix = size_x * size_y;
> +    mask_x = av_mallocz(size_x);
>      if (!mask_x) {
>          ret = AVERROR(ENOMEM);
>          goto end;
>      }
> -    mask_y = av_mallocz(sizeof(uint32_t) * (2 * step_y + 1));
> +    mask_y = av_mallocz(size_y);
>      if (!mask_y) {
>          ret = AVERROR(ENOMEM);
>          goto end;
> @@ -200,21 +202,24 @@ int ff_opencl_apply_unsharp(AVFilterContext *ctx, AVFrame *in, AVFrame *out)
>  
>  int ff_opencl_unsharp_init(AVFilterContext *ctx)
>  {
> -    int ret = 0;
> +    int ret = 0, i;
>      UnsharpContext *unsharp = ctx->priv;
> +    size_t size_x[2], size_y[2];
> +    cl_mem *mask_matrix[2];
> +    mask_matrix[0] = &unsharp->opencl_ctx.cl_luma_mask;
> +    mask_matrix[1] = &unsharp->opencl_ctx.cl_chroma_mask;
> +    size_x[0] = sizeof(uint32_t) * (2 * unsharp->luma.steps_x + 1);
> +    size_x[1] = sizeof(uint32_t) * (2 * unsharp->chroma.steps_x + 1);
> +    size_y[0] = sizeof(uint32_t) * (2 * unsharp->luma.steps_y + 1);
> +    size_y[1] = sizeof(uint32_t) * (2 * unsharp->chroma.steps_y + 1);
>      ret = av_opencl_init(NULL);
>      if (ret < 0)
>          return ret;
> -    ret = av_opencl_buffer_create(&unsharp->opencl_ctx.cl_luma_mask,
> -                                  sizeof(uint32_t) * (2 * unsharp->luma.steps_x + 1) * (2 * unsharp->luma.steps_y + 1),
> -                                  CL_MEM_READ_ONLY, NULL);
> -    if (ret < 0)
> -        return ret;
> -    ret = av_opencl_buffer_create(&unsharp->opencl_ctx.cl_chroma_mask,
> -                                  sizeof(uint32_t) * (2 * unsharp->chroma.steps_x + 1) * (2 * unsharp->chroma.steps_y + 1),
> -                                  CL_MEM_READ_ONLY, NULL);
> -    if (ret < 0)
> -        return ret;
> +    for(i = 0; i < 2; i++) {

Nit++: for_(

Also this looks like unrelated refactoring, right? In this case it
would be better to put it in a separate patch.

> +        ret = av_opencl_buffer_create(mask_matrix[i], size_x[i] * size_y[i], CL_MEM_READ_ONLY, NULL);
> +        if (ret < 0)
> +            return ret;
> +    }
>      ret = generate_mask(ctx);
>      if (ret < 0)
>          return ret;
[...]
-- 
FFmpeg = Forgiving Fostering Mean Proud Emblematic Game