From patchwork Fri Mar 13 23:15:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 18180 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 CC0AF449F07 for ; Sat, 14 Mar 2020 01:16:02 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 9D21B68B302; Sat, 14 Mar 2020 01:16:02 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1443968B2C6 for ; Sat, 14 Mar 2020 01:15:56 +0200 (EET) Received: by mail-wr1-f67.google.com with SMTP id d5so13811699wrc.2 for ; Fri, 13 Mar 2020 16:15:56 -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=VQJZQ6iFuUQA5aw1MJCmORz/Kj+1lCC9+P/rAKIXk1c=; b=U9LeqjHpnuGsmXyXFsvJPZJtMRI24LyAl2KCzrCoDeDTg0qmN68Uo5Zrf5xRXALYw7 gzmLg4eeJ8qo/bNplvxKRTzZYRyQ3Ets56yewDIj1KnaLBF9LAvdbLuHy1ftqKgyrse3 HgAAHu/LO9K9NWf07AhKz9Azz3Z54QgVZYkzri4jkLjyw6UCDvfdg/FBKacKWkkwg57j usyzYp2oY3JnWOMUuHlk2tmXdPlueND/9nget+eErIXwPPF67bmRmBL2rhXbQhqFSefm /FFhABHymQeS0itkHB9Z24xw/EWJShsXSwkrbon4J1I43jro4O5ajXm5zlmOuVisENHH Drtw== 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=VQJZQ6iFuUQA5aw1MJCmORz/Kj+1lCC9+P/rAKIXk1c=; b=E/gOnjeLYdp6QgRTHdoFUczMalznLzqkn3M7rycqXgdQ7gnZD0T7cY7bhpwv7LbWgZ eX67tbdoo8/bhbqV8U2mxr0PIUkTO6oXqONajzcZqHI9c3ME7DPhaX0nMWS//WSpHSuF LdGQ9oSm/3tUexhPrSaXAGYZt/KFXpD/10MJ8Zqcz+GTOD44SX1c10TpexE+XeXyCsvx RtzRkHO2uSTG0DWvVVY1jTA+iAl9t1aFNn5XzRvDxWV7rGuT+ctsAR4d/Bq/CVrlTt1m Rrwd+8wXoClmKe4+7IYvy/JTvwnmlnUTy3Y4SZHWf3Ja/hiXnn1t6Z75/uKyHnIgBbM5 Ri5w== X-Gm-Message-State: ANhLgQ1BWdXdR1P9JDY0Q0iKgHn9xOG4alL4oKXPqkkIxfD8WF8CFO5k eX181lEizt29HJDE/v38l3Vv8Elz X-Google-Smtp-Source: ADFU+vs91Pd8VfaRNdNF71dgDOG+1t4louM2E9ud8ZXvHReGBx1BUl4AhhJk1nlUCF7rz4xORsNl5Q== X-Received: by 2002:adf:eec7:: with SMTP id a7mr4140693wrp.405.1584141355059; Fri, 13 Mar 2020 16:15:55 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab4b.dynamic.kabel-deutschland.de. [188.193.171.75]) by smtp.gmail.com with ESMTPSA id k18sm31666134wru.94.2020.03.13.16.15.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Mar 2020 16:15:54 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sat, 14 Mar 2020 00:15:36 +0100 Message-Id: <20200313231536.16949-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200101005837.11356-15-andreas.rheinhardt@gmail.com> References: <20200101005837.11356-15-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2] avformat/matroskaenc: Simplify writing Cues 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" When the Matroska muxer writes the Cues (the index), it groups index entries with the same timestamp into the same CuePoint to save space. But given Matroska's variable-length length fields, it either needs to have an upper bound of the final size of the CuePoint before writing it or the CuePoint has to be assembled in a different buffer, so that after having assembled the CuePoint (when the real size is known), the CuePoint's header can be written and its data copied after it. The first of these approaches is the currently used one. This entails finding out the number of entries in a CuePoint before starting the CuePoint and therefore means that the list is read at least twice. Furthermore, a worst-case upper-bound for the length of a single entry was used, so that sometimes bytes are wasted on length fields. This commit switches to the second approach. This is no longer more expensive than the current approach if one only resets the dynamic buffer used to write the CuePoint's content instead of opening a new buffer for every CuePoint: Writing the trailer of a file with 540.000 CuePoints improved actually from 219054414 decicycles to 2164379394 decicycles (based upon 50 iterations). Signed-off-by: Andreas Rheinhardt --- The earlier commit message falsely stated to be about matroskadec. This fixes this. Nothing else changed. libavformat/matroskaenc.c | 74 +++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 41 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index bbf3a3de7f..a5fe509fdc 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -170,12 +170,8 @@ typedef struct MatroskaMuxContext { * offset, 4 bytes for target EBML ID */ #define MAX_SEEKENTRY_SIZE 21 -/** per-cuepoint-track - 5 1-byte EBML IDs, 5 1-byte EBML sizes, 3 8-byte uint max - * and one 1-byte uint for the track number (this assumes MAX_TRACKS to be <= 255) */ -#define MAX_CUETRACKPOS_SIZE 35 - -/** per-cuepoint - 1 1-byte EBML ID, 1 1-byte EBML size, 8-byte uint max */ -#define MAX_CUEPOINT_CONTENT_SIZE(num_tracks) 10 + MAX_CUETRACKPOS_SIZE * num_tracks +/** 4 * (1-byte EBML ID, 1-byte EBML size, 8-byte uint max) */ +#define MAX_CUETRACKPOS_SIZE 40 /** Seek preroll value for opus */ #define OPUS_SEEK_PREROLL 80000000 @@ -509,58 +505,54 @@ static int mkv_add_cuepoint(MatroskaMuxContext *mkv, int stream, int tracknum, i static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tracks, int num_tracks) { MatroskaMuxContext *mkv = s->priv_data; - AVIOContext *dyn_cp, *pb = s->pb; + AVIOContext *dyn_cp, *pb = s->pb, *cuepoint; int64_t currentpos; - int i, j, ret; + int ret; currentpos = avio_tell(pb); ret = start_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CUES); if (ret < 0) return ret; - for (i = 0; i < cues->num_entries; i++) { - ebml_master cuepoint, track_positions; - mkv_cuepoint *entry = &cues->entries[i]; - uint64_t pts = entry->pts; - int ctp_nb = 0; - - // Calculate the number of entries, so we know the element size - for (j = 0; j < num_tracks; j++) - tracks[j].has_cue = 0; - for (j = 0; j < cues->num_entries - i && entry[j].pts == pts; j++) { - int idx = entry[j].stream_idx; + ret = avio_open_dyn_buf(&cuepoint); + if (ret < 0) { + ffio_free_dyn_buf(&dyn_cp); + return ret; + } - av_assert0(idx >= 0 && idx < num_tracks); - if (tracks[idx].has_cue && s->streams[idx]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE) - continue; - tracks[idx].has_cue = 1; - ctp_nb ++; - } + for (mkv_cuepoint *entry = cues->entries, *end = entry + cues->num_entries; + entry < end;) { + uint64_t pts = entry->pts; + uint8_t *buf; + int size; - cuepoint = start_ebml_master(dyn_cp, MATROSKA_ID_POINTENTRY, MAX_CUEPOINT_CONTENT_SIZE(ctp_nb)); - put_ebml_uint(dyn_cp, MATROSKA_ID_CUETIME, pts); + put_ebml_uint(cuepoint, MATROSKA_ID_CUETIME, pts); // put all the entries from different tracks that have the exact same // timestamp into the same CuePoint - for (j = 0; j < num_tracks; j++) + for (int j = 0; j < num_tracks; j++) tracks[j].has_cue = 0; - for (j = 0; j < cues->num_entries - i && entry[j].pts == pts; j++) { - int idx = entry[j].stream_idx; + do { + ebml_master track_positions; + int idx = entry->stream_idx; + av_assert0(idx >= 0 && idx < num_tracks); if (tracks[idx].has_cue && s->streams[idx]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE) continue; tracks[idx].has_cue = 1; - track_positions = start_ebml_master(dyn_cp, MATROSKA_ID_CUETRACKPOSITION, MAX_CUETRACKPOS_SIZE); - put_ebml_uint(dyn_cp, MATROSKA_ID_CUETRACK , entry[j].tracknum ); - put_ebml_uint(dyn_cp, MATROSKA_ID_CUECLUSTERPOSITION , entry[j].cluster_pos); - put_ebml_uint(dyn_cp, MATROSKA_ID_CUERELATIVEPOSITION, entry[j].relative_pos); - if (entry[j].duration != -1) - put_ebml_uint(dyn_cp, MATROSKA_ID_CUEDURATION , entry[j].duration); - end_ebml_master(dyn_cp, track_positions); - } - i += j - 1; - end_ebml_master(dyn_cp, cuepoint); - } + track_positions = start_ebml_master(cuepoint, MATROSKA_ID_CUETRACKPOSITION, MAX_CUETRACKPOS_SIZE); + put_ebml_uint(cuepoint, MATROSKA_ID_CUETRACK , entry->tracknum ); + put_ebml_uint(cuepoint, MATROSKA_ID_CUECLUSTERPOSITION , entry->cluster_pos); + put_ebml_uint(cuepoint, MATROSKA_ID_CUERELATIVEPOSITION, entry->relative_pos); + if (entry->duration != -1) + put_ebml_uint(cuepoint, MATROSKA_ID_CUEDURATION , entry->duration); + end_ebml_master(cuepoint, track_positions); + } while (++entry < end && entry->pts == pts); + size = avio_get_dyn_buf(cuepoint, &buf); + put_ebml_binary(dyn_cp, MATROSKA_ID_POINTENTRY, buf, size); + ffio_reset_dyn_buf(cuepoint); + } + ffio_free_dyn_buf(&cuepoint); end_ebml_master_crc32(pb, &dyn_cp, mkv); return currentpos;