[FFmpeg-devel] [PATCH] Updated original IFF demuxer code to be 100% standard IFF-compliance

[Ronald S. Bultje]

Hi,

On Tue, Apr 20, 2010 at 3:02 PM, Sebastian Vater
<cdgs.basty at googlemail.com> wrote:
> Sorry, I did accidentally send the older version of the patch with the
> identation stuff included, here's the updated version.
[..]
>              st->codec->codec_type            = AVMEDIA_TYPE_VIDEO;
> -            st->codec->width                 = get_be16(pb);
> -            st->codec->height                = get_be16(pb);
> -            url_fskip(pb, 4); // x, y offset
> -            st->codec->bits_per_coded_sample = get_byte(pb);
> -            url_fskip(pb, 1); // masking
> -            compression                      = get_byte(pb);
> -            url_fskip(pb, 3); // paddding, transparent
> -            st->sample_aspect_ratio.num      = get_byte(pb);
> -            st->sample_aspect_ratio.den      = get_byte(pb);
> -            url_fskip(pb, 4); // source page width, height
> +
> +            if (data_size >= 2) {
> +                st->codec->width                 = get_be16(pb);
> +
> +                data_size_padding -= 2;
> +            }
[etc.]

That's not OK. A file with only width but no height is clearly
invalid. I doubt they exist.

If (size < 11) return error;
read width/height/bpcs/comp
if (size >= 16) {
read sar
}
skip remainder

And then try to do this without changing the lines above, so that
you're only adding 1-2 length-checks in the middle of the code. Same
for the other cases.

> +            if (data_size & 1)
> +                url_fskip(pb, 1);

Can probably be generalized at the end of the loop (before the switch)
to decrease lines-of-code.

>          case ID_BODY:
>              iff->body_size = data_size;
> -            done = 1;
> +
> +            if (data_size & 1)
> +                url_fskip(pb, 1);
>              break;

That would break things badly, no? Don't skip 1 byte BEFORE reading
the data. And you're no longer exiting the loop so we will never
actually decode the file.

Ronald

Ronald