diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c index 5ae931da9b..59493033db 100644 --- a/libavformat/id3v2.c +++ b/libavformat/id3v2.c @@ -487,7 +487,7 @@ static void list_append(ID3v2ExtraMeta *new_elem, ExtraMetaList *list) * Parse GEOB tag into a ID3v2ExtraMetaGEOB struct. */ static void read_geobtag(AVFormatContext *s, AVIOContext *pb, int taglen, - const char *tag, ExtraMetaList *extra_meta, int isv34) + const char *tag, ExtraMetaList *extra_meta, int version) { ID3v2ExtraMetaGEOB *geob_data = NULL; ID3v2ExtraMeta *new_extra = NULL; @@ -618,7 +618,7 @@ static void rstrip_spaces(char *buf) } static void read_apic(AVFormatContext *s, AVIOContext *pb, int taglen, - const char *tag, ExtraMetaList *extra_meta, int isv34) + const char *tag, ExtraMetaList *extra_meta, int version) { int enc, pic_type; char mimetype[64] = {0}; @@ -628,7 +628,7 @@ static void read_apic(AVFormatContext *s, AVIOContext *pb, int taglen, ID3v2ExtraMeta *new_extra = NULL; int64_t end = avio_tell(pb) + taglen; - if (taglen <= 4 || (!isv34 && taglen <= 6)) + if (taglen <= 4 || (version == 2 && taglen <= 6)) goto fail; new_extra = av_mallocz(sizeof(*new_extra)); @@ -641,7 +641,7 @@ static void read_apic(AVFormatContext *s, AVIOContext *pb, int taglen, taglen--; /* mimetype */ - if (isv34) { + if (version != 2) { int ret = avio_get_str(pb, taglen, mimetype, sizeof(mimetype)); if (ret < 0 || ret >= taglen) goto fail; @@ -714,9 +714,8 @@ static void free_chapter(void *obj) } static void read_chapter(AVFormatContext *s, AVIOContext *pb, int len, - const char *ttag, ExtraMetaList *extra_meta, int isv34) + const char *ttag, ExtraMetaList *extra_meta, int version) { - int taglen; char tag[5]; ID3v2ExtraMeta *new_extra = NULL; ID3v2ExtraMetaCHAP *chap = NULL; @@ -739,19 +738,30 @@ static void read_chapter(AVFormatContext *s, AVIOContext *pb, int len, len -= 16; while (len > 10) { + unsigned tlen; + if (avio_read(pb, tag, 4) < 4) - goto fail; + break; tag[4] = 0; - taglen = avio_rb32(pb); + /* Embedded frames use the same header as the surrounding tag. + * 2.2 has no CHAP. 2.3 sizes are plain; 2.4 sizes are synchsafe. */ + if (version == 4) + tlen = get_size(pb, 4); + else + tlen = avio_rb32(pb); avio_skip(pb, 2); len -= 10; - if (taglen < 0 || taglen > len) - goto fail; + if (tlen > (unsigned)len) { + av_log(s, AV_LOG_WARNING, + "Invalid ID3v2 chapter subframe %.4s size %u, keeping the chapter\n", + tag, tlen); + break; + } if (tag[0] == 'T') - read_ttag(s, pb, taglen, &chap->meta, tag); + read_ttag(s, pb, tlen, &chap->meta, tag); else - avio_skip(pb, taglen); - len -= taglen; + avio_skip(pb, tlen); + len -= tlen; } ff_metadata_conv(&chap->meta, NULL, ff_id3v2_34_metadata_conv); @@ -775,7 +785,7 @@ static void free_priv(void *obj) } static void read_priv(AVFormatContext *s, AVIOContext *pb, int taglen, - const char *tag, ExtraMetaList *extra_meta, int isv34) + const char *tag, ExtraMetaList *extra_meta, int version) { ID3v2ExtraMeta *meta; ID3v2ExtraMetaPRIV *priv; @@ -813,7 +823,7 @@ typedef struct ID3v2EMFunc { const char *tag4; void (*read)(AVFormatContext *s, AVIOContext *pb, int taglen, const char *tag, ExtraMetaList *extra_meta, - int isv34); + int version); void (*free)(void *obj); } ID3v2EMFunc; @@ -1092,7 +1102,7 @@ static void id3v2_parse(AVIOContext *pb, AVDictionary **metadata, read_lang_descr_tag(s, pbx, "comment", tlen, metadata); else /* parse special meta tag */ - extra_func->read(s, pbx, tlen, tag, extra_meta, isv34); + extra_func->read(s, pbx, tlen, tag, extra_meta, version); } else if (!tag[0]) { if (tag[1]) av_log(s, AV_LOG_WARNING, "invalid frame id, assuming padding\n"); diff --git a/libavformat/id3v2enc.c b/libavformat/id3v2enc.c index 9206af8837..c538d13829 100644 --- a/libavformat/id3v2enc.c +++ b/libavformat/id3v2enc.c @@ -60,6 +60,18 @@ static void id3v2_encode_string(AVIOContext *pb, const uint8_t *str, put(pb, str); } +/* ID3v2.3 frame size is not synchsafe; 2.4 is. */ +static void id3v2_put_frame_header(ID3v2EncContext *id3, AVIOContext *pb, + uint32_t tag, int len, int flags) +{ + avio_wb32(pb, tag); + if (id3->version == 3) + avio_wb32(pb, len); + else + id3v2_put_size(pb, len); + avio_wb16(pb, flags); +} + static int id3v2_put_frame(ID3v2EncContext *id3, AVIOContext *avioc, AVIOContext *dyn_buf, const uint32_t tag, const uint8_t flags) { @@ -67,13 +79,7 @@ static int id3v2_put_frame(ID3v2EncContext *id3, AVIOContext *avioc, AVIOContext uint8_t *pb; len = avio_get_dyn_buf(dyn_buf, &pb); - avio_wb32(avioc, tag); - /* ID3v2.3 frame size is not sync-safe */ - if (id3->version == 3) - avio_wb32(avioc, len); - else - id3v2_put_size(avioc, len); - avio_wb16(avioc, flags); + id3v2_put_frame_header(id3, avioc, tag, len, flags); avio_write(avioc, pb, len); id3->len += len + ID3v2_HEADER_SIZE; @@ -463,9 +469,9 @@ static int write_chapter(AVFormatContext *s, AVIOContext *pb, len = avio_get_dyn_buf(dyn_bc, &dyn_buf); id3->len += 16 + ID3v2_HEADER_SIZE; - avio_wb32(pb, MKBETAG('C', 'H', 'A', 'P')); - avio_wb32(pb, len); - avio_wb16(pb, 0); + /* Nested frames already updated id3->len, so this header is written + * directly instead of through id3v2_put_frame(). */ + id3v2_put_frame_header(id3, pb, MKBETAG('C', 'H', 'A', 'P'), len, 0); avio_write(pb, dyn_buf, len); fail: