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

[Wei Gao]

2013/5/4 Stefano Sabatini <stefasab at gmail.com>

> 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.
>
Hi, thanks for your reviewing.There is still something I don't understand.
if the size is   "sizeof(uint32_t) * (2 * unsharp->chroma.steps_x + 1) * (2
* unsharp->chroma.steps_y + 1)", the program will crush too, so I tried to
"sizeof(uint32_t) *  sizeof(uint32_t) * (2 * unsharp->chroma.steps_x + 1) *
(2 * unsharp->chroma.steps_y + 1)", then the program is correct, so I use
size_x * size_y as a matrix size.

>
> > +        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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>