On Sat, 12 Apr 2014 13:23:52 +0200, Alessandro Ghedini <[email protected]> wrote: > 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. >
But at the top of the page it has // 4 bytes for VBR SCALE. a VBR quality indicator: 0=best 100=worst Anyway, doesn't matter. Keep it as you like. > > > + > > > + /* 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). Our lame wrapper disables the LAME tag, so that is quite expected. Try it on some file that is generated by lame directly. > > > 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. That sounds like a good idea to me. > > 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. Looking at LAME itself, seems it takes the float peak value (with 1.0 being full scale), multiplies it by 1<<23, casts the result to int, and writes that. So something like av_rescale(v, 100000, 1 << 23) should work. -- Anton Khirnov _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
