From patchwork Sun Apr 5 15:59:13 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 18668 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 B6D8F44BE3F for ; Sun, 5 Apr 2020 19:00:11 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id A25F368B21B; Sun, 5 Apr 2020 19:00:11 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7FB746882A2 for ; Sun, 5 Apr 2020 19:00:03 +0300 (EEST) Received: by mail-wm1-f66.google.com with SMTP id x25so1104518wmc.0 for ; Sun, 05 Apr 2020 09:00:03 -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=Io/g4prOlvUeSkPSsvwE9hxuxKko0T/NOE4678gKJkk=; b=i+LcnVEaHGNZHELrVavSvn8Xbksf8Augf0yMWOc7htK/WGp55H+XC5q7MDYnFmJpaP vHMNQHHv+zcp6BQ1i6lu4BGCut12igovv7gPuK/OE84Lg8626XLT8ByKMOei+XAr1xtY KCj1RFqLece5WA4uEqsI3rfhS5CancQ+wsbnFA3H+Ak9cIhSZSJJZ7pClIxD3KOUpytD YO9jZ8MwVOLw219UoA6/ytklj8eBOSW4UrTfEyV3+MtppqdEXNIdYkOmKSntcg05fNbl 8yHEfOEjpUGV4l4UqPTfQnxHp+XGjz9TWvYpoG/E/ozetbF9aK7bURcrtdtEOl8SZ1q4 JudA== 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=Io/g4prOlvUeSkPSsvwE9hxuxKko0T/NOE4678gKJkk=; b=bD9xOELjeWXmdiduMryJybsWomB8a+P4GgJrjr48340vouuSIPh5AqQwE362mKLGKz 78nhUlMV2FKJd+Jd/3r0lGm85V/sk16TIITTHcNxDoAp+JObJ2s4yxHpzZpbG36VMsyn zQvm1yID+7cYUOiWiRgKVz9kLzocaRzE9E80wAgCMl+WlIYiHLyODn3kGkgNzfTMAe6h 4vE5b2pq7HzYPyiRx8441MgRFaKzIK0nl0YYY3XF1z0O2jCJqQvf7zpwgSA6GDpWyH0c GoiS2HChThBRRWnZKKqn9aZ2vrHDnuLvQVVtq1mVLnWpgPze3v+LjyRZFr/OKG14yLOA 0ZeQ== X-Gm-Message-State: AGi0PubfA0Os0Q7tphyqHCe0A2BYlXTBiyDlIyY+p9Y4paZcfY4Tboo0 f2kQ9dbndGtLkP2CScUYQcsheCui X-Google-Smtp-Source: APiQypI9tJyrX0OTunMeZk5iLxGyf89wpk0JovlCLIPptnND1GN1+B0FqW3FSbVPIkKpGavPBFVP7Q== X-Received: by 2002:a1c:6285:: with SMTP id w127mr19492736wmb.133.1586102402460; Sun, 05 Apr 2020 09:00:02 -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.09.00.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Apr 2020 09:00:01 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sun, 5 Apr 2020 17:59:13 +0200 Message-Id: <20200405155928.9323-6-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 05/20] 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 AttachedFile containing just the stream index of the corresponding stream and the FileUID used for this AttachedFile; 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 FileUIDs of AttachedFiles: One can simply use the uid already generated for the corresponding stream. And this makes the whole allocations of the structures for AttachedFiles 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 two AttachedFiles with the same data would have the same FileUID. 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 --- This will be the last patch of my "remove allocations for"-series; after this there won't be any allocations to remove any more. 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 b93c50cb01..135d5e3abd 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" @@ -105,16 +104,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 @@ -141,7 +130,6 @@ typedef struct MatroskaMuxContext { mkv_seekhead seekhead; mkv_cues cues; mkv_track *tracks; - mkv_attachments *attachments; AVPacket cur_audio_pkt; @@ -403,10 +391,6 @@ static void mkv_deinit(AVFormatContext *s) ffio_free_dyn_buf(&mkv->tags_bc); av_freep(&mkv->cues.entries); - if (mkv->attachments) { - av_freep(&mkv->attachments->entries); - av_freep(&mkv->attachments); - } av_freep(&mkv->tracks); } @@ -1617,15 +1601,18 @@ 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, NULL); + track->uid, NULL); if (ret < 0) return ret; } @@ -1645,18 +1632,11 @@ static int mkv_write_attachments(AVFormatContext *s) { MatroskaMuxContext *mkv = s->priv_data; AVIOContext *dyn_cp = NULL, *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()); - mkv_add_seekhead_entry(mkv, MATROSKA_ID_ATTACHMENTS, avio_tell(pb)); ret = start_ebml_master_crc32(&dyn_cp, mkv); @@ -1664,20 +1644,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)) @@ -1708,29 +1682,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, MATROSKA_ID_ATTACHMENTS, 0, 0); diff --git a/tests/ref/lavf/mkv_attachment b/tests/ref/lavf/mkv_attachment index 56b1078d4a..08c9c1ebdb 100644 --- a/tests/ref/lavf/mkv_attachment +++ b/tests/ref/lavf/mkv_attachment @@ -1,3 +1,3 @@ -c339e42793d2a3cb1b8e99fc7a60de31 *tests/data/lavf/lavf.mkv_attachment -472569 tests/data/lavf/lavf.mkv_attachment +6f68dccaaaf875a0671f47e5b21c8cca *tests/data/lavf/lavf.mkv_attachment +472566 tests/data/lavf/lavf.mkv_attachment tests/data/lavf/lavf.mkv_attachment CRC=0xec6c3c68