From patchwork Wed Apr 1 10:46:33 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 18567 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 4338344A30B for ; Wed, 1 Apr 2020 13:47:35 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 27D0D68B053; Wed, 1 Apr 2020 13:47:35 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 82AFA68AFA1 for ; Wed, 1 Apr 2020 13:47:28 +0300 (EEST) Received: by mail-wr1-f68.google.com with SMTP id h15so29922519wrx.9 for ; Wed, 01 Apr 2020 03:47:28 -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=HaljruFbhKFLfr+cJkHN/uPDizBLQ1Ky2QCuYnhyJPE=; b=nmK3rMo0ED/mlwXEJuheHhsvOmuc/eLAXg7u6iar5sZA1gsIvBwFwZmeU1w2txwjTg zN97oIdsWV8dr5ZXpzG9U55ZIg9S87Yqanw45+1pkOT/qLZuBxrasR9BO5NWX+lSbuub IX5VIgKZUbwSgiNjyLzMn9YbijJjqUupD00atUDxVcXpCT09LjXZYq6K76Efgyx0YhlC QH+EqYvFSdJfHJzzTqHoP8wqctdfxq44MMAS2DWcms6oWX6oqVyjioUyvnU2Gf407rf6 e/qqYHhVvXM3PuhPdDbxj0SnSltfbN2Wuy9wVQZXXt0FJ5ceSkyGiHL5+W7s/G2WkIcm G4QA== 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=HaljruFbhKFLfr+cJkHN/uPDizBLQ1Ky2QCuYnhyJPE=; b=j8mOGGcptOyhPV1D+rfJ5sZyJpEWq1dbyB7yYFKdR0BbJgZVcMZeXX1N5fnDsnZj2/ JDaln4EbEfLtd3ToojpiKPbb6L4rAaobRbLGXLtRsy8eKc8glkR44tUA/um1Vk5pVyQm S//rmf7Os4MvrAZ2GAEVMFGTkIZg7u3owKU5oXOFEq4Xp2XM+mK5Q3S9P6OEATOjJ5XH fGx6VWPr5r0tNd6BynriHE33dAuqsHXt9pzXLuh4XIdUbx4F/u+NXuVp++W/6wga4ZKW G8TgLYjEVoVAD91IwRZeUEhCK6+niRlkyk9b/mSq945wU/ha/VtAPptDn8pB5Yc21usQ 7TXA== X-Gm-Message-State: ANhLgQ2xGZlWdS22TObd2K6gP5HZhySCmqetJ56fQ5pPvSpRYOREZGVX Kc1Wb2LKxjSBs4XMckAv3xOsTuac X-Google-Smtp-Source: ADFU+vtMgRPPae9RRyhrIC97U+j28XNAOh2th5Oxpgys9ReqzsrOAglewkOWLPKEmzvxvUEElLGuKg== X-Received: by 2002:a5d:6a43:: with SMTP id t3mr24241531wrw.87.1585738047614; Wed, 01 Apr 2020 03:47:27 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id w3sm2427800wrn.31.2020.04.01.03.47.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Apr 2020 03:47:26 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 1 Apr 2020 12:46:33 +0200 Message-Id: <20200401104634.25461-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200401104634.25461-1-andreas.rheinhardt@gmail.com> References: <20200401104634.25461-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 2/3] avformat/matroskaenc: Fix edge case of writing Cues at the beginning 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" The Matroska muxer has the ability to write the Cues (the index) at the beginning of the file (in front of the Cluster): The user inputs the amount of space that should be reserved at the beginning of the file and if this is sufficient, the Cues will be written there and the part of the reserved space not used up by the Cues will be filled with a "Void" element. There is just one problem with this: One can not fill a single byte this way, because said Void element is minimally two bytes long (one byte ID, one byte length field). Up until now, if one reserved one byte more than needed, one would run into an assert when writing the Void element. There are two solutions for this: Error out if it happens. Or adjust the length field of the Cues in order to ensure that the above situation can't happen (i.e. write the length on one byte more than necessary). The first solution is very unsatisfactory, as enough space has been reserved. Therefore this commit implements the second solution. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskaenc.c | 42 +++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 22cc693720..2ca4527dfe 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -339,14 +339,15 @@ static int start_ebml_master_crc32(AVIOContext **dyn_cp, MatroskaMuxContext *mkv } static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, - MatroskaMuxContext *mkv, uint32_t id) + MatroskaMuxContext *mkv, uint32_t id, + int length_size) { uint8_t *buf, crc[4]; int size, skip = 0; put_ebml_id(pb, id); size = avio_close_dyn_buf(*dyn_cp, &buf); - put_ebml_num(pb, size, 0); + put_ebml_num(pb, size, length_size); if (mkv->write_crc) { skip = 6; /* Skip reserved 6-byte long void element from the dynamic buffer. */ AV_WL32(crc, av_crc(av_crc_get_table(AV_CRC_32_IEEE_LE), UINT32_MAX, buf + skip, size - skip) ^ UINT32_MAX); @@ -465,7 +466,7 @@ static int mkv_write_seekhead(AVIOContext *pb, MatroskaMuxContext *mkv, put_ebml_uint(dyn_cp, MATROSKA_ID_SEEKPOSITION, entry->segmentpos); end_ebml_master(dyn_cp, seekentry); } - end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_SEEKHEAD); + end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_SEEKHEAD, 0); remaining = seekhead->filepos + seekhead->reserved_size - avio_tell(pb); put_ebml_void(pb, remaining); @@ -1382,7 +1383,7 @@ static int mkv_write_tracks(AVFormatContext *s) end_ebml_master_crc32_preliminary(pb, mkv->tracks_bc, MATROSKA_ID_TRACKS, &mkv->tracks_pos); else - end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, MATROSKA_ID_TRACKS); + end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, MATROSKA_ID_TRACKS, 0); return 0; } @@ -1438,7 +1439,7 @@ static int mkv_write_chapters(AVFormatContext *s) end_ebml_master(dyn_cp, chapteratom); } end_ebml_master(dyn_cp, editionentry); - end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CHAPTERS); + end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CHAPTERS, 0); mkv->wrote_chapters = 1; return 0; @@ -1637,7 +1638,7 @@ static int mkv_write_tags(AVFormatContext *s) end_ebml_master_crc32_preliminary(s->pb, mkv->tags_bc, MATROSKA_ID_TAGS, &mkv->tags_pos); else - end_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, MATROSKA_ID_TAGS); + end_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, MATROSKA_ID_TAGS, 0); } return 0; } @@ -1733,7 +1734,7 @@ static int mkv_write_attachments(AVFormatContext *s) mkv->attachments->entries[mkv->attachments->num_entries].stream_idx = i; mkv->attachments->entries[mkv->attachments->num_entries++].fileuid = fileuid; } - end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_ATTACHMENTS); + end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_ATTACHMENTS, 0); return 0; } @@ -1874,7 +1875,7 @@ static int mkv_write_header(AVFormatContext *s) end_ebml_master_crc32_preliminary(s->pb, mkv->info_bc, MATROSKA_ID_INFO, &mkv->info_pos); else - end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO); + end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO, 0); pb = s->pb; ret = mkv_write_tracks(s); @@ -2162,7 +2163,7 @@ static void mkv_end_cluster(AVFormatContext *s) { MatroskaMuxContext *mkv = s->priv_data; - end_ebml_master_crc32(s->pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER); + end_ebml_master_crc32(s->pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER, 0); mkv->cluster_pos = -1; avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_FLUSH_POINT); } @@ -2469,7 +2470,7 @@ static int mkv_write_trailer(AVFormatContext *s) } if (mkv->cluster_bc) { - end_ebml_master_crc32(pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER); + end_ebml_master_crc32(pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER, 0); } ret = mkv_write_chapters(s); @@ -2483,6 +2484,7 @@ static int mkv_write_trailer(AVFormatContext *s) if (mkv->cues.num_entries) { AVIOContext *cues; uint64_t size; + int length_size = 0; ret = start_ebml_master_crc32(&cues, mkv); if (ret < 0) @@ -2497,7 +2499,8 @@ static int mkv_write_trailer(AVFormatContext *s) if (mkv->reserve_cues_space) { size = avio_tell(cues); - size += 4 + ebml_num_size(size); + length_size = ebml_num_size(size); + size += 4 + length_size; if (mkv->reserve_cues_space < size) { av_log(s, AV_LOG_WARNING, "Insufficient space reserved for Cues: " @@ -2512,10 +2515,19 @@ static int mkv_write_trailer(AVFormatContext *s) ffio_free_dyn_buf(&cues); return ret64; } + if (mkv->reserve_cues_space == size + 1) { + /* There is no way to reserve a single byte because + * the minimal size of an EBML Void element is 2 + * (1 byte ID, 1 byte length field). This problem + * is solved by writing the Cues' length field on + * one byte more than necessary. */ + length_size++; + size++; + } } } mkv_add_seekhead_entry(mkv, MATROSKA_ID_CUES, avio_tell(pb)); - end_ebml_master_crc32(pb, &cues, mkv, MATROSKA_ID_CUES); + end_ebml_master_crc32(pb, &cues, mkv, MATROSKA_ID_CUES, length_size); if (mkv->reserve_cues_space) { if (size < mkv->reserve_cues_space) put_ebml_void(pb, mkv->reserve_cues_space - size); @@ -2534,11 +2546,11 @@ static int mkv_write_trailer(AVFormatContext *s) av_log(s, AV_LOG_DEBUG, "end duration = %" PRIu64 "\n", mkv->duration); avio_seek(mkv->info_bc, mkv->duration_offset, SEEK_SET); put_ebml_float(mkv->info_bc, MATROSKA_ID_DURATION, mkv->duration); - end_ebml_master_crc32(pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO); + end_ebml_master_crc32(pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO, 0); // write tracks master avio_seek(pb, mkv->tracks_pos, SEEK_SET); - end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, MATROSKA_ID_TRACKS); + end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, MATROSKA_ID_TRACKS, 0); // update stream durations if (!mkv->is_live) { @@ -2568,7 +2580,7 @@ static int mkv_write_trailer(AVFormatContext *s) } if (mkv->tags_bc && !mkv->is_live) { avio_seek(pb, mkv->tags_pos, SEEK_SET); - end_ebml_master_crc32(pb, &mkv->tags_bc, mkv, MATROSKA_ID_TAGS); + end_ebml_master_crc32(pb, &mkv->tags_bc, mkv, MATROSKA_ID_TAGS, 0); } avio_seek(pb, currentpos, SEEK_SET);