On sab, apr 12, 2014 at 09:04:56 +0200, Anton Khirnov wrote: > > On Wed, 9 Apr 2014 18:50:05 +0200, Alessandro Ghedini > <[email protected]> wrote: > > --- > > libavformat/mp3dec.c | 57 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 57 insertions(+) > > > > diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c > > index 1ab5159..e33f524 100644 > > --- a/libavformat/mp3dec.c > > +++ b/libavformat/mp3dec.c > > @@ -132,7 +132,17 @@ static void read_xing_toc(AVFormatContext *s, int64_t > > filesize, int64_t duration > > static void mp3_parse_info_tag(AVFormatContext *s, AVStream *st, > > MPADecodeHeader *c, uint32_t spf) > > { > > +#define LAST_BITS(k, n) ((k) & ((1 << (n)) - 1)) > > +#define MIDDLE_BITS(k, m, n) LAST_BITS((k) >> (m), ((n) - (m))) > > + > > + float f; > > uint32_t v; > > + > > + char version[10]; > > + > > + uint32_t peak = 0; > > + int32_t r_gain = INT32_MIN, a_gain = INT32_MIN; > > + > > MP3DecContext *mp3 = s->priv_data; > > const int64_t xing_offtbl[2][2] = {{32, 17}, {17,9}}; > > > > @@ -152,6 +162,53 @@ static void mp3_parse_info_tag(AVFormatContext *s, > > AVStream *st, > > (AVRational){spf, c->sample_rate}, > > st->time_base)); > > } > > + > > + avio_skip(s->pb, 3); > > + > > + /* VBR quality */ > > + avio_r8(s->pb); > > Doesn't matter much since we ignore the value, but isn't vbr quality this > whole > 32bit number?
http://gabriel.mp3-tech.org/mp3infotag.html says it's only one byte. > > + > > + /* Encoder short version string */ > > + avio_get_str(s->pb, 9, version, 10); > ^^ > sizeof(version) please > > Also, perhaps we should export this as 'encoder' in the stream metadata (not > in > global, so we don't conflict with whatever is read from the ID3 tag). This doesn't seem to work. I added: av_dict_set(&st->metadata, "encoder", version, 0); But when I use avprobe on a file encoded with libav, "encoder" is an empty string instead of "Lavf..." (i.e. it does conflict with the id3 tag). > And I wonder if it's safe to unconditionally assume the tag is in the LAME > format. IIUC, it can be just the XING tag with garbage/zeroes filling the rest > of the frame? > > Is there some way to check that the tag is really in the LAME format? The replaygain values already have a check, for the encoder string, maybe checking that version[0] != '\0', and that all the characters are >= 32. Another way would be to check the header CRC16 (which covers the first 190 bytes of the header) before exporting any information (encoder and replaygain). Not sure how reliable that is though. Btw, I just realized that the function doesn't return if the MKBETAG() checks fail (the fix will be in the next version of the patch). > > + > > + /* Info Tag revision + VBR method */ > > + avio_r8(s->pb); > > + > > + /* Lowpass filter value */ > > + avio_r8(s->pb); > > + > > + /* ReplayGain peak */ > > + v = avio_rb32(s->pb); > > + f = (v << 5) / (float) (1 << 28); > > http://gabriel.mp3-tech.org/mp3infotag.html says this value is stored as float > (nb: people doing such stuff need to be stabbed with a rusty spork > repeatedly), > so perhaps use av_int2float()? That doesn't work (i.e. it always return 0.0). I copied the decoding formula from eyed3 [0] so I'm not quite sure if that's correct, but it does seem to return realistic peak values. > > + > > + if (f > 0) > > + peak = f * 100000; > > + > > + v = avio_rb16(s->pb); > > + > > + /* Radio ReplayGain */ > > + if (MIDDLE_BITS(v, 13, 15) == 1) { > > + f = MIDDLE_BITS(v, 0, 8) / 10.f; > > Why /10? You can remove a zero from the multiplication below instead, no? > That will also avoid the evil inexact float arithmetics. Right. [0] https://bitbucket.org/nicfit/eyed3/src/5a4ab5730ced8d5d0e21440c7c6ff212f923f9d7/src/eyed3/mp3/headers.py?at=default#cl-608 -- perl -E '$_=q;$/= @{[@_]};and s;\S+;<inidehG ordnasselA>;eg;say~~reverse' _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
