From patchwork Sun Apr 5 15:59:09 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 18664 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 49AE944BE3F for ; Sun, 5 Apr 2020 19:00:06 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 31F3F680C83; Sun, 5 Apr 2020 19:00:06 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4655F680C46 for ; Sun, 5 Apr 2020 18:59:59 +0300 (EEST) Received: by mail-wr1-f66.google.com with SMTP id a25so14463359wrd.0 for ; Sun, 05 Apr 2020 08:59:59 -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=JcRfnoeY2PHmw1hlSVhQGnKK0gFV8LuAtbKDAsKRj60=; b=c5tznrnoX/7YlfCQcSyGo/FEOTu3W+40ztLZRHooObOVhWnr7YlCxSBH7aEhF+DTQv LZrCwUuu3u+jIfYQYMEBSJQFFm3H+gD5PR9aG/RkVXXh9d6CC0Jz7V8VFzzxJr8tRfN5 GB+4rp8wpxXp4+7hiYsAhUOUfoSCBDHi4hn3dqgP4ioi9HnGzywRsF284V3NdJMefvVy 9AR5eEVifTQvBAaJXTdNGHphf4Xij1kgPurPvz7a7wvT3PrHQwtmrJJgoALari6YQqDm wozbPXMCDG7SVqkDiszzDlLlOLplUn4w2yXUJAGfzVDdMvlZvuwMaRGQdgWEkoK1Hg+H 02yA== 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=JcRfnoeY2PHmw1hlSVhQGnKK0gFV8LuAtbKDAsKRj60=; b=kkIj6G9ye9/0tWLzmh6xI3ZOX719IZMcJ++KfqvWor0VHatlXXedYjip4dEq9X3zBg INwauf6or/iZBJTu2zllZ2QC8dsjbGRKlsfrfrpkvl8KmF0qTf3yJKhUK5UXYqD8RE1t muki+cwefn99OEqgJ4LWpv98qQ26IWhfo52uEWGKY8+76RyIfqPO9hF+v+ob/nCgLhLW ZpfacjoBHAA7V3HWPw6M59QwAhrgoKa1jo+k/Tc3zTQJBZ4nrjA7MpIK0uV0ykt97uix sP6jyqR3sMx9fQxwJ9Uii2iBNSiVpiUngiKpDMIfQA/SAiJlHznB1PsRqTfN33JeIJzV TJPA== X-Gm-Message-State: AGi0PuZwfvUBE5F87h1wse3p41fUVkfWFvIPA0mRMjZTSIiopXjOO6Gg cQRr5+gciPp/WU9mTz97g91KpIaI X-Google-Smtp-Source: APiQypK2DVsNGhUddA7qq4PMPB++sKZ4C8vcPWvpcOmUsOMAA35SKMNLNG5B4QIbQ9ArIx/z5GXH3g== X-Received: by 2002:a5d:6885:: with SMTP id h5mr12321573wru.166.1586102398230; Sun, 05 Apr 2020 08:59:58 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id v21sm20014567wmh.26.2020.04.05.08.59.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Apr 2020 08:59:57 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sun, 5 Apr 2020 17:59:09 +0200 Message-Id: <20200405155928.9323-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200405155928.9323-1-andreas.rheinhardt@gmail.com> References: <20200405155928.9323-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 01/20] 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 actually contains the lower 32 bits of the original ChapterUID (which can be 64 bits). In order to ensure that the ChapterUID is 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 060e8b7816..a377d092df 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1422,7 +1422,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) { @@ -1479,7 +1480,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; @@ -1518,7 +1519,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; @@ -1612,7 +1613,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; } @@ -1882,7 +1884,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)