[Ffmpeg-devel] [PATCH] allow ffmpeg to read mp3s beginning with partial frames

Victor Voros Victor.Voros
Sat Sep 9 19:42:27 CEST 2006


Andreas ?man wrote
> Subject: Re: [Ffmpeg-devel] [PATCH] allow ffmpeg to read mp3s beginning with partial frames
> 
> Michael Niedermayer wrote:
> 
> >>> yes thats why i am so much against doing it in lavf/mp3.c, it belongs
> >>> to the mp3 parser (libavcodec/parser.c)
> >>>
> >>>
> >> Turns out im wrong it does. libavcodec/parser.c handles initial partial
> >> frame and files work fine with just the change to mp3_read_probe as
> >> supplied. 
> > 
> > patch looks ok
> > 
> 
> I have to disagree. With this new patch ffmpeg still thinks some of my 
> files are mp3-files (especially .ts-files)
> 
> However, having bumped into the same problem I've had a patch that fixes 
> the problem (without generating any false positives). It searches the 
> first 4k of the file for two consecutive mp3-frames.

This looks more reliable and works with all the problematic mp3 files I can find. Can I make a suggestion. The line 

header = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];

be replaced with

memcpy(&header,buf,sizeof(header));

I know the libav* is riddled with the former style however under mingw for x86 memcpy gives me 2 instructions vs about a dozen for the first style. I Moreover the second style is a hell of a lot clearer and less prone to error. I suspect the former was better in days gone by when memcpy was not inlined and resulted in a function call and a loop. Also since the PIII (I think) 32-bit loads on a non-4-byte-boundary are as fast as they are on a 4-byte boundary. This was not the case on older intels. 

Also it would be nice to replace 

if(p->buf[0] == 'I' && p->buf[1] == 'D' && p->buf[2] == '3' &&
       p->buf[3] < 5)
     
with if (!id3_match(...) && size is at least ID3_HEADER_SIZE) as I had in my patch. Know this is not the immediate problem, however it is potentially problematic. The 2 checks are not quite the same, however they clearly need to be. 
        
> 
> Due to some recent mail-malfunctioning I could not respond to the thread 
> earlier. Sorry 'bout that. And I haven't gotten around to send the patch 
> either, shame on me...
>


------------------------------------------------------------------------------
This e-mail and its contents are intended for the use of the addressee(s) and may be confidential/privileged. No-one else may review, copy, disclose or otherwise use it or its contents. If you receive this e-mail in error, please contact the originator and delete it as soon as possible.
Benfield Limited is authorised by the Financial Services Authority under the reference number 311884. Registered in England no 1170753. Registered office 55 Bishopsgate.

Please refer to Benfield Limited's terms and conditions (www.benfieldgroup.com/terms) for a description of our services, duties and points of contact. Please review these terms and conditions at inception and renewal of all reinsurance and insurance contracts.  

Please note that our terms and conditions and interests in other companies may change over time and these changes will be reflected on the Benfield Limited website. The latest version of our terms and conditions supersedes all previous versions.
==============================================================================





More information about the ffmpeg-devel mailing list