| Message ID | 20171005013419.61600-1-lukas@stabe.de |
|---|---|
| State | Accepted |
| Commit | 1fd80106be3dca9fa0ea13fb364c8d221bd27c15 |
| Headers | show |
On 10/5/17, Lukas Stabe <lukas@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(-) > Fix? It wasn't broken at all.
> On 5. Oct 2017, at 09:08, Paul B Mahol <onemda@gmail.com> wrote: > > On 10/5/17, Lukas Stabe <lukas@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(-) >> > > > Fix? It wasn't broken at all. It was. I forgot to link the issue in the commit message, but see here https://trac.ffmpeg.org/ticket/6558 for details on how it was broken and when.
On Thu, 5 Oct 2017 03:34:19 +0200 Lukas Stabe <lukas@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"); > } I like this much more. Does fate pass? Will probably apply later this day.
> On 5. Oct 2017, at 10:51, wm4 <nfxjfg@googlemail.com> wrote: > > On Thu, 5 Oct 2017 03:34:19 +0200 > Lukas Stabe <lukas@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"); >> } > > I like this much more. Does fate pass? Will probably apply later this > day. Yup, fate passes. I haven't yet added a fate test for id3 chapters, but will probably follow up with a patch for one when I get around to throwing together a sample file.
On Thu, 5 Oct 2017 03:34:19 +0200 Lukas Stabe <lukas@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.
2017-10-05 17:13 GMT+02:00 wm4 <nfxjfg@googlemail.com>:
> Pushed, with commit message adjusted to 72 columns limit.
You forgot to add the ticket number.
Carl Eugen
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"); }