[FFmpeg-devel] [PATCH 1/2] mandelbrot: remove always-false condition in fill_from_cache

[Michael Niedermayer]

On Sun, Nov 13, 2011 at 06:42:09PM +0100, Giorgio Vazzana wrote:
> 2011/11/13 Michael Niedermayer <michaelni at gmx.at>:
> > On Sun, Nov 13, 2011 at 04:46:44PM +0100, Giorgio Vazzana wrote:
> >> Hello,
> >>
> >> I noticed this while studying the new cache code. Please comment :)
> >
> >>  vsrc_mandelbrot.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> cd78ecfb09f9d7260f8cdd5da989c78d205b5f17  0001-mandelbrot-remove-always-false-condition-in-fill_fro.patch
> >> From 05d1b40ae6c5ae7de3ede8c44ad230e0428391d2 Mon Sep 17 00:00:00 2001
> >> From: Giorgio Vazzana <mywing81 at gmail.com>
> >> Date: Sun, 13 Nov 2011 13:51:09 +0100
> >> Subject: [PATCH 1/2] mandelbrot: remove always-false condition in fill_from_cache
> >>
> >> ---
> >>  libavfilter/vsrc_mandelbrot.c |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/libavfilter/vsrc_mandelbrot.c b/libavfilter/vsrc_mandelbrot.c
> >> index 375ec7c..490224b 100644
> >> --- a/libavfilter/vsrc_mandelbrot.c
> >> +++ b/libavfilter/vsrc_mandelbrot.c
> >> @@ -144,7 +144,7 @@ static void fill_from_cache(AVFilterContext *ctx, uint32_t *color, int *in_cidx,
> >>      for(; *in_cidx < mb->cache_used; (*in_cidx)++){
> >>          Point *p= &mb->point_cache[*in_cidx];
> >>          int x;
> >> -        if(*in_cidx >= mb->cache_used || p->p[1] > py)
> >> +        if(p->p[1] > py)
> >>              break;
> >
> > i think this is wrong, i didnt check though but
> > when the zoom factor becomes big enough the double precission floats
> > will no longer be able to represent the screen pixel positions and
> > at that point the cache fills up and wont be able to hold all
> > values anymore.
> > I think at this point this condition can become true.
> > also it could become true when we zoom out instead of in
> 
> Thanks for your answer Michael. I applied the following patch:
> 
> diff --git a/libavfilter/vsrc_mandelbrot.c b/libavfilter/vsrc_mandelbrot.c
> index 7a1a864..6f1002b 100644
> --- a/libavfilter/vsrc_mandelbrot.c
> +++ b/libavfilter/vsrc_mandelbrot.c
> @@ -144,6 +144,8 @@ static void fill_from_cache(AVFilterContext *ctx,
> uint32_t *color, int *in_cidx,
>      for(; *in_cidx < mb->cache_used; (*in_cidx)++){
>          Point *p= &mb->point_cache[*in_cidx];
>          int x;
> +        if(*in_cidx >= mb->cache_used)
> +            av_log(0, AV_LOG_WARNING, "*in_cidx >= mb->cache_used\n");
>          if(*in_cidx >= mb->cache_used || p->p[1] > py)
>              break;
>          x= round((p->p[0] - mb->start_x) / scale + mb->w/2);
> 
> but couldn't trigger the condition neither while zooming in nor while
> zooming out. But as you said, I got lots of messages "Mandelbrot cache
> is too small!" after a certain point. When zooming in I made sure I
> let it run long enough to get pixelated pictures due to the big zoom
> factor.
> 
> My reasoning was: we only enter into the for cycle when *in_cidx <
> mb->cache_used. Inside the for we have only two instructions before
> the if:

oops, i missed that the check was there twice. in this case your patch
is correct of course.
Applied & thanks for explaining !

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111113/7e5f4b1c/attachment.asc>