| Message ID | AM7PR03MB6660A229FA9A243E46078F548FC49@AM7PR03MB6660.eurprd03.prod.outlook.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/8] avformat/vorbiscomment: Don't compute strlen twice | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Andreas Rheinhardt: > This is in preparation for further commits. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavformat/oggparsevorbis.c | 142 ++++++++++++++++++----------------- > 1 file changed, 75 insertions(+), 67 deletions(-) > > diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c > index c48658ceda..69c15c3dce 100644 > --- a/libavformat/oggparsevorbis.c > +++ b/libavformat/oggparsevorbis.c > @@ -84,6 +84,77 @@ int ff_vorbis_stream_comment(AVFormatContext *as, AVStream *st, > return updates; > } > > +static int vorbis_parse_single_comment(AVFormatContext *as, AVDictionary **m, > + const uint8_t *buf, uint32_t size, > + int *updates, int parse_picture) > +{ > + const char *t = buf, *v = memchr(t, '=', size); > + char *tt, *ct; > + int tl, vl; > + > + if (!v) > + return 0; > + > + tl = v - t; > + vl = size - tl - 1; > + v++; > + > + if (!tl || !vl) > + return 0; > + > + tt = av_malloc(tl + 1); > + ct = av_malloc(vl + 1); > + if (!tt || !ct) { > + av_freep(&tt); > + av_freep(&ct); > + return AVERROR(ENOMEM); > + } > + > + memcpy(tt, t, tl); > + tt[tl] = 0; > + > + memcpy(ct, v, vl); > + ct[vl] = 0; > + > + /* The format in which the pictures are stored is the FLAC format. > + * Xiph says: "The binary FLAC picture structure is base64 encoded > + * and placed within a VorbisComment with the tag name > + * 'METADATA_BLOCK_PICTURE'. This is the preferred and > + * recommended way of embedding cover art within VorbisComments." > + */ > + if (!av_strcasecmp(tt, "METADATA_BLOCK_PICTURE") && parse_picture) { > + int ret, len = AV_BASE64_DECODE_SIZE(vl); > + uint8_t *pict = av_malloc(len); > + > + if (!pict) { > + av_log(as, AV_LOG_WARNING, "out-of-memory error. Skipping cover art block.\n"); > + av_freep(&tt); > + av_freep(&ct); > + return 0; > + } > + ret = av_base64_decode(pict, ct, len); > + av_freep(&tt); > + av_freep(&ct); > + if (ret > 0) > + ret = ff_flac_parse_picture(as, pict, ret, 0); > + av_freep(&pict); > + if (ret < 0) { > + av_log(as, AV_LOG_WARNING, "Failed to parse cover art block.\n"); > + return 0; > + } > + } else if (!ogm_chapter(as, tt, ct)) { > + (*updates)++; > + if (av_dict_get(*m, tt, NULL, 0)) { > + av_dict_set(m, tt, ";", AV_DICT_APPEND); > + } > + av_dict_set(m, tt, ct, > + AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL | > + AV_DICT_APPEND); > + } > + > + return 0; > +} > + > int ff_vorbis_comment(AVFormatContext *as, AVDictionary **m, > const uint8_t *buf, int size, > int parse_picture) > @@ -92,7 +163,7 @@ int ff_vorbis_comment(AVFormatContext *as, AVDictionary **m, > const uint8_t *end = buf + size; > int updates = 0; > unsigned n; > - int s; > + int s, ret; > > /* must have vendor_length and user_comment_list_length */ > if (size < 8) > @@ -108,79 +179,16 @@ int ff_vorbis_comment(AVFormatContext *as, AVDictionary **m, > n = bytestream_get_le32(&p); > > while (end - p >= 4 && n > 0) { > - const char *t, *v; > - int tl, vl; > - > s = bytestream_get_le32(&p); > > if (end - p < s || s < 0) > break; > > - t = p; > + ret = vorbis_parse_single_comment(as, m, p, s, &updates, parse_picture); > + if (ret < 0) > + return ret; > p += s; > n--; > - > - v = memchr(t, '=', s); > - if (!v) > - continue; > - > - tl = v - t; > - vl = s - tl - 1; > - v++; > - > - if (tl && vl) { > - char *tt, *ct; > - > - tt = av_malloc(tl + 1); > - ct = av_malloc(vl + 1); > - if (!tt || !ct) { > - av_freep(&tt); > - av_freep(&ct); > - return AVERROR(ENOMEM); > - } > - > - memcpy(tt, t, tl); > - tt[tl] = 0; > - > - memcpy(ct, v, vl); > - ct[vl] = 0; > - > - /* The format in which the pictures are stored is the FLAC format. > - * Xiph says: "The binary FLAC picture structure is base64 encoded > - * and placed within a VorbisComment with the tag name > - * 'METADATA_BLOCK_PICTURE'. This is the preferred and > - * recommended way of embedding cover art within VorbisComments." > - */ > - if (!av_strcasecmp(tt, "METADATA_BLOCK_PICTURE") && parse_picture) { > - int ret, len = AV_BASE64_DECODE_SIZE(vl); > - char *pict = av_malloc(len); > - > - if (!pict) { > - av_log(as, AV_LOG_WARNING, "out-of-memory error. Skipping cover art block.\n"); > - av_freep(&tt); > - av_freep(&ct); > - continue; > - } > - ret = av_base64_decode(pict, ct, len); > - av_freep(&tt); > - av_freep(&ct); > - if (ret > 0) > - ret = ff_flac_parse_picture(as, pict, ret, 0); > - av_freep(&pict); > - if (ret < 0) { > - av_log(as, AV_LOG_WARNING, "Failed to parse cover art block.\n"); > - continue; > - } > - } else if (!ogm_chapter(as, tt, ct)) { > - updates++; > - if (av_dict_get(*m, tt, NULL, 0)) { > - av_dict_set(m, tt, ";", AV_DICT_APPEND); > - } > - av_dict_set(m, tt, ct, > - AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL | > - AV_DICT_APPEND); > - } > - } > } > > if (p != end) > Will apply the rest of this patchset tomorrow unless there are objections. - Andreas
diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c index c48658ceda..69c15c3dce 100644 --- a/libavformat/oggparsevorbis.c +++ b/libavformat/oggparsevorbis.c @@ -84,6 +84,77 @@ int ff_vorbis_stream_comment(AVFormatContext *as, AVStream *st, return updates; } +static int vorbis_parse_single_comment(AVFormatContext *as, AVDictionary **m, + const uint8_t *buf, uint32_t size, + int *updates, int parse_picture) +{ + const char *t = buf, *v = memchr(t, '=', size); + char *tt, *ct; + int tl, vl; + + if (!v) + return 0; + + tl = v - t; + vl = size - tl - 1; + v++; + + if (!tl || !vl) + return 0; + + tt = av_malloc(tl + 1); + ct = av_malloc(vl + 1); + if (!tt || !ct) { + av_freep(&tt); + av_freep(&ct); + return AVERROR(ENOMEM); + } + + memcpy(tt, t, tl); + tt[tl] = 0; + + memcpy(ct, v, vl); + ct[vl] = 0; + + /* The format in which the pictures are stored is the FLAC format. + * Xiph says: "The binary FLAC picture structure is base64 encoded + * and placed within a VorbisComment with the tag name + * 'METADATA_BLOCK_PICTURE'. This is the preferred and + * recommended way of embedding cover art within VorbisComments." + */ + if (!av_strcasecmp(tt, "METADATA_BLOCK_PICTURE") && parse_picture) { + int ret, len = AV_BASE64_DECODE_SIZE(vl); + uint8_t *pict = av_malloc(len); + + if (!pict) { + av_log(as, AV_LOG_WARNING, "out-of-memory error. Skipping cover art block.\n"); + av_freep(&tt); + av_freep(&ct); + return 0; + } + ret = av_base64_decode(pict, ct, len); + av_freep(&tt); + av_freep(&ct); + if (ret > 0) + ret = ff_flac_parse_picture(as, pict, ret, 0); + av_freep(&pict); + if (ret < 0) { + av_log(as, AV_LOG_WARNING, "Failed to parse cover art block.\n"); + return 0; + } + } else if (!ogm_chapter(as, tt, ct)) { + (*updates)++; + if (av_dict_get(*m, tt, NULL, 0)) { + av_dict_set(m, tt, ";", AV_DICT_APPEND); + } + av_dict_set(m, tt, ct, + AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL | + AV_DICT_APPEND); + } + + return 0; +} + int ff_vorbis_comment(AVFormatContext *as, AVDictionary **m, const uint8_t *buf, int size, int parse_picture) @@ -92,7 +163,7 @@ int ff_vorbis_comment(AVFormatContext *as, AVDictionary **m, const uint8_t *end = buf + size; int updates = 0; unsigned n; - int s; + int s, ret; /* must have vendor_length and user_comment_list_length */ if (size < 8) @@ -108,79 +179,16 @@ int ff_vorbis_comment(AVFormatContext *as, AVDictionary **m, n = bytestream_get_le32(&p); while (end - p >= 4 && n > 0) { - const char *t, *v; - int tl, vl; - s = bytestream_get_le32(&p); if (end - p < s || s < 0) break; - t = p; + ret = vorbis_parse_single_comment(as, m, p, s, &updates, parse_picture); + if (ret < 0) + return ret; p += s; n--; - - v = memchr(t, '=', s); - if (!v) - continue; - - tl = v - t; - vl = s - tl - 1; - v++; - - if (tl && vl) { - char *tt, *ct; - - tt = av_malloc(tl + 1); - ct = av_malloc(vl + 1); - if (!tt || !ct) { - av_freep(&tt); - av_freep(&ct); - return AVERROR(ENOMEM); - } - - memcpy(tt, t, tl); - tt[tl] = 0; - - memcpy(ct, v, vl); - ct[vl] = 0; - - /* The format in which the pictures are stored is the FLAC format. - * Xiph says: "The binary FLAC picture structure is base64 encoded - * and placed within a VorbisComment with the tag name - * 'METADATA_BLOCK_PICTURE'. This is the preferred and - * recommended way of embedding cover art within VorbisComments." - */ - if (!av_strcasecmp(tt, "METADATA_BLOCK_PICTURE") && parse_picture) { - int ret, len = AV_BASE64_DECODE_SIZE(vl); - char *pict = av_malloc(len); - - if (!pict) { - av_log(as, AV_LOG_WARNING, "out-of-memory error. Skipping cover art block.\n"); - av_freep(&tt); - av_freep(&ct); - continue; - } - ret = av_base64_decode(pict, ct, len); - av_freep(&tt); - av_freep(&ct); - if (ret > 0) - ret = ff_flac_parse_picture(as, pict, ret, 0); - av_freep(&pict); - if (ret < 0) { - av_log(as, AV_LOG_WARNING, "Failed to parse cover art block.\n"); - continue; - } - } else if (!ogm_chapter(as, tt, ct)) { - updates++; - if (av_dict_get(*m, tt, NULL, 0)) { - av_dict_set(m, tt, ";", AV_DICT_APPEND); - } - av_dict_set(m, tt, ct, - AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL | - AV_DICT_APPEND); - } - } } if (p != end)
This is in preparation for further commits. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavformat/oggparsevorbis.c | 142 ++++++++++++++++++----------------- 1 file changed, 75 insertions(+), 67 deletions(-)