From patchwork Sat May 2 17:16:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 19435 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 EFA8344BB45 for ; Sat, 2 May 2020 20:17:24 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id CC8FB68C33A; Sat, 2 May 2020 20:17:24 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E1D3F68C33A for ; Sat, 2 May 2020 20:17:17 +0300 (EEST) Received: by mail-wr1-f65.google.com with SMTP id e16so10566519wra.7 for ; Sat, 02 May 2020 10:17:17 -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:in-reply-to:references :mime-version:content-transfer-encoding; bh=LBSyA3NtLESXj/TwG8SSD7XMK1uIE17+qP4IavnifrY=; b=j2bRnLt/lAJdeXpiBPG6KGRwQruc8dpNsqamwiNDF2qbHEhZvuMClWS2Op87stiWjp GyY5VzX7o/gJtOSiYBHYM0/VxWGOXBfWE7hsekGlNevyUaGn280ZjSx0VvZKCaAiZOyw ljLwsAy6n4lc5g4FIBZJWFHu9t4ZSAxQmepiiRC2UVEKAaDHv7jeBD/PJwShLCGnvRMf E+NHhBGpv9De+EI1QYqUc6l4GsDdjhR/F9X/fY8JEmOSLa4kYVK7A4EoidIRizC9EyZs uhB/jKv1HhTTZRzWxK7WybXBV3Xwn6geVUHRorrXysLcUsp1VO2lncYCuzCbbM4nE6T6 m+5g== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=LBSyA3NtLESXj/TwG8SSD7XMK1uIE17+qP4IavnifrY=; b=JYPEgf4TYYSF1FN18LmFtXQBZgF2v+hFwxOF3I+WGXYvTjucWXjIGBHKvh77MEdUdV M6+8s//BAIXer1k2jN0ck6eJfTaxtO9SDwjfwQKBWizFXBBhQ7RFcdueHu/0u9xzAQsP EwC+0UZHQGeF/9Pyj4nVsavfw8Qj60Yl2Q+WQmBwg0oh+fUMPAxz3ovrxYV5HiexJ5qg Vyz0X1wPRApW9EucA7VeNxN/t130kN6GxuZnyVGIh5D7CHFhjadQoc3Z1EFGP5mkUlFl BSwwVykleRyn33di6sARTamzXH1hq6qYYvRs02n+iFSmDKZFIe3iAuV2oW3Ir86+A5Ym S2cA== X-Gm-Message-State: AGi0PuZRTM6uYKfX/7hQoyQNN6ZNhycLzSxxkhZxrLVl3beT/OAt/KTz 7n25VY8X5XdTdjF3oc5/wRLuk7Wc X-Google-Smtp-Source: APiQypL7B/HK++hdRvcBHVpSKQSzoBFJ0kVANPKZHpsvefIA7pQvmE1YFWyAAhKLiXUd2cFXK0LR/Q== X-Received: by 2002:a05:6000:108b:: with SMTP id y11mr9245603wrw.380.1588439836894; Sat, 02 May 2020 10:17:16 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id u74sm5241524wmu.13.2020.05.02.10.17.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 02 May 2020 10:17:16 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sat, 2 May 2020 19:16:53 +0200 Message-Id: <20200502171700.28991-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200429222156.29129-1-andreas.rheinhardt@gmail.com> References: <20200429222156.29129-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 06/13] avformat/matroskaenc: Only write Cluster if it has in fact been opened 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: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Since commit 4aa0665f393847c35387a1c673e62346d0acfc95, the dynamic buffer destined for the contents of the current Cluster is no longer constantly allocated, reallocated and then freed after writing the content; instead it is reset and reused when closing a Cluster. Yet the code in mkv_write_trailer() still checked for whether a Cluster is open by checking whether the pointer to the dynamic buffer is NULL or not (instead of checking whether the position of the current Cluster is -1 or not). If a Cluster was not open, an empty Cluster would be output. One usually does not run into this issue, because unless there are errors, there are only three possibilities to not have an opened Cluster at the end of writing a packet: The first is if one sent an audio packet to the muxer. It might trigger closing and outputting the old Cluster, but because the muxer caches audio packets internally, it would not be output immediately and therefore no new Cluster would be opened. The second is an audio packet that does not contain data (such packets are sometimes sent for side-data only, e.g. by the FLAC encoder). The only difference to the first scenario is that such packets are not cached. The third is if one explicitly flushes the muxer by sending a NULL packet via av_write_frame(). If one also allows for errors, then there is also another possibility: Caching the audio packet may fail in the first scenario. If one calls av_write_trailer() after the first scenario, the cached audio packet will be output when writing the trailer, for which a Cluster is opened and everything is fine; because flushing the muxer does currently not output the cached audio packet (if one is cached), the issue also does not exist if an audio packet has been cached before flushing. The issue only exists in one of the other scenarios. Signed-off-by: Andreas Rheinhardt --- Given that this is a recent regression caused by me I will push this soon if no one objects. That the cached packet is not output on flushing looks like a bug to me. Will send a patch soon. libavformat/matroskaenc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 8593c3fce1..99b549ecc4 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -2464,7 +2464,7 @@ static int mkv_write_trailer(AVFormatContext *s) } } - if (mkv->cluster_bc) { + if (mkv->cluster_pos != -1) { ret = end_ebml_master_crc32(pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER, 0, 0, 0); if (ret < 0)