From patchwork Wed Nov 27 05:38:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 16426 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 4BB6A4494F4 for ; Wed, 27 Nov 2019 07:38:59 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 26F9968B039; Wed, 27 Nov 2019 07:38:59 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id ECD5C689B7F for ; Wed, 27 Nov 2019 07:38:52 +0200 (EET) Received: by mail-wm1-f65.google.com with SMTP id y5so5912206wmi.5 for ; Tue, 26 Nov 2019 21:38:52 -0800 (PST) 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=f6d/0p2X1hzOWUfNMZgOVcDYAVfrQ74xcbKdVuQ0Jgg=; b=UOmf9JvjuA9Ql1BLzSAk4MX9lDxIV9XdIBlssYLpHko8+6DX5YrSHODHDab09X00iY zId+L0gkjLDo5AkOrHuxmOoKsXX3R5VB+cKtOYebTmUb1onbyql2TLnMPfAb07Rmaeyc 5XuysfQpI3mjW2m7lQD62mSrzfkLhXrDfxzyWmbKU9wjDo5GYCUBQRObMRFqKiGTtXym HDaRmKB9up/aC5pMSRap6yOfuabK1c484oA6pvZ9fgwC/ojffYN516J3IjBnyTOya7eD a6QBs3FLSw2Gr87F8ny7T1CnDdsH1L1U9dzCJVlTcv8/FGYUFZhUUPN0ccASZdv1aOU7 i+4A== 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=f6d/0p2X1hzOWUfNMZgOVcDYAVfrQ74xcbKdVuQ0Jgg=; b=UmQuDtghfQtslsAPGTN6zSMxIASg69JtA28DqpLbsd0DzWSRX34+7e0oEYuArmMcDJ y/x0PbR2610GqWZGrul9zWEke2wXjxWijtSHbNbn8bEE6CiXL7GLvX5djES//POqvEnX Tn5wwEOqLpODDrrbUapU8AL2WODD3Y4a5iwlw/ZHUY2UYITM4i3U4qzIE8dy9adZPKXD YK1Aug9LXOkBCoUFYX2qpuvrr9COLi0D1qvcWIXCvJdbzdWbfdxT2t1wRsw0Sbw8oPTy MEITP5RQZsSQqJ3FtcDHjOjdpLg7lASn5/c2Y9KAABoZxf+KRe8ltDBO9NUelLzi4xc3 PYmQ== X-Gm-Message-State: APjAAAVZrpVz4wE5KIsxHqyZrm57ppPdCPyxTCCdOouSMrp5hDugqrbB 4v+v43DsI6ephWmWHKbzQblKGb8S X-Google-Smtp-Source: APXvYqxm5BAkfi7bvdu3RgA2KhGQ4Tn24JK8cLFjXT8c6FUeEUaUc/MfIFfgWfBlxdcJL5DT0rktgA== X-Received: by 2002:a7b:c858:: with SMTP id c24mr2453557wml.174.1574833132281; Tue, 26 Nov 2019 21:38:52 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc08e23.dynamic.kabel-deutschland.de. [188.192.142.35]) by smtp.gmail.com with ESMTPSA id n9sm3372511wrt.93.2019.11.26.21.38.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Nov 2019 21:38:51 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 27 Nov 2019 06:38:38 +0100 Message-Id: <20191127053842.19481-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191106024922.19228-3-andreas.rheinhardt@gmail.com> References: <20191106024922.19228-3-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 1/5] avformat/matroskaenc: Ensure that ChapterUID are != 0 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" AVChapters have an int as id field and therefore this value can appear <= 0. When remuxing from Matroska, this value is actually the lower 32 bits of the original ChapterUID (which can be 64 bits). In order to ensure that the ChapterUID be always > 0, they were offset as follows (since 07704c61): First max(0, 1LL - chapter[i].id) was computed and stored in an uint32_t. And then the IDs were offset using this value. This has two downsides: 1. It does not ensure that the UID is actually != 0: Namely if there is a chapter with id == INT_MIN, then the offset will be 2^31 + 1 and a chapter with id == INT_MAX will become 2^31 - 1 + 2^31 + 1 = 2^32 = 0, because the actual calculation was performed in 32 bits. 2. As soon as a chapter id appears to be negative, a nontrivial offset is used, so that not even a ChapterUID that only uses 32 bits is preserved. So change this by treating the id as an unsigned value internally and only offset (by 1) if an id vanishes. The actual offsetting then has to be performed in 64 bits in order to make sure that no UINT32_MAX wraps around. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskaenc.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 5c04ec98f6..1d3203c969 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1500,7 +1500,8 @@ static int mkv_write_chapters(AVFormatContext *s) } chapteratom = start_ebml_master(dyn_cp, MATROSKA_ID_CHAPTERATOM, 0); - put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERUID, c->id + mkv->chapter_id_offset); + put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERUID, + (uint32_t)c->id + (uint64_t)mkv->chapter_id_offset); put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERTIMESTART, chapterstart); put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERTIMEEND, chapterend); if (mkv->mode != MODE_WEBM) { @@ -1557,7 +1558,7 @@ static int mkv_write_simpletag(AVIOContext *pb, AVDictionaryEntry *t) } static int mkv_write_tag_targets(AVFormatContext *s, uint32_t elementid, - unsigned int uid, ebml_master *tag) + uint64_t uid, ebml_master *tag) { AVIOContext *pb; MatroskaMuxContext *mkv = s->priv_data; @@ -1595,7 +1596,7 @@ static int mkv_check_tag_name(const char *name, uint32_t elementid) } static int mkv_write_tag(AVFormatContext *s, AVDictionary *m, uint32_t elementid, - unsigned int uid) + uint64_t uid) { MatroskaMuxContext *mkv = s->priv_data; ebml_master tag; @@ -1686,7 +1687,8 @@ static int mkv_write_tags(AVFormatContext *s) if (!mkv_check_tag(ch->metadata, MATROSKA_ID_TAGTARGETS_CHAPTERUID)) continue; - ret = mkv_write_tag(s, ch->metadata, MATROSKA_ID_TAGTARGETS_CHAPTERUID, ch->id + mkv->chapter_id_offset); + ret = mkv_write_tag(s, ch->metadata, MATROSKA_ID_TAGTARGETS_CHAPTERUID, + (uint32_t)ch->id + (uint64_t)mkv->chapter_id_offset); if (ret < 0) return ret; } @@ -1961,7 +1963,10 @@ static int mkv_write_header(AVFormatContext *s) return ret; for (i = 0; i < s->nb_chapters; i++) - mkv->chapter_id_offset = FFMAX(mkv->chapter_id_offset, 1LL - s->chapters[i]->id); + if (!s->chapters[i]->id) { + mkv->chapter_id_offset = 1; + break; + } ret = mkv_write_chapters(s); if (ret < 0)