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

Stefano Sabatini stefasab at gmail.com
Thu Mar 8 13:01:27 CET 2012


On date Wednesday 2012-03-07 16:40:47 +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 |  187 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libavutil/bprint.h |  120 +++++++++++++++++++++++++++++++++
>  5 files changed, 315 insertions(+), 3 deletions(-)
>  create mode 100644 libavutil/bprint.c
>  create mode 100644 libavutil/bprint.h
> 
> 
> No change in the code since my last proposal. Added some tests/examples as
> requested by Clément.
> 
> If there are no further remarks, I will try to apply before the end of the
> week.
> 
[...]
> +#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)
> +
> +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;
> +    buf->str      = buf->reserved_internal_buffer;
> +    buf->len      = 0;
> +    buf->size     = FFMIN(asize, size_max);
> +    buf->size_max = size_max;
> +    *buf->str = 0;
> +    if (size_init > buf->size)
> +        av_bprint_grow(buf, size_init);
> +}
> +
> +void av_bprintf(AVBPrint *buf, const char *fmt, ...)
> +{
> +    unsigned room;
> +    char *dst;
> +    va_list vl;
> +    int add;
> +
> +    while (1) {
> +        room = av_bprint_room(buf);
> +        dst = room ? buf->str + buf->len : NULL;
> +        va_start(vl, fmt);
> +        add = vsnprintf(dst, room, fmt, vl);
> +        va_end(vl);
> +        if (add <= 0)
> +            return;
> +        if (add < room)
> +            break;
> +        if (av_bprint_grow(buf, 0))
> +            break;
> +    }

> +    add = FFMIN(add, UINT_MAX - 5 - buf->len);

why 5?

> +    buf->len += add;
> +}
> +
> +void av_bprint_chars(AVBPrint *buf, char c, unsigned n)
> +{
> +    unsigned room, real_n;
> +
> +    while (1) {
> +        room = av_bprint_room(buf);
> +        if (n < room)
> +            break;
> +        if (av_bprint_grow(buf, 0))
> +            break;
> +    }
> +    if (room) {
> +        real_n = FFMIN(n, room - 1);
> +        memset(buf->str + buf->len, c, real_n);
> +        buf->str[buf->len + real_n] = 0;
> +    }
> +    buf->len += FFMIN(n, UINT_MAX - 5 - buf->len);
> +}
> +
> +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;
> +}
> +
> +
> +#ifdef TEST
> +
> +#undef printf
> +
> +static void bprint_pascal(AVBPrint *b, unsigned size)
> +{
> +    unsigned p[size + 1], i, j;
> +
> +    p[0] = 1;
> +    av_bprintf(b, "%8d\n", 1);
> +    for (i = 1; i <= size; i++) {
> +        p[i] = 1;
> +        for (j = i - 1; j > 0; j--)
> +            p[j] = p[j] + p[j - 1];
> +        for (j = 0; j <= i; j++)
> +            av_bprintf(b, "%8d", p[j]);
> +        av_bprintf(b, "\n");
> +    }
> +}
> +
> +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_init(&b, 0, -1);
> +    bprint_pascal(&b, 25);
> +    printf("Long text in unlimited buffer: %zu/%u\n", strlen(b.str), b.len);
> +
> +    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_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..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__ })];
> +
> +/**
> + * 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.

IIRC doxygen links function names ending with ().

> + *
> + * 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.

I have a bad feeling about "abusing" the size_max int for such
purposes, maybe we should rather use a separate enum for specifying
the appending mode. This should also improve readability.

Overall the API seems useful (e.g. I need something like that for
escaped strings in ffprobe.c), even if it seems quite complex.

> + */
> +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

Nit+: finish sentences with a trailing dot, an empty line before the
params helps readability, here and below.

> + * @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);
> +
> +/**
> + * Append a formated string to a print buffer
> + */
> +void av_bprintf(AVBPrint *buf, const char *fmt, ...) av_printf_format(2, 3);
> +

> +/**
> + * Append n times char c to a print buffer

Append char c n times to a print buffer

(English syntax: TRANSITIVE_VERB OBJECT ...)

> + */
> +void av_bprint_chars(AVBPrint *buf, char c, unsigned n);
> +
> +/**
> + * Tests if the print buffer is complete

Tests -> Test

Please add a short description of what "complete" means in this
context.

> + * It may have been truncated due to a memory allocation failure
> + * or the size_max limit (compare size and size_max if necessary).
> + */
> +static inline int av_bprint_is_complete(AVBPrint *buf)
> +{
> +    return buf->len < buf->size;
> +}
> +
> +/**
> + * Finalize a print buffer
> + * The print buffer can no longer be used afterwards,
> + * but the len and size fields are still valid.
> + * @arg[out] ret_str  if not NULL, used to return a permanent copy of the
> + *                    buffer contents, or NULL if memory allocation fails;
> + *                    if NULL, the buffer is discarded and freed
> + */
> +void av_bprint_finalize(AVBPrint *buf, char **ret_str);

Maybe you could return an error code here.

[...]
-- 
FFmpeg = Fundamentalist & Forgiving Magic Pure Ecstatic God


More information about the ffmpeg-devel mailing list