[FFmpeg-devel] [PATCH] IVF demuxer

[Michael Niedermayer]

On Sat, May 22, 2010 at 09:14:01AM +0200, Reimar D?ffinger wrote:
> 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, 

IMO the probe function shouldnt touch witdh/height/sample_rate unless it is
not possible to detect the format reliably without such hacks.
also the rate of false positives should be weighted against the rate of false
negatives when deciding what to check and what not.
a width != 0 check is not going to gain you much in terms of false positives
but if such file can be demuxed correctly it is a loss in terms of false
negatives
and it can be quite usefull to be able to demux a file even if we cannot
decode it, for debuging and implementing and fixing ...


> but IMO unless it is too complex to implement
> a probe function _must_ fail for all files where read_header would fail.

the probe function checks if "this" is for example mov, it does not check
if our mov demuxer supports all features to demux it. These are 2 seperate
things and its not the job of probe to reject files with features we do not
support. Thats quite unwanted behavior, it would only lead to the file being
detected as ac3 or mp3 with low score especially if it contains an audio track
with mp3


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100523/a43a8dfb/attachment.pgp>