From patchwork Wed Nov 27 05:38:41 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 16429 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 91E724495CE for ; Wed, 27 Nov 2019 07:39:19 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7746968B05E; Wed, 27 Nov 2019 07:39:19 +0200 (EET) 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 B5E5568B03B for ; Wed, 27 Nov 2019 07:39:14 +0200 (EET) Received: by mail-wr1-f68.google.com with SMTP id w9so25198452wrr.0 for ; Tue, 26 Nov 2019 21:39:14 -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=mzQU7EVszrsVPYJtHxJsWJQdi+U/GGif2hLzxpgtDsg=; b=AdMRwXVw1Q5wXarsnC9i6sGcJi4Ga4IG/UpaW6aehCUxZsmKiX+uI2KNJl37zKnJSL Y6l8O/1ibRmhIAFGdAO7usSeUPvkWkpjmR5Szus6vK8YAwYIQh7SeD10z8gvXirvjaDO IkSyxNRtwVZu+b1+zgTJQ+PGONRd+KPjFxExEq6Tl87IcH/Ffr3OWSX3w6Kci+oImD0Z eCHUrvUVp5L2H4nDp0s5C5eRG/H7h3c1GjYYlTiLkPK2Hg/e/XlYN35nIOB2qQ00mPyf JDeyftw5wKAEs/fP7+vb3k195RoRq1hzGNb+MmKYqRCu0+nH6pOxDof3PV3exPceP5nG k+Yg== 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=mzQU7EVszrsVPYJtHxJsWJQdi+U/GGif2hLzxpgtDsg=; b=Nn7dlkOB2M9jVSVEwEKHnsNT5e9tqiEMiuzXGTxcK+gyvMHdmD4HrmWF/PdIt/3tpi Q+Y/K7pfsw4b04mXHlqwlvnUsGN2JbBxMBRVV7lz49w0byP5iViRo3tzDsxisT/iNmnq FZRd0uTiD/okbct8R9IErTi7iShl6LOR4KMy47SOV50Zs+n+AOyv3m1ZYTeg9jtjpDm8 NjTCmhvAhzfY8ayejheXbyVZH9ZsjLdxpVWa5aUCTiseLQ4Hj5J0n2MMjZRUGw66N6hE Vqo5objBvT++ZCspZSIRhboodFeniZf0wg+cfyaU7igJvLygpiCgOkcyL4pHZ1RA9vrs Xc+w== X-Gm-Message-State: APjAAAWAyzO+KjbjSEcTCRXgQ+wIjDmVbzakyQqPPHdLJJ5QD6aW6Yin Tf/jXZyqkcCgvyJ2IAavEA5WHigd X-Google-Smtp-Source: APXvYqzUiVwjftcsClp8CvLCpd4snvCSyiU/w3tj6BYvBKsEhIAQwlYX5lgju05Pi4BZFhi9c3wKYw== X-Received: by 2002:a5d:5586:: with SMTP id i6mr22484117wrv.116.1574833154115; Tue, 26 Nov 2019 21:39:14 -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.39.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Nov 2019 21:39:13 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 27 Nov 2019 06:38:41 +0100 Message-Id: <20191127053842.19481-4-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191127053842.19481-1-andreas.rheinhardt@gmail.com> References: <20191106024922.19228-3-andreas.rheinhardt@gmail.com> <20191127053842.19481-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 4/5] avformat/matroskaenc: Remove allocations for attachments 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" If there are attachments to write, the Matroska muxer currently allocates two objects: An array that contains an entry for each attachment containing just the stream index of the corresponding stream and the uid used for this attachment; and a structure with a pointer to said array and a counter for said array. These uids are generated via code special to attachments: It uses an AVLFG in the normal and a sha of the attachment data in the bitexact case. (Said sha requires an allocation, too.) But now that an uid is generated for each stream in mkv_init(), there is no need any more to use special code for generating the uids of attachments: One can simply use the uid already generated for the attachment stream. And this makes the whole allocations of the structures for attachments as well as the structures itself superfluous. They have been removed. In case AVFMT_FLAG_BITEXACT is set, the uids will be different from the old ones which is the reason why the FATE-test lavf-mkv_attachment needed to be updated. The old method had the drawback that muxing the same attachment twice in the same file would create a file where two attachments have the same uid. The new one doesn't. Also notice that the dynamic buffer used to write the attachments leaks if an error happens when writing the buffer. By removing the allocations potential sources of errors have been removed. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskaenc.c | 63 +++++------------------------------ tests/ref/lavf/mkv_attachment | 4 +-- 2 files changed, 11 insertions(+), 56 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index aecd776157..1833d2a122 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -50,7 +50,6 @@ #include "libavutil/random_seed.h" #include "libavutil/rational.h" #include "libavutil/samplefmt.h" -#include "libavutil/sha.h" #include "libavutil/stereo3d.h" #include "libavcodec/xiph.h" @@ -104,16 +103,6 @@ typedef struct mkv_track { int64_t ts_offset; } mkv_track; -typedef struct mkv_attachment { - int stream_idx; - uint32_t fileuid; -} mkv_attachment; - -typedef struct mkv_attachments { - mkv_attachment *entries; - int num_entries; -} mkv_attachments; - #define MODE_MATROSKAv2 0x01 #define MODE_WEBM 0x02 @@ -140,7 +129,6 @@ typedef struct MatroskaMuxContext { mkv_seekhead *seekhead; mkv_cues *cues; mkv_track *tracks; - mkv_attachments *attachments; AVPacket cur_audio_pkt; @@ -408,10 +396,6 @@ static void mkv_deinit(AVFormatContext *s) av_freep(&mkv->cues->entries); av_freep(&mkv->cues); } - if (mkv->attachments) { - av_freep(&mkv->attachments->entries); - av_freep(&mkv->attachments); - } av_freep(&mkv->tracks); } @@ -1700,14 +1684,17 @@ static int mkv_write_tags(AVFormatContext *s) } if (mkv->have_attachments && mkv->mode != MODE_WEBM) { - for (i = 0; i < mkv->attachments->num_entries; i++) { - mkv_attachment *attachment = &mkv->attachments->entries[i]; - AVStream *st = s->streams[attachment->stream_idx]; + for (i = 0; i < s->nb_streams; i++) { + mkv_track *track = &mkv->tracks[i]; + AVStream *st = s->streams[i]; + + if (st->codecpar->codec_type != AVMEDIA_TYPE_ATTACHMENT) + continue; if (!mkv_check_tag(st->metadata, MATROSKA_ID_TAGTARGETS_ATTACHUID)) continue; - ret = mkv_write_tag(s, st->metadata, MATROSKA_ID_TAGTARGETS_ATTACHUID, attachment->fileuid); + ret = mkv_write_tag(s, st->metadata, MATROSKA_ID_TAGTARGETS_ATTACHUID, track->uid); if (ret < 0) return ret; } @@ -1726,18 +1713,11 @@ static int mkv_write_attachments(AVFormatContext *s) { MatroskaMuxContext *mkv = s->priv_data; AVIOContext *dyn_cp, *pb = s->pb; - AVLFG c; int i, ret; if (!mkv->have_attachments) return 0; - mkv->attachments = av_mallocz(sizeof(*mkv->attachments)); - if (!mkv->attachments) - return AVERROR(ENOMEM); - - av_lfg_init(&c, av_get_random_seed()); - ret = mkv_add_seekhead_entry(mkv->seekhead, MATROSKA_ID_ATTACHMENTS, avio_tell(pb)); if (ret < 0) return ret; @@ -1746,20 +1726,14 @@ static int mkv_write_attachments(AVFormatContext *s) for (i = 0; i < s->nb_streams; i++) { AVStream *st = s->streams[i]; + mkv_track *track = &mkv->tracks[i]; ebml_master attached_file; - mkv_attachment *attachment = mkv->attachments->entries; AVDictionaryEntry *t; const char *mimetype = NULL; - uint32_t fileuid; if (st->codecpar->codec_type != AVMEDIA_TYPE_ATTACHMENT) continue; - attachment = av_realloc_array(attachment, mkv->attachments->num_entries + 1, sizeof(mkv_attachment)); - if (!attachment) - return AVERROR(ENOMEM); - mkv->attachments->entries = attachment; - attached_file = start_ebml_master(dyn_cp, MATROSKA_ID_ATTACHEDFILE, 0); if (t = av_dict_get(st->metadata, "title", NULL, 0)) @@ -1790,29 +1764,10 @@ static int mkv_write_attachments(AVFormatContext *s) return AVERROR(EINVAL); } - if (s->flags & AVFMT_FLAG_BITEXACT) { - struct AVSHA *sha = av_sha_alloc(); - uint8_t digest[20]; - if (!sha) - return AVERROR(ENOMEM); - av_sha_init(sha, 160); - av_sha_update(sha, st->codecpar->extradata, st->codecpar->extradata_size); - av_sha_final(sha, digest); - av_free(sha); - fileuid = AV_RL32(digest); - } else { - fileuid = av_lfg_get(&c); - } - av_log(s, AV_LOG_VERBOSE, "Using %.8"PRIx32" for attachment %d\n", - fileuid, mkv->attachments->num_entries); - put_ebml_string(dyn_cp, MATROSKA_ID_FILEMIMETYPE, mimetype); put_ebml_binary(dyn_cp, MATROSKA_ID_FILEDATA, st->codecpar->extradata, st->codecpar->extradata_size); - put_ebml_uint(dyn_cp, MATROSKA_ID_FILEUID, fileuid); + put_ebml_uint(dyn_cp, MATROSKA_ID_FILEUID, track->uid); end_ebml_master(dyn_cp, attached_file); - - 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); diff --git a/tests/ref/lavf/mkv_attachment b/tests/ref/lavf/mkv_attachment index 230dff369a..790c0f693b 100644 --- a/tests/ref/lavf/mkv_attachment +++ b/tests/ref/lavf/mkv_attachment @@ -1,3 +1,3 @@ -4a25c53150c09537cd4fcbff8f8f14ff *tests/data/lavf/lavf.mkv_attachment -472706 tests/data/lavf/lavf.mkv_attachment +f54cf193e1925eff8a44554116a5e149 *tests/data/lavf/lavf.mkv_attachment +472703 tests/data/lavf/lavf.mkv_attachment tests/data/lavf/lavf.mkv_attachment CRC=0xec6c3c68