diff mbox

[FFmpeg-devel,PATCHv4] avformat: parse iTunes gapless information

Message ID 20161208211207.12297-2-kode54@gmail.com
State Superseded
Headers show

Commit Message

Christopher Snowhill Dec. 8, 2016, 9:12 p.m. UTC 
---
 libavformat/mp3dec.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 69 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer Dec. 9, 2016, 2:39 p.m. UTC | #1 
On Thu, Dec 08, 2016 at 01:12:07PM -0800, Christopher Snowhill wrote:
> ---
>  libavformat/mp3dec.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 69 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> index 56c7f8c..47f4028 100644
> --- a/libavformat/mp3dec.c
> +++ b/libavformat/mp3dec.c
> @@ -295,6 +295,59 @@ 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;

inconsistent indention


> +
> +    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, "%"PRIx32" %"PRIx32" %"PRIx32" %"PRIx64" %"PRIx32" %"PRIx64, &zero, &start_pad, &end_pad, duration, &zero, &last_eight_frames_offset) < 6 ||

> +        duration < 0 ||

duration is a pointer



> +        start_pad > (576 * 2 * 32) ||
> +        end_pad > (576 * 2 * 64) ||

> +        last_eight_frames_offset >= (avio_size(s->pb) - base - vbrtag_size)) {

avio_size() could fail i think


> +        *duration = 0;
> +        return;
> +    }
> +

> +    mp3->start_pad = (signed int) start_pad;
> +    mp3->end_pad = (signed int) end_pad;

useless casts



> +    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;

> +    if (avio_seek(s->pb, base + vbrtag_size + last_eight_frames_offset, SEEK_SET) < 0)
> +	return;

tabs are not allowed in ffmpeg git

[...]
Christopher Snowhill Jan. 27, 2017, 12:47 a.m. UTC | #2 
Let's try this again. I can't believe I missed all those things you
mentioned. It's almost as if I didn't actually proofread my patch
before sending it.
diff mbox

Patch

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 56c7f8c..47f4028 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -295,6 +295,59 @@  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, "%"PRIx32" %"PRIx32" %"PRIx32" %"PRIx64" %"PRIx32" %"PRIx64, &zero, &start_pad, &end_pad, duration, &zero, &last_eight_frames_offset) < 6 ||
+        duration < 0 ||
+        start_pad > (576 * 2 * 32) ||
+        end_pad > (576 * 2 * 64) ||
+        last_eight_frames_offset >= (avio_size(s->pb) - base - vbrtag_size)) {
+        *duration = 0;
+        return;
+    }
+
+    mp3->start_pad = (signed int) start_pad;
+    mp3->end_pad = (signed int) 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;
+    if (avio_seek(s->pb, base + vbrtag_size + last_eight_frames_offset, SEEK_SET) < 0)
+	return;
+
+    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 +356,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 +382,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;
 }