[FFmpeg-devel] [PATCH 0/7] ffmpeg: add a grow_array() helper function

[Michael Niedermayer]

On Thu, Aug 19, 2010 at 03:16:16PM +0200, Aurelien Jacobs wrote:
> On Wed, Aug 18, 2010 at 10:46:14PM +0200, Michael Niedermayer wrote:
> > On Tue, Aug 17, 2010 at 12:09:52AM +0200, Aurelien Jacobs wrote:
> > > On Mon, Aug 16, 2010 at 11:44:26AM -0400, Ronald S. Bultje wrote:
> > > > Hi,
> > > > 
> > > > On Mon, Aug 16, 2010 at 2:00 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > > > > On Fri, Aug 13, 2010 at 08:28:12PM +0200, Aurelien Jacobs wrote:
> > > > >> Ooops... I slightly messed up this patch serie.
> > > > >> I forgot the first patch which adds a grow_array() function which is
> > > > >> then used by all the following patches. Here it is.
> > > > >> The rest of the serie changes each *[MAX_STREAMS] arrays from ffmpeg.c
> > > > >> into a dynamically allocated array.
> > > > >>
> > > > >> Aurel
> > > > >>
> > > > >>
> > > > >> diff --git a/ffmpeg.c b/ffmpeg.c
> > > > >> index aec1f79..28ce27f 100644
> > > > >> --- a/ffmpeg.c
> > > > >> +++ b/ffmpeg.c
> > > > >> @@ -646,6 +646,23 @@ static int ffmpeg_exit(int ret)
> > > > >> ? ? ?return ret;
> > > > >> ?}
> > > > >>
> > > > >> +static void *grow_array(void *array, int elem_size, int *size, int new_size)
> > > > >> +{
> > > > >> + ? ?if (*size < new_size) {
> > > > >> + ? ? ? ?uint8_t *tmp = av_realloc(array, new_size*elem_size);
> > > > >
> > > > > integer overflow
> > > 
> > > Added a check.
> > > 
> > > > > also see ff_dynarray_add() and add notes that refer to it and back so changed
> > > > > done to one can be checked if they would also be usefull to the other
> > > 
> > > Note added.
> > > 
> > > > Isn't this the same as av_fast_realloc()?
> > > 
> > > Not exactly. It is different from both ff_dynarray_add() and
> > > av_fast_realloc(). This one indeed extand the size of an allocation, but
> > > it also memset(0) the newly allocated part (to allow sparse array with
> > > no uninitialized elements) and it keep track of the number of elements
> > > in the array, instead of the size of the buffer.
> > > 
> > > Aurel
> > >  ffmpeg.c             |   22 ++++++++++++++++++++++
> > >  libavformat/cutils.c |    1 +
> > >  2 files changed, 23 insertions(+)
> > > f233c6a7f4fba2259ca9fa0fa8d9431965737e40  grow_array.diff
> > > commit a392dcf1f80cec191262051cbb315f24549e724f
> > > Author: Aurelien Jacobs <aurel at gnuage.org>
> > > Date:   Fri Aug 13 16:58:19 2010 +0200
> > > 
> > >     ffmpeg: add a grow_array() helper function
> > > 
> > > diff --git a/ffmpeg.c b/ffmpeg.c
> > > index aec1f79..da74dce 100644
> > > --- a/ffmpeg.c
> > > +++ b/ffmpeg.c
> > > @@ -646,6 +646,28 @@ static int ffmpeg_exit(int ret)
> > >      return ret;
> > >  }
> > >  
> > > +/* similar to ff_dynarray_add() and av_fast_realloc() */
> > > +static void *grow_array(void *array, int elem_size, int *size, int new_size)
> > > +{
> > > +    if (new_size >= INT_MAX / elem_size) {
> > > +        fprintf(stderr, "Array too big.\n");
> > > +        ffmpeg_exit(1);
> > > +    }
> > > +    if (*size < new_size) {
> > > +        uint8_t *tmp = av_realloc(array, new_size*elem_size);
> > > +        if (!tmp) {
> > > +            fprintf(stderr, "Could not alloc buffer.\n");
> > > +            ffmpeg_exit(1);
> > > +        }
> > > +        memset(tmp + *size*elem_size, 0, (new_size-*size) * elem_size);
> > > +        *size = new_size;
> > > +        return tmp;
> > > +    }
> > > +    return array;
> > > +}
> > 
> > > +#define GROW_ARRAY(array, size)  \
> > > +    array = grow_array(array, sizeof(*array), &nb_##array, size)
> > 
> > iam not a fan of such macros, they hurt readability alot for the little
> > gain of a few keypresses.
> 
> I'm generally not fond of such kind of macro either, but in this case,
> it avoid repeating "array" 4 times for each graw_array() call. With the
> typical length of the "array" name, without macro we get ugly overly
> long lines for each grow_array() call.
> So I really don't care about the number of keypresses. I only cared
> about keeping line length reasonable...
> 
> But anyway, I really have no strong feeling about this, so I will inline

> this macro. Do you want me to resend this whole patchset for review ?

no

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100820/aa8a2e8d/attachment.pgp>