From patchwork Sat Apr 4 22:09:08 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Jan Chren (rindeal)" X-Patchwork-Id: 18650 Delivered-To: andriy.gelman@gmail.com Received: by 2002:a0c:c987:0:0:0:0:0 with SMTP id b7csp6220qvk; Sat, 4 Apr 2020 15:09:54 -0700 (PDT) X-Google-Smtp-Source: APiQypIgTJhS8deTq76fxD241txfWxXY3X/GG5NhlR0KxnIBuP7IzuU6BTkHj6JY91AxCn+MktB3 X-Received: by 2002:a17:906:7d15:: with SMTP id u21mr14409887ejo.364.1586038194594; Sat, 04 Apr 2020 15:09:54 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1586038194; cv=none; d=google.com; s=arc-20160816; b=vqx/aVS7j7bLaWP1OloDVx+IdqJcuRHB9iv7pGzX6Ol5QhAjYPXzhMjSo/RWdrXG6R MjJ23F8t973XbT0QV3ia+s2FNYS+V0pQdQQ2dMuiO0A1b3WaK6FW7p9cDduHhgBdm2kZ 5v+P+yebLGMfK3HYTNiMZiv7b6fM6gjIFVxo6OmD89pp3/KFjCYfTs7U+RpvuAhpJIX5 HARs5jG7fhvMTRymNmQ5UU5Gv+BrvSlAV/Da7R5OJvacwW7sSxrQLlgx/G3FhST3DvYz kbSZt5xvW+tA/OgdtvNsHp4PaaLM5YybODTk5HqGl8RRgwES2+KMl2sfOgsMUzUIY6Nr jMWw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:to :message-id:date:from:in-reply-to:references:mime-version :dkim-signature:delivered-to; bh=mx65ZIOTctZPx+Yhi/Q2bmqL+QjKofI+cZ70gSEqolk=; b=hirY48t/GV5vZg6g6fkRkb1jepnhwrQ3uvZ1Jt0VsVeYuH3sChIzkyZAZeLO/QfwFQ SsudS1dGd0xP/0fBTcNM4cm9Lgqioi9dM8lLZ0uqTcZ4UWF2e5/7djdICbIz0FhDrr0J q9CT+vTglcc1Wd42cL2tuBdDfhdZTJ9yd6S4BQJGEAPohH9/NL72JQkOu6WrdLln6uE4 i//tsSI/PPxtm2stc5gbiPzTAAFuLomP9JowlghA1NL2JSEOp4g1Btdh/4T8tC4+vbxp HW8V9t5OrG61XZkjdVb9zL8wD+KaioaUDm6Uey/yX/L1z7K1SJH1whJQPYTNbaPMGOJ6 3lcg== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=HmldYnJU; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id gf7si5164638ejb.322.2020.04.04.15.09.54; Sat, 04 Apr 2020 15:09:54 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=HmldYnJU; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id B752A68B514; Sun, 5 Apr 2020 01:09:52 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-oi1-f193.google.com (mail-oi1-f193.google.com [209.85.167.193]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 26B0D68B435 for ; Sun, 5 Apr 2020 01:09:46 +0300 (EEST) Received: by mail-oi1-f193.google.com with SMTP id q204so9611782oia.13 for ; Sat, 04 Apr 2020 15:09:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=5iT5avDEDwDwffJRYepUVmG4rn8YfalhwdFQdEyA1Fg=; b=HmldYnJUewyukuveZYJRIGzRcMoEI0sqprzHEYjAlnGpOfcCTxS5l5KRBSmY3CqLdU dfdphpOHyO8iJIa+/BdrQwA2ynkXYnq7b/ts73qSM1EYal92VjlPivpvJPcjRn14JUWE VWT9AA7KyR8Y1MkGY2coGMgllYJlfsCHTPeSOTotoeG+f0c8MwP+IEf7D8JnIhFpX9Q7 DU+fYOzdzpTIyFuYXJ5Tbw9TfchdhBPiNZ9xlGdv9Q81kD3Vlj1CkY3YkU84e/Kue7wb a4Tfp3g+RoHVzMIloPffsQ6T8yuT1BMcknmW1b6zKl2eS2Y8P3Nq8fbaMnE8sOgS+QXK RTbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=5iT5avDEDwDwffJRYepUVmG4rn8YfalhwdFQdEyA1Fg=; b=r7llCNA7LdjpqtG0ZEZeGvuUlbdfrDtw3yWyGOQE1wdGNJTaoVpNgQvYs+a8yJcLMU DbYl1rqCWtXtIwZ0feFxymOCDoVwtJEjoWE3pwRQid0OEMS3SilcxYmmmdy8zoGbbfVs F20vOy6deZIpmEMffdzqCF/DkGHOE7haxz9eYtaI0mSeG1a7cJCsYFCz/DufxKzxIHmE AOc+l9VaFx6a+JqCg8pj7uVRr4favoV4YsTo/c9VZyJWHUIOioh2Cf/jDShZnwrBMuqg QzCreOn1N+j3Lk5mzEvE5VPJR5uIRJ98H4imHvO6jbPO9I8mK2BH5iVIcSH/J4q60Ys7 WrjA== X-Gm-Message-State: AGi0PuYNVbrgHdloajow9JwFMv8WMBlJUjEdqu/qjBfwcBkmhwj6Wgr4 PyS9RSkPUZDp1F7t/MiBneYfkqcwPv1CIJdaPYHqBwS2FFY= X-Received: by 2002:aca:5508:: with SMTP id j8mr8177936oib.71.1586038184505; Sat, 04 Apr 2020 15:09:44 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: "Jan Chren (rindeal)" Date: Sat, 4 Apr 2020 22:09:08 +0000 Message-ID: To: ffmpeg-devel@ffmpeg.org Subject: [FFmpeg-devel] [PATCH v2] avformat/matroskaenc: remove MAX_TRACKS limit 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" X-TUID: 0sZn88Lbx1cR Content-Length: 7262 On Sat Apr 4 09:29:20 EEST 2020, Andreas Rheinhardt wrote: > Are you running into this limitation? Yes. > If so, do the files that you > create with this patch that have more than 127 tracks work? They > shouldn't. The reason for this (and the reason that I didn't remove this > limit altogether in 65ef74f74900590f134b4a130e8f56e5272d1925) lies in > the way the TrackNumber is encoded in the (Simple)Blocks: They are > encoded as variable-length numbers, so that encoding small TrackNumbers > takes up less bytes than encoding bigger TrackNumbers. More precisely, > TrackNumbers in the range 1..127 are encodable on one byte. And the way > they are written in mkv_write_block() and mkv_write_vtt_blocks() relies > on this. If you simply remove said limit, the tracks with TrackNumbers > > 127 will not have any (Simple)Blocks associated with them; instead the > encoded TrackNumber will be the desired TrackNumber modulo 128 and the > (Simple)Block will appear to belong to the track with the encoded > TrackNumber (if one exists).** The results will of course be catastrophic. I skimmed through libmatroska's code in src/KaxBlock.cpp and their limit is 0x4000-1, which is way more reasonable. > Notice that I have sent a patchset that slightly increases the number of > allowed tracks (without fixing the root cause though, because I didn't > know if there are really people who run into this): [1] makes > attachments no longer count towards the limit; [2] increases the limit > to 127. I will resend said patchset soon. > > - Andreas > > [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-November/253452.html > [2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-November/252563.html > > *: The reason for setting MAX_TRACKS to 126 instead of 127 is probably > that the encoding corresponding to the number 127 is special when > encoding lengths (which use the same variable-length number scheme): > Here it denotes an unknown length instead of a length of 127. But there > are no such values with a special meaning for encoded TrackNumbers. Indeed, libmatroska allows 0x80-1 (127) per byte. Your second patch should have been merged. > **: Notice that a TrackNumber of 0 is invalid, so any (Simple)Block that > ought to belong to the tracks with TrackNumber 128, 256, 384, ... will > have no matching track at all. FFmpeg's Matroska demuxer treats > encountering such (Simple)Blocks as a sign of invalid data and it will > try to resync to the next level 1 element (typically the next Cluster). > Similar things can happen if you use attachments (in this case there may > be gaps in the used TrackNumbers). I have included a new patch, which writes the track_number using put_ebml_num(), which should work perhaps? From 477d1ee08115e09f2f61f75dd70ce7bc882d2c34 Mon Sep 17 00:00:00 2001 From: Jan Chren (rindeal) Date: Sat, 4 Apr 2020 00:00:00 +0000 Subject: [PATCH] avformat/matroskaenc: remove track limit Strictly speaking, ebml_num_size() wastes one bit here. Employing new ebml_num_size() and put_ebml_num() just for track_number would solve this. --- libavformat/matroskaenc.c | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 5dae53026d..30132a53e8 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -117,10 +117,6 @@ typedef struct mkv_attachments { #define MODE_MATROSKAv2 0x01 #define MODE_WEBM 0x02 -/** Maximum number of tracks allowed in a Matroska file (with track numbers in - * range 1 to 126 (inclusive) */ -#define MAX_TRACKS 126 - typedef struct MatroskaMuxContext { const AVClass *class; int mode; @@ -2089,9 +2085,8 @@ static int mkv_write_block(AVFormatContext *s, AVIOContext *pb, } put_ebml_id(pb, blockid); - put_ebml_num(pb, size + 4, 0); - // this assumes stream_index is less than 126 - avio_w8(pb, 0x80 | track_number); + put_ebml_num(pb, size + ebml_num_size(track_number) + 2 + 1, 0); + put_ebml_num(track_number, 0); avio_wb16(pb, ts - mkv->cluster_pts); avio_w8(pb, (blockid == MATROSKA_ID_SIMPLEBLOCK && keyframe) ? (1 << 7) : 0); avio_write(pb, data + offset, size); @@ -2155,8 +2150,8 @@ static int mkv_write_vtt_blocks(AVFormatContext *s, AVIOContext *pb, AVPacket *p blockgroup = start_ebml_master(pb, MATROSKA_ID_BLOCKGROUP, mkv_blockgroup_size(size)); put_ebml_id(pb, MATROSKA_ID_BLOCK); - put_ebml_num(pb, size + 4, 0); - avio_w8(pb, 0x80 | (pkt->stream_index + 1)); // this assumes stream_index is less than 126 + put_ebml_num(pb, size + ebml_num_size(pkt->stream_index + 1) + 2 + 1, 0); + put_ebml_num(pkt->stream_index + 1, 0); avio_wb16(pb, ts - mkv->cluster_pts); avio_w8(pb, flags); avio_printf(pb, "%.*s\n%.*s\n%.*s", id_size, id, settings_size, settings, pkt->size, pkt->data); @@ -2604,13 +2599,6 @@ static int mkv_init(struct AVFormatContext *s) { int i; - if (s->nb_streams > MAX_TRACKS) { - av_log(s, AV_LOG_ERROR, - "At most %d streams are supported for muxing in Matroska\n", - MAX_TRACKS); - return AVERROR(EINVAL); - } - for (i = 0; i < s->nb_streams; i++) { if (s->streams[i]->codecpar->codec_id == AV_CODEC_ID_ATRAC3 || s->streams[i]->codecpar->codec_id == AV_CODEC_ID_COOK || -- 2.26.0