[FFmpeg-devel] [PATCH] lavu: add av_bprintf and related.
Stefano Sabatini
stefasab at gmail.com
Sun Mar 11 16:38:48 CET 2012
On date Thursday 2012-03-08 15:02:10 +0100, Nicolas George encoded:
>
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
> doc/APIchanges | 3 +
> libavutil/Makefile | 6 +-
> libavutil/avutil.h | 2 +-
> libavutil/bprint.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> libavutil/bprint.h | 125 +++++++++++++++++++++++++++++++++
> 5 files changed, 328 insertions(+), 3 deletions(-)
> create mode 100644 libavutil/bprint.c
> create mode 100644 libavutil/bprint.h
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index cd93495..fe7a73e 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,9 @@ libavutil: 2011-04-18
>
> API changes, most recent first:
>
> +2012-03-07 - xxxxxxx - lavu 51.43.100
> + Add bprint.h for bprint API.
> +
> 2012-02-21 - xxxxxxx - lavc 54.4.100
> Add av_get_pcm_codec() function.
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 23049f6..54f420e 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -10,6 +10,7 @@ HEADERS = adler32.h \
> avstring.h \
> avutil.h \
> base64.h \
> + bprint.h \
> bswap.h \
> common.h \
> cpu.h \
> @@ -47,6 +48,7 @@ OBJS = adler32.o \
> audioconvert.o \
> avstring.o \
> base64.o \
> + bprint.o \
> cpu.o \
> crc.o \
> des.o \
> @@ -82,8 +84,8 @@ OBJS-$(ARCH_PPC) += ppc/cpu.o
> OBJS-$(ARCH_X86) += x86/cpu.o
>
>
> -TESTPROGS = adler32 aes avstring base64 cpu crc des eval file fifo lfg lls \
> - md5 opt pca parseutils random_seed rational sha tree
> +TESTPROGS = adler32 aes avstring base64 bprint cpu crc des eval file fifo \
> + lfg lls md5 opt pca parseutils random_seed rational sha tree
> TESTPROGS-$(HAVE_LZO1X_999_COMPRESS) += lzo
>
> TOOLS = ffeval
> diff --git a/libavutil/avutil.h b/libavutil/avutil.h
> index 2ec2f2e..db3bc5c 100644
> --- a/libavutil/avutil.h
> +++ b/libavutil/avutil.h
> @@ -153,7 +153,7 @@
> */
>
> #define LIBAVUTIL_VERSION_MAJOR 51
> -#define LIBAVUTIL_VERSION_MINOR 42
> +#define LIBAVUTIL_VERSION_MINOR 43
> #define LIBAVUTIL_VERSION_MICRO 100
>
> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> diff --git a/libavutil/bprint.c b/libavutil/bprint.c
> new file mode 100644
> index 0000000..72e8297
> --- /dev/null
> +++ b/libavutil/bprint.c
> @@ -0,0 +1,195 @@
> +/*
> + * Copyright (c) 2012 Nicolas George
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <stdarg.h>
> +#include <stddef.h>
> +#include <stdio.h>
> +#include <string.h>
Nit+++: maybe you can remove some of these includes, not that I care much...
> +#include "bprint.h"
> +#include "common.h"
> +#include "error.h"
> +#include "mem.h"
> +
> +#define av_bprint_room(buf) ((buf)->size - FFMIN((buf)->len, (buf)->size))
> +#define av_bprint_is_allocated(buf) ((buf)->str != (buf)->reserved_internal_buffer)
> +
> +static int av_bprint_grow(AVBPrint *buf, unsigned new_size)
> +{
> + char *old_str, *new_str;
> +
> + if (buf->size == buf->size_max)
> + return AVERROR(EIO);
> + if (!av_bprint_is_complete(buf))
> + return AVERROR_INVALIDDATA; /* it is already truncated anyway */
> + if (!new_size)
> + new_size = buf->size > buf->size_max / 2 ? buf->size_max :
> + buf->size * 2;
> + old_str = av_bprint_is_allocated(buf) ? buf->str : NULL;
> + new_str = av_realloc(old_str, new_size);
> + if (!new_str)
> + return AVERROR(ENOMEM);
> + if (!old_str)
> + memcpy(new_str, buf->str, buf->len + 1);
> + buf->str = new_str;
> + buf->size = new_size;
> + return 0;
> +}
> +
> +void av_bprint_init(AVBPrint *buf, unsigned size_init, unsigned size_max)
> +{
> + unsigned asize = (char *)buf + sizeof(*buf) - buf->reserved_internal_buffer;
> +
> + if (size_max == 1)
> + size_max = asize;
Nit++: maybe asize -> a more descriptive name, such as available_size
or automatic_size or a feasible abbreviation.
[...]
> +int main(void)
> +{
> + AVBPrint b;
> +
> + av_bprint_init(&b, 0, -1);
> + bprint_pascal(&b, 5);
> + printf("Short text in unlimited buffer: %zu/%u\n", strlen(b.str), b.len);
> + printf("%s\n", b.str);
> + av_bprint_finalize(&b, NULL);
> +
> + av_bprint_init(&b, 0, -1);
> + bprint_pascal(&b, 25);
> + printf("Long text in unlimited buffer: %zu/%u\n", strlen(b.str), b.len);
> + av_bprint_finalize(&b, NULL);
You may add another example for max_size = 0:
av_bprint_init(&b, 0, 0);
bprint_pascal(&b, 25);
printf("Long text in empty buffer: %zu/%u\n", strlen(b.str), b.len);
av_bprint_finalize(&b, NULL);
> +
> + av_bprint_init(&b, 0, 2048);
> + bprint_pascal(&b, 25);
> + printf("Long text in limited buffer: %zu/%u\n", strlen(b.str), b.len);
> + av_bprint_finalize(&b, NULL);
> +
> + av_bprint_init(&b, 0, 1);
> + bprint_pascal(&b, 5);
> + printf("Short text in automatic buffer: %zu/%u\n", strlen(b.str), b.len);
> +
> + av_bprint_init(&b, 0, 1);
> + bprint_pascal(&b, 25);
> + printf("Long text in automatic buffer: %zu/%u\n", strlen(b.str), b.len);
> + /* Note that the size of the automatic buffer is arch-dependant. */
> +
> + return 0;
> +}
> +
> +#endif
> diff --git a/libavutil/bprint.h b/libavutil/bprint.h
> new file mode 100644
> index 0000000..eab98d4
> --- /dev/null
> +++ b/libavutil/bprint.h
[...]
> + * Append operations do not need to be tested for failure: if a memory
> + * allocation fails, data stop being appended to the buffer, but the length
> + * is still updated. This situation can be tested with
> + * av_bprint_is_complete().
> + *
> + * The size_max field determines several possible behaviours:
> + *
> + * size_max = -1 (= UINT_MAX) or any large value will let the buffer be
> + * reallocated as necessary, with an amortized linear cost.
> + *
> + * size_max = 0 prevents writing anything to the buffer: only the total
> + * length is computed. The write operations can then possibly be repeated in
> + * a buffer with exactly the necessary size
> + * (using size_init = size_max = len + 1).
> + *
> + * size_max = 1 is automatically replaced by the exact size available in the
> + * structure itself, thus ensuring no dynamic memory allocation. The
> + * internal buffer is large enough to hold a reasonable paragraph of text,
> + * such as the current paragraph.
> + */
> +typedef struct AVBPrint {
> + FF_PAD_STRUCTURE(1024,
> + char *str; /** string so far; or NULL if size == 0 */
> + unsigned len; /** length so far */
> + unsigned size; /** allocated memory */
> + unsigned size_max; /** maximum allocated memory */
> + char reserved_internal_buffer[1];
> + )
> +} AVBPrint;
> +
> +/**
> + * Init a print buffer.
> + *
> + * @param buf buffer to init
> + * @param size_init initial size (including the final 0)
> + * @param size_max maximum size;
> + * 0 means do not write anything, just count the length;
> + * 1 is replaced by the maximum value for automatic storage
> + */
> +void av_bprint_init(AVBPrint *buf, unsigned size_init, unsigned size_max);
Nit: init_size and max_size sound more correct to my
non-native-English-speacker ears.
Elaborating more on what I told the other time, using symbolic
constants may help readability:
av_bprint_init(&b, 0, -1) -> av_bprint_init(&b, 0, AV_BPRINT_UNLIMITED_SIZE)
av_bprint_init(&b, 0, 1) -> av_bprint_init(&b, 0, AV_BPRINT_AUTOMATIC_SIZE)
av_bprint_init(&b, 0, 0) -> av_bprint_init(&b, 0, AV_BPRINT_NULL_SIZE)
so people won't have to check the docs.
I agree that using a distinct parameter for the "buffering/appending
mode" may increase API complexity too much, so I'm not sure that was a
good idea.
LGTM otherwise.
--
FFmpeg = Fundamentalist and Fierce Mystic Portable EnGine
More information about the ffmpeg-devel
mailing list