[FFmpeg-devel] [PATCH] avformat: fix id3 chapters

wm4 nfxjfg at googlemail.com
Thu Oct 5 18:13:15 EEST 2017


On Thu,  5 Oct 2017 03:34:19 +0200
Lukas Stabe <lukas at stabe.de> wrote:

> These changes store id3 chapter data in ID3v2ExtraMeta and introduce ff_id3v2_parse_chapters to parse them into the format context if needed.
> 
> Encoders using ff_id3v2_read, which previously parsed chapters into the format context automatically, were adjusted to call ff_id3v2_parse_chapters.
> ---
>  libavformat/aiffdec.c  |   3 +-
>  libavformat/asfdec_f.c |   4 +-
>  libavformat/asfdec_o.c |   4 +-
>  libavformat/dsfdec.c   |   4 +-
>  libavformat/id3v2.c    | 120 ++++++++++++++++++++++++++++++++++++-------------
>  libavformat/id3v2.h    |  15 +++++--
>  libavformat/iff.c      |   3 +-
>  libavformat/omadec.c   |   5 +++
>  libavformat/utils.c    |   2 +
>  9 files changed, 122 insertions(+), 38 deletions(-)
> 
> diff --git a/libavformat/aiffdec.c b/libavformat/aiffdec.c
> index 2ef9386edb..99e05c78ec 100644
> --- a/libavformat/aiffdec.c
> +++ b/libavformat/aiffdec.c
> @@ -258,7 +258,8 @@ static int aiff_read_header(AVFormatContext *s)
>              position = avio_tell(pb);
>              ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta, size);
>              if (id3v2_extra_meta)
> -                if ((ret = ff_id3v2_parse_apic(s, &id3v2_extra_meta)) < 0) {
> +                if ((ret = ff_id3v2_parse_apic(s, &id3v2_extra_meta)) < 0 ||
> +                    (ret = ff_id3v2_parse_chapters(s, &id3v2_extra_meta)) < 0) {
>                      ff_id3v2_free_extra_meta(&id3v2_extra_meta);
>                      return ret;
>                  }
> diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
> index cc648b9a2f..64a0b9d7f2 100644
> --- a/libavformat/asfdec_f.c
> +++ b/libavformat/asfdec_f.c
> @@ -307,8 +307,10 @@ static void get_id3_tag(AVFormatContext *s, int len)
>      ID3v2ExtraMeta *id3v2_extra_meta = NULL;
>  
>      ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta, len);
> -    if (id3v2_extra_meta)
> +    if (id3v2_extra_meta) {
>          ff_id3v2_parse_apic(s, &id3v2_extra_meta);
> +        ff_id3v2_parse_chapters(s, &id3v2_extra_meta);
> +    }
>      ff_id3v2_free_extra_meta(&id3v2_extra_meta);
>  }
>  
> diff --git a/libavformat/asfdec_o.c b/libavformat/asfdec_o.c
> index 818d6f3573..5122e33c78 100644
> --- a/libavformat/asfdec_o.c
> +++ b/libavformat/asfdec_o.c
> @@ -460,8 +460,10 @@ static void get_id3_tag(AVFormatContext *s, int len)
>      ID3v2ExtraMeta *id3v2_extra_meta = NULL;
>  
>      ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta, len);
> -    if (id3v2_extra_meta)
> +    if (id3v2_extra_meta) {
>          ff_id3v2_parse_apic(s, &id3v2_extra_meta);
> +        ff_id3v2_parse_chapters(s, &id3v2_extra_meta);
> +    }
>      ff_id3v2_free_extra_meta(&id3v2_extra_meta);
>  }
>  
> diff --git a/libavformat/dsfdec.c b/libavformat/dsfdec.c
> index 49ca336427..41538fd83c 100644
> --- a/libavformat/dsfdec.c
> +++ b/libavformat/dsfdec.c
> @@ -53,8 +53,10 @@ static void read_id3(AVFormatContext *s, uint64_t id3pos)
>          return;
>  
>      ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta, 0);
> -    if (id3v2_extra_meta)
> +    if (id3v2_extra_meta) {
>          ff_id3v2_parse_apic(s, &id3v2_extra_meta);
> +        ff_id3v2_parse_chapters(s, &id3v2_extra_meta);
> +    }
>      ff_id3v2_free_extra_meta(&id3v2_extra_meta);
>  }
>  
> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
> index f15cefee47..6c216ba7a2 100644
> --- a/libavformat/id3v2.c
> +++ b/libavformat/id3v2.c
> @@ -670,59 +670,68 @@ fail:
>      avio_seek(pb, end, SEEK_SET);
>  }
>  
> +static void free_chapter(void *obj)
> +{
> +    ID3v2ExtraMetaCHAP *chap = obj;
> +    av_freep(&chap->element_id);
> +    av_dict_free(&chap->meta);
> +    av_freep(&chap);
> +}
> +
>  static void read_chapter(AVFormatContext *s, AVIOContext *pb, int len, const char *ttag, ID3v2ExtraMeta **extra_meta, int isv34)
>  {
> -    AVRational time_base = {1, 1000};
> -    uint32_t start, end;
> -    AVChapter *chapter;
> -    uint8_t *dst = NULL;
>      int taglen;
>      char tag[5];
> +    ID3v2ExtraMeta *new_extra = NULL;
> +    ID3v2ExtraMetaCHAP *chap  = NULL;
>  
> -    if (!s) {
> -        /* We should probably just put the chapter data to extra_meta here
> -         * and do the AVFormatContext-needing part in a separate
> -         * ff_id3v2_parse_apic()-like function. */
> -        av_log(NULL, AV_LOG_DEBUG, "No AVFormatContext, skipped ID3 chapter data\n");
> -        return;
> -    }
> +    new_extra = av_mallocz(sizeof(*new_extra));
> +    chap      = av_mallocz(sizeof(*chap));
> +
> +    if (!new_extra || !chap)
> +        goto fail;
> +
> +    if (decode_str(s, pb, 0, &chap->element_id, &len) < 0)
> +        goto fail;
>  
> -    if (decode_str(s, pb, 0, &dst, &len) < 0)
> -        return;
>      if (len < 16)
> -        return;
> +        goto fail;
>  
> -    start = avio_rb32(pb);
> -    end   = avio_rb32(pb);
> +    chap->start = avio_rb32(pb);
> +    chap->end   = avio_rb32(pb);
>      avio_skip(pb, 8);
>  
> -    chapter = avpriv_new_chapter(s, s->nb_chapters + 1, time_base, start, end, dst);
> -    if (!chapter) {
> -        av_free(dst);
> -        return;
> -    }
> -
>      len -= 16;
>      while (len > 10) {
>          if (avio_read(pb, tag, 4) < 4)
> -            goto end;
> +            goto fail;
>          tag[4] = 0;
>          taglen = avio_rb32(pb);
>          avio_skip(pb, 2);
>          len -= 10;
>          if (taglen < 0 || taglen > len)
> -            goto end;
> +            goto fail;
>          if (tag[0] == 'T')
> -            read_ttag(s, pb, taglen, &chapter->metadata, tag);
> +            read_ttag(s, pb, taglen, &chap->meta, tag);
>          else
>              avio_skip(pb, taglen);
>          len -= taglen;
>      }
>  
> -    ff_metadata_conv(&chapter->metadata, NULL, ff_id3v2_34_metadata_conv);
> -    ff_metadata_conv(&chapter->metadata, NULL, ff_id3v2_4_metadata_conv);
> -end:
> -    av_free(dst);
> +    ff_metadata_conv(&chap->meta, NULL, ff_id3v2_34_metadata_conv);
> +    ff_metadata_conv(&chap->meta, NULL, ff_id3v2_4_metadata_conv);
> +
> +    new_extra->tag  = "CHAP";
> +    new_extra->data = chap;
> +    new_extra->next = *extra_meta;
> +    *extra_meta     = new_extra;
> +
> +    return;
> +
> +fail:
> +    if (chap)
> +        free_chapter(chap);
> +    av_freep(&new_extra);
>  }
>  
>  static void free_priv(void *obj)
> @@ -782,7 +791,7 @@ typedef struct ID3v2EMFunc {
>  static const ID3v2EMFunc id3v2_extra_meta_funcs[] = {
>      { "GEO", "GEOB", read_geobtag, free_geobtag },
>      { "PIC", "APIC", read_apic,    free_apic    },
> -    { "CHAP","CHAP", read_chapter, NULL         },
> +    { "CHAP","CHAP", read_chapter, free_chapter },
>      { "PRIV","PRIV", read_priv,    free_priv    },
>      { NULL }
>  };
> @@ -1164,3 +1173,54 @@ int ff_id3v2_parse_apic(AVFormatContext *s, ID3v2ExtraMeta **extra_meta)
>  
>      return 0;
>  }
> +
> +int ff_id3v2_parse_chapters(AVFormatContext *s, ID3v2ExtraMeta **extra_meta)
> +{
> +    int ret = 0;
> +    ID3v2ExtraMeta *cur;
> +    AVRational time_base = {1, 1000};
> +    ID3v2ExtraMetaCHAP **chapters = NULL;
> +    int num_chapters = 0;
> +    int i;
> +
> +    // since extra_meta is a linked list where elements are prepended,
> +    // we need to reverse the order of chapters
> +    for (cur = *extra_meta; cur; cur = cur->next) {
> +        ID3v2ExtraMetaCHAP *chap;
> +
> +        if (strcmp(cur->tag, "CHAP"))
> +            continue;
> +        chap = cur->data;
> +
> +        if ((ret = av_dynarray_add_nofree(&chapters, &num_chapters, chap)) < 0)
> +            goto end;
> +    }
> +
> +    for (i = 0; i < (num_chapters / 2); i++) {
> +        ID3v2ExtraMetaCHAP *right;
> +        int right_index;
> +
> +        right_index = (num_chapters - 1) - i;
> +        right = chapters[right_index];
> +
> +        chapters[right_index] = chapters[i];
> +        chapters[i] = right;
> +    }
> +
> +    for (i = 0; i < num_chapters; i++) {
> +        ID3v2ExtraMetaCHAP *chap;
> +        AVChapter *chapter;
> +
> +        chap = chapters[i];
> +        chapter = avpriv_new_chapter(s, i, time_base, chap->start, chap->end, chap->element_id);
> +        if (!chapter)
> +            continue;
> +
> +        if ((ret = av_dict_copy(&chapter->metadata, chap->meta, 0)) < 0)
> +            goto end;
> +    }
> +
> +end:
> +    av_freep(&chapters);
> +    return ret;
> +}
> diff --git a/libavformat/id3v2.h b/libavformat/id3v2.h
> index 6e7a8c9abf..5e64ead096 100644
> --- a/libavformat/id3v2.h
> +++ b/libavformat/id3v2.h
> @@ -79,6 +79,12 @@ typedef struct ID3v2ExtraMetaPRIV {
>      uint32_t datasize;
>  } ID3v2ExtraMetaPRIV;
>  
> +typedef struct ID3v2ExtraMetaCHAP {
> +    uint8_t *element_id;
> +    uint32_t start, end;
> +    AVDictionary *meta;
> +} ID3v2ExtraMetaCHAP;
> +
>  /**
>   * Detect ID3v2 Header.
>   * @param buf   must be ID3v2_HEADER_SIZE byte long
> @@ -97,8 +103,6 @@ int ff_id3v2_tag_len(const uint8_t *buf);
>  /**
>   * Read an ID3v2 tag into specified dictionary and retrieve supported extra metadata.
>   *
> - * Chapters are not currently read by this variant.
> - *
>   * @param metadata Parsed metadata is stored here
>   * @param extra_meta If not NULL, extra metadata is parsed into a list of
>   * ID3v2ExtraMeta structs and *extra_meta points to the head of the list
> @@ -106,7 +110,7 @@ int ff_id3v2_tag_len(const uint8_t *buf);
>  void ff_id3v2_read_dict(AVIOContext *pb, AVDictionary **metadata, const char *magic, ID3v2ExtraMeta **extra_meta);
>  
>  /**
> - * Read an ID3v2 tag, including supported extra metadata and chapters.
> + * Read an ID3v2 tag, including supported extra metadata.
>   *
>   * Data is read from and stored to AVFormatContext.
>   *
> @@ -158,6 +162,11 @@ void ff_id3v2_free_extra_meta(ID3v2ExtraMeta **extra_meta);
>   */
>  int ff_id3v2_parse_apic(AVFormatContext *s, ID3v2ExtraMeta **extra_meta);
>  
> +/**
> + * Create chapters for all CHAP tags found in the ID3v2 header.
> + */
> +int ff_id3v2_parse_chapters(AVFormatContext *s, ID3v2ExtraMeta **extra_meta);
> +
>  extern const AVMetadataConv ff_id3v2_34_metadata_conv[];
>  extern const AVMetadataConv ff_id3v2_4_metadata_conv[];
>  
> diff --git a/libavformat/iff.c b/libavformat/iff.c
> index 6a09eb0e31..4cf17f6e1a 100644
> --- a/libavformat/iff.c
> +++ b/libavformat/iff.c
> @@ -312,7 +312,8 @@ static int parse_dsd_prop(AVFormatContext *s, AVStream *st, uint64_t eof)
>              id3v2_extra_meta = NULL;
>              ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta, size);
>              if (id3v2_extra_meta) {
> -                if ((ret = ff_id3v2_parse_apic(s, &id3v2_extra_meta)) < 0) {
> +                if ((ret = ff_id3v2_parse_apic(s, &id3v2_extra_meta)) < 0 ||
> +                    (ret = ff_id3v2_parse_chapters(s, &id3v2_extra_meta)) < 0) {
>                      ff_id3v2_free_extra_meta(&id3v2_extra_meta);
>                      return ret;
>                  }
> diff --git a/libavformat/omadec.c b/libavformat/omadec.c
> index fa53636f1a..423d52b3aa 100644
> --- a/libavformat/omadec.c
> +++ b/libavformat/omadec.c
> @@ -397,6 +397,11 @@ static int oma_read_header(AVFormatContext *s)
>      OMAContext *oc = s->priv_data;
>  
>      ff_id3v2_read(s, ID3v2_EA3_MAGIC, &extra_meta, 0);
> +    if ((ret = ff_id3v2_parse_chapters(s, &extra_meta)) < 0) {
> +        ff_id3v2_free_extra_meta(&extra_meta);
> +        return ret;
> +    }
> +
>      ret = avio_read(s->pb, buf, EA3_HEADER_SIZE);
>      if (ret < EA3_HEADER_SIZE)
>          return -1;
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 7abca632b5..1a7996c4fd 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -613,6 +613,8 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
>              !strcmp(s->iformat->name, "tta")) {
>              if ((ret = ff_id3v2_parse_apic(s, &id3v2_extra_meta)) < 0)
>                  goto fail;
> +            if ((ret = ff_id3v2_parse_chapters(s, &id3v2_extra_meta)) < 0)
> +                goto fail;
>          } else
>              av_log(s, AV_LOG_DEBUG, "demuxer does not support additional id3 data, skipping\n");
>      }

Pushed, with commit message adjusted to 72 columns limit.


More information about the ffmpeg-devel mailing list