Re: [FFmpeg-devel] [PATCH 3/8] avutil/mem: Add av_fast_realloc_array()

["Tomas Härdin" <git@haerdin.se>]

[-- Attachment #1: Type: text/plain, Size: 2752 bytes --]

tis 2022-09-27 klockan 17:23 +0200 skrev Tomas Härdin:
> mån 2022-09-26 klockan 16:24 +0200 skrev Tomas Härdin:
> > mån 2022-09-26 klockan 14:25 +0200 skrev Andreas Rheinhardt:
> > > Anton Khirnov:
> > > > Quoting Andreas Rheinhardt (2022-07-14 14:51:07)
> > > > > Anton Khirnov:
> > > > > > Quoting Andreas Rheinhardt (2022-07-12 16:12:16)
> > > > > > > Anton really dislikes the av_fast_* naming and instead
> > > > > > > wants
> > > > > > > this to be
> > > > > > > called av_realloc_array_reuse(). I don't care either way.
> > > > > > > Any
> > > > > > > more
> > > > > > > opinions on this (or on the patch itself)?
> > > > > > 
> > > > > > If people dislike _reuse(), I am open to other reasonable
> > > > > > suggestions.
> > > > > > This 'fast' naming sucks because
> > > > > > - it tells you nothing about how this function is "fast"
> > > > > > - it is added at the beginning rather than the end, which
> > > > > > is
> > > > > >   against standard namespacing conventions
> > > > > > 
> > > > > 
> > > > > Isn't reusing the basic modus operandi for a reallocation
> > > > > function? So
> > > > > your suggested name doesn't seem to fit either.
> > > > 
> > > > Ordinary realloc just keeps the data, I wouldn't call that
> > > > "reuse"
> > > > since
> > > > it will often be a copy. This "fast" realloc OTOH reuses the
> > > > actual
> > > > buffer, same as all the other "fast" mem.h functions.
> > > > 
> > > > But feel free to suggest another naming pattern if you can
> > > > think
> > > > of
> > > > one.
> > > > 
> > > 
> > > I see two differences between this function and ordinary realloc:
> > > It
> > > never shrinks the buffer and it overallocates. These two
> > > properties
> > > make
> > > it more likely that these functions can avoid copies more often
> > > than
> > > plain realloc (but in contrast to realloc, we can not grow the
> > > buffer
> > > in
> > > case there is free space after it), but it is nevertheless the
> > > same
> > > as
> > > realloc.
> > > 
> > > But I don't really care that much about the name and will
> > > therefore
> > > use
> > > your name as I can't come up with anything better.
> > > (Of course, I am still open to alternative suggestions.)
> > > 
> > > - Andreas
> > 
> > So this means av_realloc_array_reuse()? Eh, it works. I will add a
> > function that also zeroes the newly allocated space, what should we
> > call that? av_realloc_array_reusez()?
> > av_realloc_array_reuse_zerofill()?
> 
> Here's a draft patch that calls it av_reallocz_array_reuse(). Needs a
> minor version bump of course

This makes me realize something: av_realloc_array_reuse() requires that
*nb_allocated == 0 initially but this isn't specified in the
documentation. Patch attached relaxes this.

/Tomas

[-- Attachment #2: 0001-lavu-mem-Do-not-require-nb_allocated-0-when-ptr-NULL.patch --]
[-- Type: text/x-patch, Size: 1149 bytes --]

From df72691f514e2437b1917d808b6fcd153c393c20 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
Date: Wed, 28 Sep 2022 11:34:45 +0200
Subject: [PATCH] lavu/mem: Do not require *nb_allocated == 0 when *ptr == NULL

---
 libavutil/mem.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavutil/mem.c b/libavutil/mem.c
index 781dcbaded..bd2ee342fe 100644
--- a/libavutil/mem.c
+++ b/libavutil/mem.c
@@ -561,7 +561,10 @@ int av_realloc_array_reuse(void *ptr, size_t *nb_allocated,
     void *array;
     size_t nb, max_alloc_size_bytes;
 
-    if (min_nb <= *nb_allocated)
+    memcpy(&array, ptr, sizeof(array));
+
+    // make no demands on *nb_allocated if *ptr == NULL
+    if (array && min_nb <= *nb_allocated)
         return 0;
 
     max_alloc_size_bytes = atomic_load_explicit(&max_alloc_size, memory_order_relaxed);
@@ -571,7 +574,6 @@ int av_realloc_array_reuse(void *ptr, size_t *nb_allocated,
         return AVERROR(ERANGE);
 
     nb = compute_nb(min_nb, max_nb);
-    memcpy(&array, ptr, sizeof(array));
 
     array = av_realloc(array, nb * elsize);
     if (!array)
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".