[MPlayer-dev-eng] [PATCH] libmpdemux/demux_audio: Skip ID3v2.4 tags.
Benoît Thébaudeau
benoit.thebaudeau at advansee.com
Mon Jan 16 20:51:11 CET 2012
Ingo,
> I was misunderstanding the 3DI footer and thinking of something like
> ID3-frames-data-3DI<EOF> (in which case the footer wouldn't have made
> much sense).
OK.
> > @@ -358,9 +358,11 @@ static int demux_audio_open(demuxer_t* d
> > step = 4;
> > } else if( hdr[0] == 'I' && hdr[1] == 'D' && hdr[2] == '3' &&
> > (hdr[3] >= 2)) {
> > int len;
> > - stream_skip(s,2);
> > + stream_skip(s,1);
> > + stream_read(s,hdr,1);
> > + len = hdr[0] & 0x10 ? 10 : 0;
> > stream_read(s,hdr,4);
> > - len = (hdr[0]<<21) | (hdr[1]<<14) | (hdr[2]<<7) | hdr[3];
> > + len += (hdr[0]<<21) | (hdr[1]<<14) | (hdr[2]<<7) | hdr[3];
> > stream_skip(s,len);
> > step = 4;
> > } else if( found_WAVE && hdr[0] == 'f' && hdr[1] == 'm' &&
> > hdr[2] == 't' && hdr[3] == ' ' ) {
>
> Should be ok. (Although a stream_read(s,hdr,2) and hdr[1] & 0x10
> would save a
> call.)
I'll do that if you prefer.
> > @@ -446,6 +448,17 @@ static int demux_audio_open(demuxer_t* d
> > g = stream_read_char(s);
> > demux_info_add(demuxer,"Genre",genres[g]);
> > }
> > + stream_seek(s,demuxer->movi_end-10);
> > + stream_read(s,tag,4);
> > + if( tag[0] == '3' && tag[1] == 'D' && tag[2] == 'I' &&
> > ((uint8_t)tag[3] >= 2)) {
>
> One pair of () is pointless and I personally would prefer an unsigned
> char
> cast, but I don't know what Reimar thinks (he probably would go with
> hdr).
OK. I used this writing to keep the code as close as possible to the
lines above ("} else if( hdr[0] == 'I' && hdr[1] == 'D' &&
hdr[2] == '3' && (hdr[3] >= 2)) {").
> The version check should be 4.
Or simply removed? The ID3v2 major version does not really matter here
nor above since only the tag header/footer are interpreted and not the
tag contents. However, we could check that it is not 0xff, as specified.
> > + int len;
> > + stream_skip(s,1);
> > + stream_read(s,tag,1);
> > + len = tag[0] & 0x10 ? 20 : 10;
> > + stream_read(s,tag,4);
> > + len += (tag[0]<<21) | (tag[1]<<14) | (tag[2]<<7) | tag[3];
> > + demuxer->movi_end = stream_tell(s)-len;
>
> Indentation.
What do you mean? These lines are indented exactly like the surrounding
lines. See "g = stream_read_char(s);" for instance. Do you want only spaces
instead?
> Shouldn't the existence of 3DI already prove a len of 20? Hmm, A 3DI
> without
> flag could indicate that it isn't a 3DI footer at all...
Indeed, strictly speaking, but I considered that borderline ID3 generators
could skip the header of appended tags like the footer of prepended tags can
legally be, so it's more robust with this test. I can remove it if you prefer.
Benoît
More information about the MPlayer-dev-eng
mailing list