| Message ID | 1470437665-78199-2-git-send-email-kode54@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Fri, Aug 05, 2016 at 03:54:25PM -0700, kode54@gmail.com wrote: > From: Chris Moeller <kode54@gmail.com> > > --- > libavformat/mp3dec.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 63 insertions(+), 1 deletion(-) > > diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c > index 56c7f8c..3055e2c 100644 > --- a/libavformat/mp3dec.c > +++ b/libavformat/mp3dec.c > @@ -295,6 +295,53 @@ static void mp3_parse_vbri_tag(AVFormatContext *s, AVStream *st, int64_t base) > } > } > > +static void mp3_parse_itunes_tag(AVFormatContext *s, AVStream *st, MPADecodeHeader *c, int64_t base, int vbrtag_size, unsigned int *size, uint64_t *duration) > +{ > + uint32_t v; > + AVDictionaryEntry *de; > + MP3DecContext *mp3 = s->priv_data; > + size_t length; > + uint32_t zero, start_pad, end_pad; > + uint64_t last_eight_frames_offset; > + int i; > + > + if (!s->metadata || !(de = av_dict_get(s->metadata, "iTunSMPB", NULL, 0))) > + return; > + > + length = strlen(de->value); > + > + /* Minimum length is one digit per field plus the whitespace, maximum length should depend on field type > + * There are four fields we need in the first six, the rest are currently zero padding */ > + if (length < (12 + 11) || length > (10 * 8 + 2 * 16 + 11)) > + return; > + > + if (sscanf(de->value, "%x %x %x %llx %x %llx", &zero, &start_pad, &end_pad, duration, &zero, &last_eight_frames_offset) < 6) { > + *duration = 0; > + return; > + } libavformat/mp3dec.c: In function ‘mp3_parse_itunes_tag’: libavformat/mp3dec.c:318:5: warning: format ‘%llx’ expects argument of type ‘long long unsigned int *’, but argument 6 has type ‘uint64_t *’ [-Wformat] libavformat/mp3dec.c:318:5: warning: format ‘%llx’ expects argument of type ‘long long unsigned int *’, but argument 8 has type ‘uint64_t *’ [-Wformat] check for duration < 0 missing > + > + mp3->start_pad = start_pad; > + mp3->end_pad = end_pad; assigning unsigend to signed with no range checks could result in overflow, though even if it doesnt overflow the value should be checked to be within a sane range > + if (end_pad >= 528 + 1) > + mp3->end_pad = end_pad - (528 + 1); > + st->start_skip_samples = mp3->start_pad + 528 + 1; > + av_log(s, AV_LOG_DEBUG, "pad %d %d\n", mp3->start_pad, mp3->end_pad); > + if (!s->pb->seekable) > + return; > + > + *size = (unsigned int) last_eight_frames_offset; value could be truncated, missing range check > + avio_seek(s->pb, base + vbrtag_size + last_eight_frames_offset, SEEK_SET); missing seek faiure check also please provide a testcase/sample for this (a fate test would be even better) thx [...]
This hopefully fixes all of the shortcomings that were still present in the patch I last submitted. Unfortunately, I don't track the mailing list as closely as I should, so I missed the responses to my last round of patches. The first one made it in, though. There are reasonable limits applied to check the validity of the input values now, giving approximately 32 frames of lead and 64 frames of tail padding, with a rudimentary check on total file size before attempting the total file size check, and error checking on the seek operation. Christopher Snowhill (1): avformat: parse iTunes gapless information libavformat/mp3dec.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-)
diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c index 56c7f8c..3055e2c 100644 --- a/libavformat/mp3dec.c +++ b/libavformat/mp3dec.c @@ -295,6 +295,53 @@ static void mp3_parse_vbri_tag(AVFormatContext *s, AVStream *st, int64_t base) } } +static void mp3_parse_itunes_tag(AVFormatContext *s, AVStream *st, MPADecodeHeader *c, int64_t base, int vbrtag_size, unsigned int *size, uint64_t *duration) +{ + uint32_t v; + AVDictionaryEntry *de; + MP3DecContext *mp3 = s->priv_data; + size_t length; + uint32_t zero, start_pad, end_pad; + uint64_t last_eight_frames_offset; + int i; + + if (!s->metadata || !(de = av_dict_get(s->metadata, "iTunSMPB", NULL, 0))) + return; + + length = strlen(de->value); + + /* Minimum length is one digit per field plus the whitespace, maximum length should depend on field type + * There are four fields we need in the first six, the rest are currently zero padding */ + if (length < (12 + 11) || length > (10 * 8 + 2 * 16 + 11)) + return; + + if (sscanf(de->value, "%x %x %x %llx %x %llx", &zero, &start_pad, &end_pad, duration, &zero, &last_eight_frames_offset) < 6) { + *duration = 0; + return; + } + + mp3->start_pad = start_pad; + mp3->end_pad = end_pad; + if (end_pad >= 528 + 1) + mp3->end_pad = end_pad - (528 + 1); + st->start_skip_samples = mp3->start_pad + 528 + 1; + av_log(s, AV_LOG_DEBUG, "pad %d %d\n", mp3->start_pad, mp3->end_pad); + if (!s->pb->seekable) + return; + + *size = (unsigned int) last_eight_frames_offset; + avio_seek(s->pb, base + vbrtag_size + last_eight_frames_offset, SEEK_SET); + for (i = 0; i < 8; i++) { + v = avio_rb32(s->pb); + if (ff_mpa_check_header(v) < 0) + return; + if (avpriv_mpegaudio_decode_header(c, v) != 0) + break; + *size += c->frame_size; + avio_skip(s->pb, c->frame_size - 4); + } +} + /** * Try to find Xing/Info/VBRI tags and compute duration from info therein */ @@ -303,8 +350,10 @@ static int mp3_parse_vbr_tags(AVFormatContext *s, AVStream *st, int64_t base) uint32_t v, spf; MPADecodeHeader c; int vbrtag_size = 0; + unsigned int size = 0; MP3DecContext *mp3 = s->priv_data; int ret; + uint64_t duration = 0; ffio_init_checksum(s->pb, ff_crcA001_update, 0); @@ -327,16 +376,29 @@ static int mp3_parse_vbr_tags(AVFormatContext *s, AVStream *st, int64_t base) mp3_parse_vbri_tag(s, st, base); if (!mp3->frames && !mp3->header_filesize) + vbrtag_size = 0; + + mp3_parse_itunes_tag(s, st, &c, base, vbrtag_size, &size, &duration); + + if (!mp3->frames && !size && !duration) return -1; /* Skip the vbr tag frame */ avio_seek(s->pb, base + vbrtag_size, SEEK_SET); - if (mp3->frames) + if (duration) + st->duration = av_rescale_q(duration, (AVRational){1, c.sample_rate}, st->time_base); + else if (mp3->frames) st->duration = av_rescale_q(mp3->frames, (AVRational){spf, c.sample_rate}, st->time_base); if (mp3->header_filesize && mp3->frames && !mp3->is_cbr) st->codecpar->bit_rate = av_rescale(mp3->header_filesize, 8 * c.sample_rate, mp3->frames * (int64_t)spf); + if (size) { + if (duration) + st->codecpar->bit_rate = av_rescale(size, 8 * c.sample_rate, duration); + else if (mp3->frames) + st->codecpar->bit_rate = av_rescale(size, 8 * c.sample_rate, mp3->frames * (int64_t)spf); + } return 0; }
From: Chris Moeller <kode54@gmail.com> --- libavformat/mp3dec.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-)