[FFmpeg-devel] [PATCH] lavu: add av_bprintf and related.

Clément Bœsch ubitux at gmail.com
Wed Feb 22 19:18:04 CET 2012


On Sun, Feb 19, 2012 at 10:40:17AM +0100, Nicolas George wrote:
> 
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
>  libavutil/Makefile |    2 +
>  libavutil/bprint.c |  136 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libavutil/bprint.h |  120 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 258 insertions(+), 0 deletions(-)
>  create mode 100644 libavutil/bprint.c
>  create mode 100644 libavutil/bprint.h
> 
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 6d78bd5..ac3eed0 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                                                            \
> diff --git a/libavutil/bprint.c b/libavutil/bprint.c
> new file mode 100644
> index 0000000..88e2a2e
> --- /dev/null
> +++ b/libavutil/bprint.c
> @@ -0,0 +1,136 @@
> +/*
> + * 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>
> +#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)
> +

I don't really like the av_ prefix since it is not exported.

[...]
> +void av_bprint_finalize(AVBPrint *buf, char **ret_str)
> +{
> +    unsigned real_size = FFMIN(buf->len + 1, buf->size);
> +    char *str;
> +
> +    if (ret_str) {
> +        if (av_bprint_is_allocated(buf)) {
> +            str = av_realloc(buf->str, real_size);
> +            if (!str)
> +                str = buf->str;
> +            buf->str = NULL;
> +        } else {
> +            str = av_malloc(real_size);
> +            if (str)
> +                memcpy(str, buf->str, real_size);
> +        }
> +        *ret_str = str;
> +    } else {
> +        if (av_bprint_is_allocated(buf))
> +            av_freep(&buf->str);
> +    }
> +    buf->size = real_size;
> +}
> +

git complains about this trailing \n

> diff --git a/libavutil/bprint.h b/libavutil/bprint.h
> new file mode 100644
> index 0000000..7303435
> --- /dev/null
> +++ b/libavutil/bprint.h
> @@ -0,0 +1,120 @@
> +/*
> + * 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
> + */
> +
> +#ifndef AVUTIL_BPRINT_H
> +#define AVUTIL_BPRINT_H
> +
> +#include "attributes.h"
> +
> +/**
> + * Define a structure with extra padding to a fixed size
> + * This helps ensuring binary compatibility with future versions.
> + */
> +#define FF_PAD_STRUCTURE(size, ...) \
> +    __VA_ARGS__ \
> +    char reserved_padding[size - sizeof(struct { __VA_ARGS__ })];
> +

Is the __VA_ARGS__ really needed?

> +/**
> + * Buffer to print data progressively
> + *
> + * The string buffer grows as necessary and is always 0-terminated.
> + * The content of the string is never accessed, and thus is
> + * encoding-agnostic and can even hold binary data.
> + *
> + * Small buffers are kept in the structure itself, and thus require no
> + * memory allocation at all (unless the contents of the buffer is needed
> + * after the structure goes out of scope). This is almost as lightweight as
> + * declaring a local "char buf[512]".
> + *
> + * The length of the string can go beyond the allocated size: the buffer is
> + * then truncated, but the functions still keep account of the actual total
> + * length.
> + *
> + * 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.
> + */

Thank you for the long explanation (though, examples/tests are nice :-°).

Anyway, it looks good enough to be applied if tested, so no more comment
from me.

[...]

Note: don't forget to update APIChanges and bump

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120222/042a684e/attachment.asc>


More information about the ffmpeg-devel mailing list