[FFmpeg-devel] [PATCH] fixes in mpeg audio parsing
Michael Niedermayer
michaelni
Thu Nov 13 16:47:52 CET 2008
On Thu, Nov 13, 2008 at 04:47:58PM +0200, Yoav Steinberg wrote:
>
>
> Michael Niedermayer wrote:
> > On Thu, Nov 13, 2008 at 09:16:24AM +0200, Yoav Steinberg wrote:
> >>
> >> Michael Niedermayer wrote:
> >>> On Wed, Nov 12, 2008 at 07:02:55PM +0200, Yoav Steinberg wrote:
> >>>> Hi,
> >>>> Attached are patches witch make some improvements for mp3 parsing.
> >>>> skip_vbrtag_and_id3v1.patch:
> >>>> - When file contains ID3v1 tag at the end don't attempt to read it as
> >>>> part of a packet. In some cases there are mp3 files where the ID3v1 tag
> >>>> was stuck forcefully at the end of the file inside the last frame
> >>>> boundary. In those cases the patch eliminates attempts to parse/decode
> >>>> the ID3 tag.
> >>>> - If we find a VBR tag at the beginning of the file don't attempt to
> >>>> parse it as a valid frame.
> >>> These are 2 seperate things which must be in seperate patches.
> >> Attached is a single patch for the prevention of reading ID3v1 as part of a
> >> packet.
> >>
> >>>> @@ -505,10 +515,20 @@
> >>>> static int mp3_read_packet(AVFormatContext *s, AVPacket *pkt)
> >>>> {
> >>>> int ret, size;
> >>>> + int64_t size_left;
> >>>> // AVStream *st = s->streams[0];
> >>>> size= MP3_PACKET_SIZE;
> >>>> + /* if we're nearing the end of the input and there's an
> >>>> + ID3v1 tag then make sure we don't read it as a packet */
> >>>> + if (((MPAContext*)s->priv_data)->found_id3v1 && s->file_size && +
> >>>> (size_left = s->file_size - url_ftell(s->pb) - ID3v1_TAG_SIZE) <
> >>>> size) {
> >>>> + size = size_left;
> >>>> + if (size <= 0)
> >>>> + return AVERROR(EIO);
> >>>> + }
> >>>> +
> >>>> ret= av_get_packet(s->pb, pkt, size);
> >>>> pkt->stream_index = 0;
> >>> file_size is not known for stdin/pipes
> >> That's why I put the '&& s->file_size' so that this logic will be avoided
> >> when file size is not known. In any case the found_id3v1 will be false in
> >> such cases. But for future compatibility in case the tag will somehow be
> >> known but not the file size I added the '&& s->file_size' (which is set to
> >> 0 if file size is not known).
> >
> > Fixing a bug just for seekable input _requires_ a proof that it cannot be
> > fixed for non seekable or that a complete fix would be too messy.
> >
>
> Not quite sure what you mean by proof.
i mean proof in the mathematical sense, that is that no sequence of
instructions could detect it reliably for non seekable streams.
> I'll explain why it cannot be
> handled nicely for non seekable, let me know what other info/proof you want.
fine
> The only way to detect ID3v1 tag is by probing the last 128 bytes of the
> stream.
thats not an explanation, thats just the same claim in slightly different
words
[...]
>
> It _might_ be possible to change read_packet to always buffer the last
> 128 bytes of data and return them with a delay after more data is
> available.
Thats no "might", thats certainly possible. But thats not what i mean.
First why do you not detect the id3v1 tag? i mean check if there is a mp3
frame or a id3v1 tag by looking at the header.
One certainly could have concatenated 2 mp3 files with a id3v1 ending in
the middle it would be nice if the parser & decoder would skip/ignore this
trash in the middle.
To me it seem that it shouldnt be too hard to make the parser skip
this, unless a valid id3v1 tag can also be a valid mp3 frame.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I count him braver who overcomes his desires than him who conquers his
enemies for the hardest victory is over self. -- Aristotle
-------------- 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/20081113/a45a858f/attachment.pgp>
More information about the ffmpeg-devel
mailing list