[FFmpeg-devel] [PATCH] Fix non-rounding up to next 16-bit aligned bug in IFF decoder

[Ronald S. Bultje]

Hi,

On Wed, Apr 28, 2010 at 7:30 AM, Sebastian Vater
<cdgs.basty at googlemail.com> wrote:
> I have fixed the wrong IFF decoding issue in the IFF decoder.
>
> The reason is that the IFF docs say that each line in the BODY chunk has
> it's width rounded up to next 16-bit boundary, such that each new line
> begins on a word boundary (address divisible by 2).
[..]
> +    s->planesize = ((avctx->width + 15) >> 4) * 2;

This is not how people commonly round up to 2. Most people do
(x+1)&~1, so you could do:

((width+15)>>4)<<1

or (which is what most people do, so I'd prefer this):

((width+15)>>3)&~1

But both are fine. And add a comment what this does, e.g. "/* round up
width to word-boundary */" or so, for the unsuspecting reader.

The rest of the patch is unrelated to this change and should be
reviewed separately.

> -    const unsigned b = (buf_size * 8) + bps - 1;
> +    const uint8_t *end = dst + (buf_size * 8);

I'm not convinced that is correct. "It doesn't change the output of my
files" isn't a good enough reason. You're changing the math so it will
either get worse or better, and for the "better" part it'd need to fix
a bug or you'd need to show the documentation pointing out that this
is better.

Ronald