Mattias Wadman: > On Thu, Apr 23, 2020 at 5:15 PM Mattias Wadman <mattias.wad...@gmail.com> > wrote: >> >> lavf flacenc could previously write truncated metadata picture size if >> the picture data did not fit in 24 bits. Detect this by truncting the >> size found inside the picture block and if it matches the block size >> use it and read rest of picture data. >> >> Also only enable this workaround flac files and not ogg files with flac >> METADATA_BLOCK_PICTURE comment. >> >> flacenc was fixed in e447a4d112bcfee10126c54eb4481fa8712957c8 >> before the fix a broken flac for reproduction could be generated with: >> ffmpeg -f lavfi -i sine -f lavfi -i color=red:size=2400x2400 -map 0:0 -map >> 1:0 -c:v:0 bmp -disposition:1 attached_pic -t 1 test.flac >> >> Fixes ticket 6333 >> --- >> libavformat/flac_picture.c | 27 ++++++++++++++++++++++++--- >> libavformat/flac_picture.h | 2 +- >> libavformat/flacdec.c | 2 +- >> libavformat/oggparsevorbis.c | 2 +- >> 4 files changed, 27 insertions(+), 6 deletions(-) >> >> diff --git a/libavformat/flac_picture.c b/libavformat/flac_picture.c >> index 81ddf80465..f59ec8843f 100644 >> --- a/libavformat/flac_picture.c >> +++ b/libavformat/flac_picture.c >> @@ -27,16 +27,17 @@ >> #include "id3v2.h" >> #include "internal.h" >> >> -int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size) >> +int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size, >> int truncate_workaround) >> { >> const CodecMime *mime = ff_id3v2_mime_tags; >> enum AVCodecID id = AV_CODEC_ID_NONE; >> AVBufferRef *data = NULL; >> - uint8_t mimetype[64], *desc = NULL; >> + uint8_t mimetype[64], *desc = NULL, *picturebuf = NULL; >> GetByteContext g; >> AVStream *st; >> int width, height, ret = 0; >> - unsigned int len, type; >> + unsigned int type; >> + uint32_t len, left; >> >> if (buf_size < 34) { >> av_log(s, AV_LOG_ERROR, "Attached picture metadata block too >> short\n"); >> @@ -114,6 +115,25 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t >> *buf, int buf_size) >> >> /* picture data */ >> len = bytestream2_get_be32u(&g); >> + >> + // Workaround lavf flacenc bug that allowed writing truncated metadata >> picture block size if >> + // picture size did not fit in 24 bits >> + left = bytestream2_get_bytes_left(&g); >> + if (truncate_workaround && len > left && (len & 0xffffff) == left) { >> + uint32_t lendiff; >> + >> + av_log(s, AV_LOG_INFO, "Correcting truncated metadata picture size >> from %d to %d\n", left, len); >> + >> + picturebuf = av_malloc(len); >> + if (!picturebuf) >> + RETURN_ERROR(AVERROR(ENOMEM)); >> + lendiff = len - left; >> + bytestream2_get_buffer(&g, picturebuf, left); >> + if (avio_read(s->pb, picturebuf+left, lendiff) < lendiff) >> + RETURN_ERROR(AVERROR_INVALIDDATA); >> + bytestream2_init(&g, picturebuf, len); >> + } >> + >> if (len <= 0 || len > bytestream2_get_bytes_left(&g)) { >> av_log(s, AV_LOG_ERROR, "Attached picture metadata block too >> short\n"); >> if (s->error_recognition & AV_EF_EXPLODE) >> @@ -155,6 +175,7 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t >> *buf, int buf_size) >> fail: >> av_buffer_unref(&data); >> av_freep(&desc); >> + av_freep(&picturebuf); >> >> return ret; >> } >> diff --git a/libavformat/flac_picture.h b/libavformat/flac_picture.h >> index 4374b6f4f6..61fd0c8806 100644 >> --- a/libavformat/flac_picture.h >> +++ b/libavformat/flac_picture.h >> @@ -26,6 +26,6 @@ >> >> #define RETURN_ERROR(code) do { ret = (code); goto fail; } while (0) >> >> -int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size); >> +int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size, >> int truncate_workaround); >> >> #endif /* AVFORMAT_FLAC_PICTURE_H */ >> diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c >> index cb516fb1f3..79c05f14bf 100644 >> --- a/libavformat/flacdec.c >> +++ b/libavformat/flacdec.c >> @@ -146,7 +146,7 @@ static int flac_read_header(AVFormatContext *s) >> } >> av_freep(&buffer); >> } else if (metadata_type == FLAC_METADATA_TYPE_PICTURE) { >> - ret = ff_flac_parse_picture(s, buffer, metadata_size); >> + ret = ff_flac_parse_picture(s, buffer, metadata_size, 1); >> av_freep(&buffer); >> if (ret < 0) { >> av_log(s, AV_LOG_ERROR, "Error parsing attached >> picture.\n"); >> diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c >> index 27d2c686b6..6f15204ada 100644 >> --- a/libavformat/oggparsevorbis.c >> +++ b/libavformat/oggparsevorbis.c >> @@ -165,7 +165,7 @@ int ff_vorbis_comment(AVFormatContext *as, AVDictionary >> **m, >> av_freep(&tt); >> av_freep(&ct); >> if (ret > 0) >> - ret = ff_flac_parse_picture(as, pict, ret); >> + 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"); >> -- >> 2.18.0 >> > > Trying to find what is the convention for send patch updates. Use git > send-email and manually edit and add version to subject? > > -Mattias
git send-email accepts options for git format-patch and the latter has an option for a version field: reroll-count. - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".