From patchwork Wed Jan 1 13:27:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 17114 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 BC7544490FA for ; Wed, 1 Jan 2020 15:28:14 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 9224768AB16; Wed, 1 Jan 2020 15:28:14 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 0F6A968A5BE for ; Wed, 1 Jan 2020 15:28:07 +0200 (EET) Received: by mail-wm1-f65.google.com with SMTP id p9so3550152wmc.2 for ; Wed, 01 Jan 2020 05:28:07 -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:mime-version :content-transfer-encoding; bh=E4R7sJlmtGOizkp2ZaFOWA1oQfB9pQ8aWAUqDn5HQNU=; b=u5H5Vrw7Qffdu1XzHbfkhtNRN3VaDO6wkwsaAwHlQvkLFs0YayOWOn6VKSYpcvU9U4 KnWgHSoqm3qIVSMsfZlcbWGQBul32BJSQef1KLYcfFGLgR1hslBhB3tr1/PRHnU28h7l h88HPO6Dp996v+WXVNVOITilOCK+JnweOxBbaBYRmg5U0aLzqOPi6CyFpn37jQl4/D87 1HaGhLtwv03J9DnNz6m440n9KgrSaCjpxONEx1BZrWHpvXLtKBsN7csMT/Z8GYD4vxjt GlcpjQYV422mb0aRLkYa7XM3mTEMHx9NBttB1THdteAQQd4xGlAKVnGX9SaYbX0swI1d DJ9Q== 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:mime-version :content-transfer-encoding; bh=E4R7sJlmtGOizkp2ZaFOWA1oQfB9pQ8aWAUqDn5HQNU=; b=owtZ737WW/NCwd/01oI2fYlRIgWBsRzJMlA29dFOwe9AZXqGJfgOPDq4g9ZG+NBau+ PPDcVQMjDA8IP13UAw45hF66uuhZ08jYEToiqX3xUU1F8yRPun5R8CPhOCcXvsnIYm2q +REs9aizn4suAd7OZG+Miqv6CifI/DuQUgFM/mpfuoYKoTPFEu7dMzyCPynnxHNCC3Ha ggi8a2cPu6FxDgNbHESMDDXKvKQiEHxn25tx9jXroiTjmh0sDMptnVzwIWR5A2Jty/GG uhStKd0QjGYqN9DTschJn70mEt6ObljTE9WtwqNcY82WMyq4sa6CwRvKSlRR4sMg8n4E dsyQ== X-Gm-Message-State: APjAAAVm9gB77U0dvSoP7sLrVJ8yc8OFMeUz/wfVC312pDUZnD5vkh+/ wTT6zgszsnvsuLhdK1D0i+G1AY17 X-Google-Smtp-Source: APXvYqxSyqpJeAwDsJzeqEiWFSf8U6cohXM7oClHtEE+dGhTU7L+ceAOqO/0q2h1nNcCd6QWiP4mgg== X-Received: by 2002:a1c:f210:: with SMTP id s16mr9125140wmc.57.1577885286166; Wed, 01 Jan 2020 05:28:06 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc08bbf.dynamic.kabel-deutschland.de. [188.192.139.191]) by smtp.gmail.com with ESMTPSA id f17sm5520462wmc.8.2020.01.01.05.28.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Jan 2020 05:28:05 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 1 Jan 2020 14:27:57 +0100 Message-Id: <20200101132758.4452-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 11/17] avutil/mem: Add av_fast_realloc_array() 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" This is an array-equivalent of av_fast_realloc(). Its advantages compared to using av_fast_realloc() for allocating arrays are as follows: a) It performs its own overflow checks for the multiplication that is implicit in array allocations. (And it only needs to perform these checks (as well as the multiplication itself) in case the array needs to be reallocated.) b) It allows to limit the number of elements to an upper bound given by the caller. This allows to restrict the number of allocated elements to fit into an int and therefore makes this function usable with counters of this type. It can also be used to avoid overflow checks in the caller: E.g. setting it to UINT_MAX - 1 elements makes it safe to increase the desired number of elements in steps of one. And it avoids overallocations in situations where one already has an upper bound. c) Should the caller have increased max_alloc_size, av_fast_realloc() could come in a situation where it allocates more than what fits into an unsigned. In this case the variable containing the allocated size (an unsigned) won't accurately reflect how much has been allocated. After this point, lots of reallocations will happen despite the buffer actually being big enough. d) av_fast_realloc_array() will always allocate in multiples of array elements; no memory is wasted with partial elements. e) By returning an int, av_fast_realloc_array() can distinguish between ordinary allocation failures (meriting AVERROR(ENOMEM)) and failures because of allocation limits (by returning AVERROR(ERANGE)). f) It is no longer possible for the user to accidentally lose the pointer by using ptr = av_fast_realloc(ptr, ...). Because of f) there is no need to set the number of allocated elements to zero on failure. av_fast_realloc() usually allocates size + size / 16 + 32 bytes if size bytes are desired and if the already existing buffer isn't big enough. av_fast_realloc_array() instead allocates nb + (nb + 14) / 16. Rounding up is done in order not to reallocate in steps of one if the current number is < 16; adding 14 instead of 15 has the effect of only allocating one element if one element is desired. This is done with an eye towards applications where arrays might commonly only contain one element (as happens with the Matroska CueTrackPositions). Which of the two functions allocates faster depends upon the size of the elements. E.g. if the elements have a size of 32B and the desired size is incremented in steps of one, allocations happen at 1, 3, 5, 7, 9, 11, 13, 15, 17, 20, 23, 26 ... for av_fast_realloc(), 1, 2, 4, 6, 8, 10, 12, 14, 16, 18, 21, 24 ... for av_fast_realloc_array(). For element sizes of 96B, the numbers are 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 13, 15, 17, 19, 21, 23, 25, 27, 30 ... for av_fast_realloc() whereas the pattern for av_fast_realloc_array() is unchanged. Signed-off-by: Andreas Rheinhardt --- Switched to returning an int and added the max_nb parameter. What has not been done is switching to size_t. This function can still be turned into a wrapper for a size_t function if the need for such a function arises. Furthermore, I have found that several reallocation functions don't abide by their documented behaviour: "If `size` is zero, free the memory block pointed to by `ptr`." says the documentation of av_realloc, av_reallocp, av_realloc_f (implicitly); av_realloc_array and av_reallocp_array claim to free the memory block in case the number of elements to allocate is zero. Yet realloc allocates size + !size bytes. av_realloc_f (calling av_realloc) does also not free its array in case zero elements should be allocated (or if the size of an element is zero). av_reallocp does what its documentation says; av_realloc_array (relying on av_realloc) and av_reallocp_array (relying on av_realloc_f) don't. Changing the behaviour of av_realloc to match its documentation leads to lots of failing FATE-tests, so I suggest updating the documentation to match actual behaviour. Finally, there is no check in av_max_alloc that the new max is actually bigger than 32; this is a problem given that max_alloc_size - 32 is the real limit. Maybe the 32 should simply be dropped (and the default value be set to INT_MAX - 32 if it is deemed important)? (And why 32? I would have expected AV_INPUT_BUFFER_PADDING_SIZE or so.) doc/APIchanges | 3 +++ libavutil/mem.c | 32 ++++++++++++++++++++++++++++++++ libavutil/mem.h | 28 ++++++++++++++++++++++++++++ libavutil/version.h | 2 +- 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/doc/APIchanges b/doc/APIchanges index 3c24dc6fbc..7feca58e98 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,9 @@ libavutil: 2017-10-21 API changes, most recent first: +2020-01-01 - xxxxxxxxxx - lavu 56.39.100 - mem.h + Add av_fast_realloc_array(). + 2019-12-27 - xxxxxxxxxx - lavu 56.38.100 - eval.h Add av_expr_count_func(). diff --git a/libavutil/mem.c b/libavutil/mem.c index 88fe09b179..56c82fcee7 100644 --- a/libavutil/mem.c +++ b/libavutil/mem.c @@ -497,6 +497,38 @@ void *av_fast_realloc(void *ptr, unsigned int *size, size_t min_size) return ptr; } +int av_fast_realloc_array(void *ptr, unsigned *nb_allocated, + unsigned min_nb, unsigned max_nb, size_t elsize) +{ + void *array; + unsigned nb; + + if (min_nb <= *nb_allocated) + return 0; + + max_nb = FFMIN(max_nb, (max_alloc_size - 32) / elsize); + + if (min_nb > max_nb) + return AVERROR(ERANGE); + + nb = min_nb + (min_nb + 14) / 16; + + /* If min_nb is so big that the above calculation overflowed, + * just allocate as much as we are allowed to. */ + nb = nb < min_nb ? max_nb : FFMIN(nb, max_nb); + + memcpy(&array, ptr, sizeof(array)); + + array = av_realloc(array, nb * elsize); + if (!array) + return AVERROR(ENOMEM); + + memcpy(ptr, &array, sizeof(array)); + *nb_allocated = nb; + + return 0; +} + void av_fast_malloc(void *ptr, unsigned int *size, size_t min_size) { ff_fast_malloc(ptr, size, min_size, 0); diff --git a/libavutil/mem.h b/libavutil/mem.h index 5fb1a02dd9..e05550f363 100644 --- a/libavutil/mem.h +++ b/libavutil/mem.h @@ -370,11 +370,39 @@ int av_reallocp_array(void *ptr, size_t nmemb, size_t size); * @return `ptr` if the buffer is large enough, a pointer to newly reallocated * buffer if the buffer was not large enough, or `NULL` in case of * error + * @see av_fast_realloc_array() * @see av_realloc() * @see av_fast_malloc() */ void *av_fast_realloc(void *ptr, unsigned int *size, size_t min_size); +/** + * Reallocate the given array if it is not large enough, otherwise do nothing. + * + * If `ptr` points to `NULL`, then a new uninitialized array is allocated. + * + * @param[in,out] ptr Pointer to `NULL` or an already allocated array. + * `*ptr` will be set to point to the new array. + * @param[in,out] nb_allocated Pointer to the number of elements of the array + * `*ptr`. `*nb_allocated` is updated to the new + * number of allocated elements. + * @param[in] min_nb Desired minimal number of elements in array `*ptr` + * @param[in] max_nb Maximal number of elements to allocate. + * @param[in] elsize Size of a single element of the array. + * Must not be zero. + * @return 0 on success, < 0 on failure. On failure, `*ptr` is not freed and + * `*ptr` as well as `*nb_allocated` are unchanged. + * @note `max_nb` can be used to limit allocations and make this function usable + * with counters of type int. It can also be used to avoid overflow checks + * in callers: E.g. setting it to `UINT_MAX - 1` means that incrementing + * an unsigned in steps of one need not be checked for overflow. + * @see av_fast_realloc() + * @see av_realloc() + * @see av_fast_malloc() + */ +int av_fast_realloc_array(void *ptr, unsigned *nb_allocated, + unsigned min_nb, unsigned max_nb, size_t elsize); + /** * Allocate a buffer, reusing the given one if large enough. * diff --git a/libavutil/version.h b/libavutil/version.h index af8f614aff..2bc1b98615 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,7 +79,7 @@ */ #define LIBAVUTIL_VERSION_MAJOR 56 -#define LIBAVUTIL_VERSION_MINOR 38 +#define LIBAVUTIL_VERSION_MINOR 39 #define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ From patchwork Wed Jan 1 13:27:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 17115 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 F040D4490FA for ; Wed, 1 Jan 2020 15:28:25 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id CB75268AD5A; Wed, 1 Jan 2020 15:28:25 +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 2C92068A5BE for ; Wed, 1 Jan 2020 15:28:19 +0200 (EET) Received: by mail-wr1-f67.google.com with SMTP id z7so36915937wrl.13 for ; Wed, 01 Jan 2020 05:28:19 -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=+rdPkdITsiakV8ycRHFi/vjXKZJYMnjPyOGikqUf2Os=; b=olOfrBXlls3M+FWOlBEqV+gAX6ytXQDbgAJ5fNW/UDFLUq5tv6WTQFAXNGjmgtnQMP +DRsyeJTOXo2vdmFSVcyJf0kPxwZ8ss8SRx6ejU7iXAgo7BuqlkZ7G6GFC4+vKThPsb8 AoZIOpAilTbSSwNwSEYHh46U7XIqbvxs7EPMcGdoNEmTLljBOoU68N7S/3OGJmoFp7oo 0IQk9FleZsi6WNnrwg2gk2FfWhqXEKj/PtQEIaT69F+qS3L4ydqIgd+5RLLtVcCJDcyo oIfUR1SglMZX4t4vv0XkwrcwJRqvZj7mFOAFr4pPLQhD4a9LsUdW7h3gSt529GszhFLZ z9Gw== 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=+rdPkdITsiakV8ycRHFi/vjXKZJYMnjPyOGikqUf2Os=; b=ntVMkfY5Nf6J6p/WeuLiitEBz80Nfg1b9hn2PPDCLUZ8eKNQkpUQoOo3mUGTWoyNrL L/gj/vvZxkAYo2RC1ZG7gHZn1e1GOJHm/uLC9GxKr2uBxdB34x/n/zmpvwIMaikWbPpx CnpFhf9f/ssHaeTk6rioGKRrCGP0OUIC+REgJgIlcFIa3SC7JpJJZFIB02snttX0Ft6Y rds1PtZrWw37eLluZYpnUgiltEqKWbZDaKv9U7oAOVEfJXf/cF3ewI297CU46jTtqUcl 2B2BMRYNVtaq4fJEBdcK/+HWWxFjX4bCOuTL4A46VkCP1K03HVLYR6hDKJ8K2xLaY9Rh 5PIg== X-Gm-Message-State: APjAAAUM453tCHM5zObFYidL6QVRzFJQ9+pAus4G10EaK6etR+rsWCsK ngknBxKvCfAFOfea2H4ZMxFRjVzS X-Google-Smtp-Source: APXvYqxBtuidZf9ox6RutxbfdbUBKc4BpRM79hGhL6QZZYj1zSMAzOKUuP8ojYbZGK5hqN8rlNmPhA== X-Received: by 2002:adf:b60f:: with SMTP id f15mr73293725wre.372.1577885298513; Wed, 01 Jan 2020 05:28:18 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc08bbf.dynamic.kabel-deutschland.de. [188.192.139.191]) by smtp.gmail.com with ESMTPSA id f17sm5520462wmc.8.2020.01.01.05.28.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Jan 2020 05:28:18 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 1 Jan 2020 14:27:58 +0100 Message-Id: <20200101132758.4452-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200101132758.4452-1-andreas.rheinhardt@gmail.com> References: <20200101132758.4452-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 12/17] avformat/flvenc: Use array instead of linked list for index 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" Using a linked list had very much overhead (the pointer to the next entry increased the size of the index entry struct from 16 to 24 bytes, not to mention the overhead of having separate allocations), so it is better to (re)allocate a continuous array for the index. av_fast_realloc_array() is used for this purpose, in order not to reallocate the array for each entry. Signed-off-by: Andreas Rheinhardt --- libavformat/flvenc.c | 58 ++++++++++++++------------------------------ 1 file changed, 18 insertions(+), 40 deletions(-) diff --git a/libavformat/flvenc.c b/libavformat/flvenc.c index 1aaf0333ca..74f4e499f6 100644 --- a/libavformat/flvenc.c +++ b/libavformat/flvenc.c @@ -74,7 +74,6 @@ typedef enum { typedef struct FLVFileposition { int64_t keyframe_position; double keyframe_timestamp; - struct FLVFileposition *next; } FLVFileposition; typedef struct FLVContext { @@ -108,9 +107,9 @@ typedef struct FLVContext { int acurframeindex; int64_t keyframes_info_offset; - int64_t filepositions_count; FLVFileposition *filepositions; - FLVFileposition *head_filepositions; + unsigned filepositions_count; + unsigned filepositions_allocated; AVCodecParameters *audio_par; AVCodecParameters *video_par; @@ -549,27 +548,17 @@ static void flv_write_codec_header(AVFormatContext* s, AVCodecParameters* par, i static int flv_append_keyframe_info(AVFormatContext *s, FLVContext *flv, double ts, int64_t pos) { - FLVFileposition *position = av_malloc(sizeof(FLVFileposition)); - - if (!position) { - av_log(s, AV_LOG_WARNING, "no mem for add keyframe index!\n"); - return AVERROR(ENOMEM); - } - - position->keyframe_timestamp = ts; - position->keyframe_position = pos; - - if (!flv->filepositions_count) { - flv->filepositions = position; - flv->head_filepositions = flv->filepositions; - position->next = NULL; - } else { - flv->filepositions->next = position; - position->next = NULL; - flv->filepositions = flv->filepositions->next; + int ret = av_fast_realloc_array(&flv->filepositions, + &flv->filepositions_allocated, + flv->filepositions_count + 1, + UINT_MAX - 1, + sizeof(*flv->filepositions)); + if (ret < 0) { + av_log(s, AV_LOG_WARNING, "Adding entry to keyframe index failed.\n"); + return ret; } - flv->filepositions_count++; + flv->filepositions[flv->filepositions_count++] = (FLVFileposition){ pos, ts }; return 0; } @@ -586,7 +575,7 @@ static int shift_data(AVFormatContext *s) int read_size[2]; AVIOContext *read_pb; - metadata_size = flv->filepositions_count * 9 * 2 + 10; /* filepositions and times value */ + metadata_size = flv->filepositions_count * 9LL * 2 + 10; /* filepositions and times value */ metadata_size += 2 + 13; /* filepositions String */ metadata_size += 2 + 5; /* times String */ metadata_size += 3; /* Object end */ @@ -784,7 +773,7 @@ static int flv_write_trailer(AVFormatContext *s) int64_t cur_pos = avio_tell(s->pb); if (build_keyframes_idx) { - FLVFileposition *newflv_posinfo, *p; + FLVFileposition *flv_posinfo = flv->filepositions; avio_seek(pb, flv->videosize_offset, SEEK_SET); put_amf_double(pb, flv->videosize); @@ -809,28 +798,17 @@ static int flv_write_trailer(AVFormatContext *s) avio_seek(pb, flv->keyframes_info_offset, SEEK_SET); put_amf_string(pb, "filepositions"); put_amf_dword_array(pb, flv->filepositions_count); - for (newflv_posinfo = flv->head_filepositions; newflv_posinfo; newflv_posinfo = newflv_posinfo->next) { - put_amf_double(pb, newflv_posinfo->keyframe_position + flv->keyframe_index_size); + for (unsigned i = 0; i < flv->filepositions_count; i++) { + put_amf_double(pb, flv_posinfo[i].keyframe_position + flv->keyframe_index_size); } put_amf_string(pb, "times"); put_amf_dword_array(pb, flv->filepositions_count); - for (newflv_posinfo = flv->head_filepositions; newflv_posinfo; newflv_posinfo = newflv_posinfo->next) { - put_amf_double(pb, newflv_posinfo->keyframe_timestamp); + for (unsigned i = 0; i < flv->filepositions_count; i++) { + put_amf_double(pb, flv_posinfo[i].keyframe_timestamp); } - newflv_posinfo = flv->head_filepositions; - while (newflv_posinfo) { - p = newflv_posinfo->next; - if (p) { - newflv_posinfo->next = p->next; - av_free(p); - p = NULL; - } else { - av_free(newflv_posinfo); - newflv_posinfo = NULL; - } - } + av_freep(&flv->filepositions); put_amf_string(pb, ""); avio_w8(pb, AMF_END_OF_OBJECT); From patchwork Wed Jan 1 13:46:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 17116 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 E8F07449BA5 for ; Wed, 1 Jan 2020 15:47:12 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C3F676881CD; Wed, 1 Jan 2020 15:47:12 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7A2A26880B2 for ; Wed, 1 Jan 2020 15:47:06 +0200 (EET) Received: by mail-wm1-f67.google.com with SMTP id q9so3552332wmj.5 for ; Wed, 01 Jan 2020 05:47:06 -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=kYjDxkQFQQ6bghDX5LgGtyTqaUPbb0Y1DpgHIAJoVzc=; b=h6SvcXhQdtzjyykCdMSGGsn4I/VwNf+4201CJzvCojJLxFbBS/JX9nQ3RhmCLkMf29 xOlWNuHFt4KkLaHaFvjr/yeZnFSOV8SdmwjpkSZo2ppPHmH7Rv6mXuax3jnlvjusjcOQ avZNykPooGv4ROv8jdTXVGN8qjz5/fUrOER9thknVyYExLUZUvkqcWj6AZUc3ApHsPHj lQGHPcvV4GDEpWNOboQ3YG4iIExbuqFmj0A/2bHF7+Zx7igCLglG2s0QtLamlBZlDwjo EFFslLx1TtCuWJ+qCFRyzID8t9CqaSxm4+1hedCLTJcpFnaDCi066etYXvN4kk5AVY5C fMSQ== 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=kYjDxkQFQQ6bghDX5LgGtyTqaUPbb0Y1DpgHIAJoVzc=; b=hNOIupzj5dHFO/HpWId9OEP0tGzpA4NLi3WnWOPCz5PGRwv+Tu23AuuZQOM8w72yif aGC4HgAg6hJtH6onfaYBWApnuC8+0BXqSnrAvSpHhLYCZVm8APgFFXWEDHH3fLZk2A+h BLOPLoMfrgw6cxi6o38PMkp884Ia5MK6crL/LoZbsSCpczrmoLClsYcOmQNMUR8klskX +mUngUCJ7W/TGiDbxj5FZcUWfmv4q9hV0p3q/SWB2wLS2uvpJ4bC2rf1lgdixid3gOeg MLX8d+Zz8m6XGdONu9MloRcIEBVVKi1tDaejO7XPgEGMOUzUVj0APvhuZTX8nz6Z9jv3 mnNw== X-Gm-Message-State: APjAAAU5FaZfwMztc9oJvl+zZlwcmTORA4ueJ5NXOEVqbSJAJQaD6SXo AH9ZbChYithQqUX24WdOn7tgcimD X-Google-Smtp-Source: APXvYqym6yD4eBzVmR+kjVQnYBJL2GVPi4cjXXHbo+TRMeMRPGZdKLordEQPk5g9k5S6Ulor2QTQSA== X-Received: by 2002:a05:600c:1003:: with SMTP id c3mr9485374wmc.47.1577886425753; Wed, 01 Jan 2020 05:47:05 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc08bbf.dynamic.kabel-deutschland.de. [188.192.139.191]) by smtp.gmail.com with ESMTPSA id z6sm54491593wrw.36.2020.01.01.05.47.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Jan 2020 05:47:05 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 1 Jan 2020 14:46:56 +0100 Message-Id: <20200101134657.12801-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200101132758.4452-1-andreas.rheinhardt@gmail.com> References: <20200101132758.4452-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 15/17] avformat/matroskaenc: Use av_fast_realloc_array for index entries 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" Currently, the Matroska muxer reallocates its array of index entries each time another entry is added. This is bad performance-wise, especially on Windows where reallocations are slow. This is solved by switching to av_fast_realloc_array() which ensures that actual reallocations will happen only seldomly. For an (admittedly extreme) example which consists of looping a video consisting of a single keyframe of size 4KB 540000 times this improved the time for writing a frame from 23524201 decicycles (516466 runs, 7822 skips) to 225240 decicycles (522122 runs, 2166 skips) on Windows. (Writing CRC-32 elements was disabled for these tests.) Signed-off-by: Andreas Rheinhardt --- This patch will lead to a merge conflict with my recent Matroska muxer patchset [1] (in particular, [2]), but it is trivially fixable. I will update the other patchset as soon as one gets applied. [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-January/255139.html [2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-January/255151.html libavformat/matroskaenc.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 469b604de6..2f5f8873db 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -88,7 +88,8 @@ typedef struct mkv_cuepoint { typedef struct mkv_cues { int64_t segment_offset; mkv_cuepoint *entries; - int num_entries; + unsigned num_entries; + unsigned allocated_entries; } mkv_cues; typedef struct mkv_track { @@ -171,7 +172,7 @@ typedef struct MatroskaMuxContext { /** 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 +#define MAX_CUETRACKPOS_SIZE 35LL /** 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 @@ -535,15 +536,16 @@ static mkv_cues *mkv_start_cues(int64_t segment_offset) static int mkv_add_cuepoint(mkv_cues *cues, int stream, int tracknum, int64_t ts, int64_t cluster_pos, int64_t relative_pos, int64_t duration) { - mkv_cuepoint *entries = cues->entries; + int ret; if (ts < 0) return 0; - entries = av_realloc_array(entries, cues->num_entries + 1, sizeof(mkv_cuepoint)); - if (!entries) - return AVERROR(ENOMEM); - cues->entries = entries; + ret = av_fast_realloc_array(&cues->entries, &cues->allocated_entries, + cues->num_entries + 1, UINT_MAX - 1, + sizeof(*cues->entries)); + if (ret < 0) + return ret; cues->entries[cues->num_entries].pts = ts; cues->entries[cues->num_entries].stream_idx = stream; @@ -560,21 +562,21 @@ static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tra MatroskaMuxContext *mkv = s->priv_data; AVIOContext *dyn_cp, *pb = s->pb; 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++) { + for (unsigned 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; + unsigned ctp_nb = 0, j; // Calculate the number of entries, so we know the element size - 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; @@ -591,7 +593,7 @@ static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tra // 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; From patchwork Wed Jan 1 13:46:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 17117 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 D277C449BA5 for ; Wed, 1 Jan 2020 15:47:26 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id B6FA968AD32; Wed, 1 Jan 2020 15:47:26 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id DFD4668AD25 for ; Wed, 1 Jan 2020 15:47:20 +0200 (EET) Received: by mail-wr1-f66.google.com with SMTP id c9so37016903wrw.8 for ; Wed, 01 Jan 2020 05:47:20 -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=hYD6Fhe4KN74iWw0fB6jdmHVAgURpdKx7B73mTdCaO8=; b=rMOt4JqAXtH6JJ19Z3UWSfUGipNS7NuGFYNPn/g2Fmz1P6JM8watb6WxMvYVxTiam2 AYuthPh8uLjAapyZxb+E7Lolnk3kTDhmgDWX+NZg+gPUQyBh8e6zsALrbkhTSssgMLx5 wZwvpVudBk7Um1AW4OPrQVsaKnAZ+T/s6KAJacbfLf6udK8J/PSu7a87FzPFxs7922yq O6ZbkSFuusV2grqmOZghcHNQfKljLX8zkvxymVPZSgFxJl0S78dgR8oWUfai3hofCrzz 5DXvXsoNB4zZGT6l182i6GSuJn/0pbTBOKDUbDi+hxGAdqX8MxaUN1K0dra4uFYTMg08 ohIA== 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=hYD6Fhe4KN74iWw0fB6jdmHVAgURpdKx7B73mTdCaO8=; b=lo4OwnWEwpy/fo2Kqqtifjr/d7pb2VrQ70uZ9EQiAMM0yXeBvYfT1rek2TBnA5KA7m je7zY/JtmuTt/7j6xpqKMqVt4lHymBHVb/isLb1B/HLnUfmz8J3/NQ+InfCHHipTl+zQ mSC1Qt4xPN6f2dKA6y0dX00xVkcgziav39rmVKYtKX3o8rjd7dTiWRFWU7haWxr8bbz2 v4tcaTXiWI9sEv2Rr+eWCH+ekDdaVZ0SMcXZcXGVXZ4aSTjHJpvMISn6staHW+yrI2pm oBDAXcZX72rppAVByDwjCRdC6yv6cJFQxEu+NJcH865xW8GmYjLEGSM373lB0E1/oFuR tSwQ== X-Gm-Message-State: APjAAAU/xksnjLFypN8cDelTbV4CZSIdjk9zux8aM1KHUqxdUWmNq8g9 /jHsVflid9Bso1X4j8CdnMtiTuKs X-Google-Smtp-Source: APXvYqwEyFjqzrSZ0FOX8ho3HazLs9YcZ8+iGr1tVbggDEgln53Y4mY6a2v9+1vto/l2o7Mzl597VA== X-Received: by 2002:adf:bc4f:: with SMTP id a15mr77641185wrh.160.1577886440177; Wed, 01 Jan 2020 05:47:20 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc08bbf.dynamic.kabel-deutschland.de. [188.192.139.191]) by smtp.gmail.com with ESMTPSA id z6sm54491593wrw.36.2020.01.01.05.47.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Jan 2020 05:47:19 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 1 Jan 2020 14:46:57 +0100 Message-Id: <20200101134657.12801-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200101132758.4452-1-andreas.rheinhardt@gmail.com> References: <20200101132758.4452-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 16/17] avformat/matroskadec: Use av_fast_realloc_array() 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" instead of av_fast_realloc() for allocating an array. It has the advantage of doing it's own overflow checks and does not overallocate unnecessarily: It allocates exactly one element if one element is desired. This is advantageous for CueTrackPositions: While it is allowed (and supported) to have more than one of them in a CuePoint, there is typically only one of them, so that overallocating is a waste. The memory used for a file with 540000 cues entries therefore decreased from 69624 KB to 52592 KB. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskadec.c | 59 +++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 75f72d330c..2295bafd22 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -109,8 +109,8 @@ typedef const struct EbmlSyntax { } EbmlSyntax; typedef struct EbmlList { - int nb_elem; - unsigned int alloc_elem_size; + unsigned nb_elem; + unsigned nb_allocated; void *elem; } EbmlList; @@ -1226,16 +1226,12 @@ static int ebml_parse(MatroskaDemuxContext *matroska, data = (char *) data + syntax->data_offset; if (syntax->list_elem_size) { EbmlList *list = data; - void *newelem; + res = av_fast_realloc_array(&list->elem, &list->nb_allocated, + list->nb_elem + 1, UINT_MAX - 1, + syntax->list_elem_size); + if (res < 0) + return res; - if ((unsigned)list->nb_elem + 1 >= UINT_MAX / syntax->list_elem_size) - return AVERROR(ENOMEM); - newelem = av_fast_realloc(list->elem, - &list->alloc_elem_size, - (list->nb_elem + 1) * syntax->list_elem_size); - if (!newelem) - return AVERROR(ENOMEM); - list->elem = newelem; data = (char *) list->elem + list->nb_elem * syntax->list_elem_size; memset(data, 0, syntax->list_elem_size); list->nb_elem++; @@ -1464,7 +1460,7 @@ level_check: static void ebml_free(EbmlSyntax *syntax, void *data) { - int i, j; + int i; for (i = 0; syntax[i].id; i++) { void *data_off = (char *) data + syntax[i].data_offset; switch (syntax[i].type) { @@ -1480,12 +1476,12 @@ static void ebml_free(EbmlSyntax *syntax, void *data) if (syntax[i].list_elem_size) { EbmlList *list = data_off; char *ptr = list->elem; - for (j = 0; j < list->nb_elem; + for (unsigned j = 0; j < list->nb_elem; j++, ptr += syntax[i].list_elem_size) ebml_free(syntax[i].def.n, ptr); av_freep(&list->elem); list->nb_elem = 0; - list->alloc_elem_size = 0; + list->nb_allocated = 0; } else ebml_free(syntax[i].def.n, data_off); default: @@ -1548,7 +1544,7 @@ static MatroskaTrack *matroska_find_track_by_num(MatroskaDemuxContext *matroska, int num) { MatroskaTrack *tracks = matroska->tracks.elem; - int i; + unsigned i; for (i = 0; i < matroska->tracks.nb_elem; i++) if (tracks[i].num == num) @@ -1703,7 +1699,7 @@ static void matroska_convert_tag(AVFormatContext *s, EbmlList *list, { MatroskaTag *tags = list->elem; char key[1024]; - int i; + unsigned i; for (i = 0; i < list->nb_elem; i++) { const char *lang = tags[i].lang && @@ -1737,7 +1733,7 @@ static void matroska_convert_tags(AVFormatContext *s) { MatroskaDemuxContext *matroska = s->priv_data; MatroskaTags *tags = matroska->tags.elem; - int i, j; + unsigned i, j; for (i = 0; i < matroska->tags.nb_elem; i++) { if (tags[i].target.attachuid) { @@ -1753,7 +1749,7 @@ static void matroska_convert_tags(AVFormatContext *s) } if (!found) { av_log(NULL, AV_LOG_WARNING, - "The tags at index %d refer to a " + "The tags at index %u refer to a " "non-existent attachment %"PRId64".\n", i, tags[i].target.attachuid); } @@ -1770,7 +1766,7 @@ static void matroska_convert_tags(AVFormatContext *s) } if (!found) { av_log(NULL, AV_LOG_WARNING, - "The tags at index %d refer to a non-existent chapter " + "The tags at index %u refer to a non-existent chapter " "%"PRId64".\n", i, tags[i].target.chapteruid); } @@ -1787,7 +1783,7 @@ static void matroska_convert_tags(AVFormatContext *s) } if (!found) { av_log(NULL, AV_LOG_WARNING, - "The tags at index %d refer to a non-existent track " + "The tags at index %u refer to a non-existent track " "%"PRId64".\n", i, tags[i].target.trackuid); } @@ -1836,7 +1832,7 @@ static int matroska_parse_seekhead_entry(MatroskaDemuxContext *matroska, static void matroska_execute_seekhead(MatroskaDemuxContext *matroska) { EbmlList *seekhead_list = &matroska->seekhead; - int i; + unsigned i; // we should not do any seeking in the streaming case if (!(matroska->ctx->pb->seekable & AVIO_SEEKABLE_NORMAL)) @@ -1872,7 +1868,7 @@ static void matroska_add_index_entries(MatroskaDemuxContext *matroska) EbmlList *index_list; MatroskaIndex *index; uint64_t index_scale = 1; - int i, j; + unsigned i, j; if (matroska->ctx->flags & AVFMT_FLAG_IGNIDX) return; @@ -2266,8 +2262,8 @@ static int matroska_parse_tracks(AVFormatContext *s) MatroskaDemuxContext *matroska = s->priv_data; MatroskaTrack *tracks = matroska->tracks.elem; AVStream *st; - int i, j, ret; - int k; + unsigned i, j, k; + int ret; for (i = 0; i < matroska->tracks.nb_elem; i++) { MatroskaTrack *track = &tracks[i]; @@ -2717,7 +2713,7 @@ static int matroska_parse_tracks(AVFormatContext *s) char buf[32]; if (planes[j].type >= MATROSKA_VIDEO_STEREO_PLANE_COUNT) continue; - snprintf(buf, sizeof(buf), "%s_%d", + snprintf(buf, sizeof(buf), "%s_%u", ff_matroska_video_stereo_plane[planes[j].type], i); for (k=0; k < matroska->tracks.nb_elem; k++) if (planes[j].uid == tracks[k].uid && tracks[k].stream) { @@ -2792,7 +2788,8 @@ static int matroska_read_header(AVFormatContext *s) uint64_t max_start = 0; int64_t pos; Ebml ebml = { 0 }; - int i, j, res; + unsigned i, j; + int res; matroska->ctx = s; matroska->cues_parsing_deferred = 1; @@ -3709,7 +3706,7 @@ static int matroska_read_seek(AVFormatContext *s, int stream_index, MatroskaDemuxContext *matroska = s->priv_data; MatroskaTrack *tracks = NULL; AVStream *st = s->streams[stream_index]; - int i, index; + int index; /* Parse the CUES now since we need the index data to seek. */ if (matroska->cues_parsing_deferred > 0) { @@ -3735,7 +3732,7 @@ static int matroska_read_seek(AVFormatContext *s, int stream_index, goto err; tracks = matroska->tracks.elem; - for (i = 0; i < matroska->tracks.nb_elem; i++) { + for (unsigned i = 0; i < matroska->tracks.nb_elem; i++) { tracks[i].audio.pkt_cnt = 0; tracks[i].audio.sub_packet_cnt = 0; tracks[i].audio.buf_timecode = AV_NOPTS_VALUE; @@ -3771,7 +3768,7 @@ static int matroska_read_close(AVFormatContext *s) { MatroskaDemuxContext *matroska = s->priv_data; MatroskaTrack *tracks = matroska->tracks.elem; - int n; + unsigned n; matroska_clear_queue(matroska); @@ -4044,7 +4041,7 @@ static int webm_dash_manifest_cues(AVFormatContext *s, int64_t init_range) MatroskaSeekhead *seekhead = seekhead_list->elem; char *buf; int64_t cues_start = -1, cues_end = -1, before_pos, bandwidth; - int i; + unsigned i; int end = 0; // determine cues start and end positions @@ -4100,7 +4097,7 @@ static int webm_dash_manifest_cues(AVFormatContext *s, int64_t init_range) buf = av_malloc_array(s->streams[0]->nb_index_entries, 20); if (!buf) return -1; strcpy(buf, ""); - for (i = 0; i < s->streams[0]->nb_index_entries; i++) { + for (int i = 0; i < s->streams[0]->nb_index_entries; i++) { int ret = snprintf(buf + end, 20, "%" PRId64"%s", s->streams[0]->index_entries[i].timestamp, i != s->streams[0]->nb_index_entries - 1 ? "," : "");