From patchwork Thu Apr 23 15:15:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mattias Wadman X-Patchwork-Id: 19200 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 1CF0C44A051 for ; Thu, 23 Apr 2020 18:15:52 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E6B5968BEB0; Thu, 23 Apr 2020 18:15:51 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 8446968BABB for ; Thu, 23 Apr 2020 18:15:45 +0300 (EEST) Received: by mail-wm1-f51.google.com with SMTP id 188so6822584wmc.2 for ; Thu, 23 Apr 2020 08:15:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=5/gXyITvmX3um5WzOQoEIl4BvZ38TfIAeygW3o+Z/Xg=; b=pf+phZBeeNkWo6spK5INSLA2WYt75iUb0dVYVJKHY6y+7//zPqwLfgY3wGn8gyZEvg nRYV7t7ukw95ZLo5QAqvmk7oN2pRJXB6iF8XmpQV3l/GgvlBXbjDgk1jzG0yfVp7TnPw pBzxwhw9LdjuqvtNi9cqbR3zMcK9UBqyytkhPt0cZXVlTZQ0CJZlP7O63jaE0rnyAVHk 02hCQSQ1QrxNoy/S5H9V/dY2Y9xuq1HPC9PJMGBizxmoEmgeSM6lPLb8ovwA2REYopK4 mmrdcmsIVpUtDMQJWyJdHC9mZKKSWTbtm5EqosGFLLP5/eDJAY7XCiuRfLEJ1Gs9xZsO qgjA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=5/gXyITvmX3um5WzOQoEIl4BvZ38TfIAeygW3o+Z/Xg=; b=Gy19FKpHRNfbMVtGgcQC78FQLI3TOXaivk0o46E08PULaqU7TWhceeTkodz5iAux0N NOj3JZUUbkJegGK4JYcrK999yBwHicoSEbCUDtW997nGmQNNehzXswgGZ0FSe0C6dtE8 uq7R9XZHwyGRphDFrOLJLb9NuDGU8j8oSuTJDAetV3DnqzNwFY8eGxrpImGXjcCRIvpA zZXExW+aOyncFP0roVeDgitm+cF5XN5b4P4QCclxLvBYUZt1MPD7wmmRQGump5cdmmSB kZG8juJSrdQ0PSKmOoKgTw0rHCJ6EHQmSxaPDuVNXtO90lWidJB4GTebkO8XloGIccB+ FCiA== X-Gm-Message-State: AGi0PuZjINbNwKpdoodLX3RP/QeW1hH+L8MHU7Sgw0WzpPHTZ8Ex9DIX A+ZEvbVWwpVq/nZd2bZLPrNWVZKNINw= X-Google-Smtp-Source: APiQypJ66tUXqVnvbZEHF3LkTkrinQECuExoq3D0RDvVd+b6Y1g/rJpqzgFhhRMndcTrNmVH7z4n5w== X-Received: by 2002:a05:600c:c9:: with SMTP id u9mr4615502wmm.15.1587654944292; Thu, 23 Apr 2020 08:15:44 -0700 (PDT) Received: from work-2.local.net (c-1d85e253.016-130-73746f46.bbcust.telenor.se. [83.226.133.29]) by smtp.gmail.com with ESMTPSA id n2sm4515270wrq.74.2020.04.23.08.15.42 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 23 Apr 2020 08:15:43 -0700 (PDT) From: Mattias Wadman To: ffmpeg-devel@ffmpeg.org Date: Thu, 23 Apr 2020 17:15:23 +0200 Message-Id: <20200423151522.59081-1-mattias.wadman@gmail.com> X-Mailer: git-send-email 2.18.0 Subject: [FFmpeg-devel] [PATCH] libavformat/flacdec: Workaround for truncated metadata picture size X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Mattias Wadman MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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");