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

[Michael Niedermayer]

Hi

On Thu, Sep 07, 2006 at 11:08:15PM +0100, Victor Voros wrote:
[...] 
> > 
> > Attached is a patch to libavformat\mp3.c which addresses this in diff
> -u
> > format from head (6180)
> > 
> > Simply scans the file looking for a sync frame and starts there if
> > found.
> 
> rejected, this does not belong in libavformat, demuxers should output
> whats in the file, not do codec specific parsing and discarding of stuff
> such code should be in the parser or decoder
> 
> mp3.c as I see it serves 2 purposes, 1) to determine whether the file is
> in fact an mp3 file (mp3_Read_probe) and 2) to prime the file so its
> pointing at the start of the first frame, (currently skipping a possible
> id3 tag) ready for the codec to do its work(mp3_read_header). The file
> already has code to search for a valid mpeg frame sync, the change
> expands on this by not assuming the frame sync is at the very start of
> the file. No extra codec 'knowledge' has been added to this file.
> 
> In any case, mp3_read_probe is the routine rejecting the said files. One
> way or another this has to change to accommodate the files. 

indeed mp3_read_probe() needs to be fixed, but that is the only function
in mp3.c which should be changed

[...]

> +#define MP3_PACKET_SIZE 1024
> +
> +static int mp3_is_frame_header(uint8_t *p)
> +{
> +	uint8_t d;
> +
> +	if (*p != 0xff) return 0;
> +
> +	d = p[1];
> +	if((d & 0xe0) != 0xe0 || ((d & 0x18) == 0x08 || (d & 0x06) == 0))
> +		return 0;
> +
> +	d = p[2];
> +	if((d & 0xf0) == 0xf0 || (d & 0x0c) == 0x0c)
> +		return 0;
> +
> +	return 1;
> +
> +}

looks like a duplication of ff_mpa_check_header()


> +
>  /* mp3 read */
>  
>  static int mp3_read_probe(AVProbeData *p)
>  {
>      int d;
>  
> -    if(p->buf_size < 4)
> -        return 0;
> -
> -    if(p->buf[0] == 'I' && p->buf[1] == 'D' && p->buf[2] == '3' &&
> -       p->buf[3] < 5)
> +    if((p->buf_size >= ID3_TAG_SIZE) && id3_match(p->buf))
>          return AVPROBE_SCORE_MAX;
>  
> -    if(p->buf[0] != 0xff)
> -        return 0;
> +	// handle buggy mp3 converter programs that have a habit of starting mid-frame
> +	// and find the first frame. if that sounds ott Windows Media player and itunes
> +	// handle these files.
>  
> -    d = p->buf[1];
> -    if((d & 0xe0) != 0xe0 || ((d & 0x18) == 0x08 || (d & 0x06) == 0))
> -        return 0;
> -
> -    d = p->buf[2];
> -    if((d & 0xf0) == 0xf0 || (d & 0x0c) == 0x0c)
> -        return 0;
> +    for (d=0;d<FFMIN(MP3_PACKET_SIZE,p->buf_size-4);d++) {
>  
> -    return AVPROBE_SCORE_MAX;
> +        if (mp3_is_frame_header(p->buf+d)) return AVPROBE_SCORE_MAX;

not ok due to false positive detection (mp3 in avi, mp3 in mov, mp3 in mpeg-ps
and many others)


[...]
>  }
>  
> +
>  static int mp3_read_header(AVFormatContext *s,

cosmetic change


[changes outside mp3_read_probe()]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is