[FFmpeg-devel] [PATCH] IVF demuxer

Reimar Döffinger Reimar.Doeffinger
Sat May 22 09:14:01 CEST 2010


On Fri, May 21, 2010 at 07:15:25PM -0400, David Conrad wrote:
> On May 21, 2010, at 1:21 AM, Reimar D?ffinger wrote:
> 
> > On Thu, May 20, 2010 at 09:26:23PM -0400, David Conrad wrote:
> >> +    st->codec->width  = get_le16(s->pb);
> >> +    st->codec->height = get_le16(s->pb);
> >> +    st->time_base.den = get_le32(s->pb);
> >> +    st->time_base.num = get_le32(s->pb);
> > 
> > 
> > The demuxer won't work without valid values for those, so you should
> > check them in the probe function, it is a bit weak currently IMO.
> 
> Width/height can be set in the decoder if missing from the format, and an invalid frame rate should be checks in read_header IMO.

The thing the probe function should probe for is "does this look like this format", not "could someone create a format that looks
like this and we maybe could still play, but does not exist in the wild", the latter IMO should at most get a score of MAX/4.
And yes, the invalid frame rate maybe must be checked in read_header as well, but IMO unless it is too complex to implement
a probe function _must_ fail for all files where read_header would fail.

> I changed the probe function to look at the header size too; I think checking the first 8 bytes for exact vaules is strong enough; the AVI probe only checks 8 bytes too.

You can't just go by the number of bytes, the previous version only checked for ASCII and 0, which makes for a very
weak test, one that usually even probtest complains about.
With the additional size check it's probably reasonably strong now.



More information about the ffmpeg-devel mailing list