[FFmpeg-devel] [PATCH] Indeo5 decoder
Thu Apr 2 12:57:34 CEST 2009
On Thu, Apr 02, 2009 at 11:40:49AM +0200, Maxim wrote:
> > useless & 
> > indeo5patch.txt:676:+ band->dequant_intra = &deq4x4_intra;
> > indeo5patch.txt:677:+ band->dequant_inter = &deq4x4_inter;
> changing it to
> band->dequant_intra = deq4x4_intra;
> causes the following compiler's message:
> warning: assignment from incompatible pointer type
One & cancels with one , so that would be
band->dequant_intra = deq4x4_intra;
> > indeo5patch.txt:1035:+ av_freep(&planes.buf1);
> > indeo5patch.txt:1036:+ av_freep(&planes.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;
> > indeo5patch.txt:1295:+ int32_t *src, *dst, tmp;
> 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).
More information about the ffmpeg-devel