[MPlayer-dev-eng] Re: [Patch] Trying harder to detect MP3

Alban Bedel albeu at free.fr
Sun Jan 25 13:40:39 CET 2004


Hi Michael Behrisch,

on Sat, 24 Jan 2004 20:46:11 +0100 you wrote:

> On Sun, Dec 28, 2003 at 02:29:35PM +0100, Alban Bedel wrote:
> > Hi Michael Behrisch,
> > 
> > on Sat, 27 Dec 2003 21:14:32 +0100 you wrote:
> > 
> > > Hello,
> > > 
> > > this one incorporates code from mp3check in order
> > > to recognize mp3's with (at most 100 bytes of) junk at start.
> > > It works only with files (seekable streams).
> > 
> > The main idea is ok to me, but it's not implemented at the right
> > place. The function is cut into 2 parts. The first try to findout the
> > file type. The second one setup the demuxer stream, etc.
> > In the case of mp3 it's not so clear bcs the second part also perform
> > some extra test to ensure that the file is really an mp3.
> > So imho your extra check should go into the first part where it
> > currently only set frmt to MP3.
> 
> i thought it is very similar to the check, which already is there,
> looking for 5 consecutive valid headers, that's why I put it there

No. You misunderstood again i fear. Past the switch(frmt) the demuxer
type is know. The tests that follow are there to ensure that the file
really is an mp3. You must put your test in the first loop, where it
try to detect the file type !!!!
Also you should not do this test for non-seekable stream as it was in
your first patch.

> I did it a little rework and resent the patch in the appendix
> Maybe it's clearer then why it simply replaces the check which
> was there. Also demuxer->movi_start is ok now.

Don't remove that without extended testing !!!!!

> > IIRC ad_mp3lib skip the first frame too now, so it may not be needed
> > anymore. Anyway i highly recommend that you checks your patch with
> > LOTS of mp3. (And with LOTS i really mean at least some 1000's
> > including old stuff ;)
> 
> I don't have thousands but I checked with some, and it works fine for
> me.

I can test, but first the patch must be acceptable, sorry.

	Albeu

-- 

Everything is controlled by a small evil group
to which, unfortunately, no one we know belongs.





More information about the MPlayer-dev-eng mailing list