[FFmpeg-devel] [PATCH] Indeo5 decoder

[Reimar Döffinger]

On Thu, Apr 02, 2009 at 11:40:49AM +0200, Maxim wrote:
> > useless & [0]
> > indeo5patch.txt:676:+                band->dequant_intra = &deq4x4_intra[0][0];
> > indeo5patch.txt:677:+                band->dequant_inter = &deq4x4_inter[0][0];
> >   
> 
> changing it to
> 
> band->dequant_intra = deq4x4_intra;
> 
> causes the following compiler's message:
> warning: assignment from incompatible pointer type

One & cancels with one [0], so that would be
band->dequant_intra = deq4x4_intra[0];

> > indeo5patch.txt:1035:+    av_freep(&planes[0].buf1);
> > indeo5patch.txt:1036:+    av_freep(&planes[0].buf2);
> >   
> 
> Hmm, shouldn't av_freep receive a POINTER to a POINTER as it's declared
> in its prototype? Will then the removing of the "&" lead to a wrong
> program behavior?

I think those a false positives, if you still haven't noticed that was
an automated "review" by tools/patcheck
You could probably use &planes->buf1 here though, but that's probably
worse (i.e. uglier)

> > missing const?
> > indeo5patch.txt:1248:+    int32_t *src, *dst, tmp[64];
> > indeo5patch.txt:1295:+    int32_t *src, *dst, tmp[16];
> 
> Yes, this is abit complicated! The src pointer should be declared as a
> pointer to a const data in fact. But due to the given design of the
> transform it cannot be made so because both IVI_IREFLECT and
> IVI_SLANT_BFLY make changes to their operands. I need to redesign the
> whole transform in order to make the src const. Will it worth it?

This might just be a false positive by the script.
I think Michael just wanted to make you look over the obvious things a
simple tool can detect before "wasting" a human's time with a review
(there are just too few people doing patch reviews).