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

[Giorgio Vazzana]

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:

1) Point *p= &mb->point_cache[*in_cidx];
2) int x;

and neither of them changes *in_cidx or mb->cache_used, so inside the
for the condition in_cidx >= mb->cache_used is always false, unless
I'm missing something here.

Giorgio Vazzana