[FFmpeg-devel] [PATCH 2/3] lavd/utils: always look at file extension when probing

[Reimar Döffinger]

On Sat, Nov 13, 2010 at 11:22:41AM +0100, Anton Khirnov wrote:
> On Sat, Nov 13, 2010 at 11:00:14AM +0100, Anton Khirnov wrote:
> > On Sat, Nov 13, 2010 at 10:39:44AM +0100, Reimar D?ffinger wrote:
> > > On Sat, Nov 13, 2010 at 10:22:38AM +0100, Anton Khirnov wrote:
> > > > On Fri, Nov 12, 2010 at 07:15:17PM +0100, Michael Niedermayer wrote:
> > > > > On Fri, Nov 12, 2010 at 06:18:10PM +0100, Anton Khirnov wrote:
> > > > > > ---
> > > > > >  libavformat/utils.c |    5 +++--
> > > > > >  1 files changed, 3 insertions(+), 2 deletions(-)
> > > > > 
> > > > > rejected
> > > > > this can (and i wager a bet it will) break things.
> > > > what would it break?
> > > 
> > > Values indicating an unsure detection will suddenly become values indicating
> > > a certain detection.
> > > In case of e.g. MP3 or DV and only let's say the first 12 bytes of a file
> > > in the probe buffer that is almost certain to result in false positives.
> > > 
> > > > > and you provide no example that it fixes so its same or worse than svn
> > > > This is related to the issue i mentioned before -- files with huge id3v2
> > > > headers fail to be detected if the buffer isn't large enough. Yes i know
> > > > the solution is to provide a bigger buffer, but IMO lavf should do the
> > > > best possible guess with the information it has and that includes the
> > > > file extension.
> > > 
> > > File extension can never make a detection reliable, which AFAICT your
> > > patch would pretend to.
> > > Neither can it resolve a conflict if two formats are detected with
> > > low probability.
> > > And it probably shouldn't be considered a conflict if a format
> > > is only detected with low probability but that does not match
> > > the extension (consider a wav/RIFF file containing MP3, and a lot of
> > > people will give that a .mp3 extension on top. I have also seen
> > > AVI with .mp2, raw MPEG-4 with .mp4, DV in mov with .dv, all
> > > of which have a high risk that this extension "offset" will
> > > make the detection break).
> > > 
> > > > Anyway, if you're dead set against using file extensions, why are they
> > > > even there?
> > > 
> > > They're useful information, but your implementation gives them almost
> > > the same priority as the probe function itself, that is not appropriate.
> > I merely copied the number that was already there.
> > Would the attached patch be more acceptable then? An extension adds 25%
> > to the score, up to 50% max.
> > 
> ok, let's pretend i didn't send that :)

Which addresses neither of my comments, it still can increase the
score above MAX/4 and it might trigger the condition
if (score == *score_max)
    fmt = NULL;
And honestly it may trigger
if (score > *score_max)
as well.
One of those alone IMO is reason enough for rejection.
I guess you possibly could add a
if (!fmt && *score_max == 0) {
  fmt = av_guess_format(...);
  if (fmt)
    *score_max = 1;
}