[FFmpeg-devel] [PATCH 1/2] lavu: add av_bprintf and related.
Clément Bœsch
ubitux at gmail.com
Sat Feb 18 18:08:27 CET 2012
On Sat, Feb 18, 2012 at 03:56:48PM +0100, Nicolas George wrote:
>
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
> libavutil/avstring.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++
> libavutil/avstring.h | 60 ++++++++++++++++++++++++++++
> libavutil/common.h | 7 +++
> 3 files changed, 172 insertions(+), 0 deletions(-)
>
>
> This new version addresses Michael's concern that the structure size being
> part of the ABI may cause future compatibility problems in a way that I am a
> bit proud of:
>
> The structure is padded to a constant size (1k; arbitrary): the extra size
> is used as an initial buffer, thus avoiding completely dynamic memory
> allocation for small strings. If new fields are needed in the structure,
> they eat some of that padding, which leaves us plenty of room to manoeuvre.
>
> On the whole, this is a little bit hackish, but I believe it is worth it,
> because a lot of functions that build strings could benefit from a
> lightweight API like that: av_strerror, av_get_channel_layout_string, etc.
>
> (Hum, then maybe a lightweight separate bprint.[ch] would be better than
> appending to avstring.)
>
I agree with moving this to a dedicated place.
> Also, I may consider adding:
>
> void av_bprint_init_buf(AVBPrint *buf, char *mem, unsigned mem_size);
>
> to create an AVBPrint that writes directly in the provided buffer.
>
> Please comment.
>
> Regards,
>
> --
> Nicolas George
>
>
>
> diff --git a/libavutil/avstring.c b/libavutil/avstring.c
> index 76f6bb2..6cb3ddf 100644
> --- a/libavutil/avstring.c
> +++ b/libavutil/avstring.c
> @@ -210,6 +210,111 @@ int av_strncasecmp(const char *a, const char *b, size_t n)
> return c1 - c2;
> }
>
> +#define av_bprint_room(buf) ((buf)->size - FFMIN((buf)->len, (buf)->size))
> +#define av_bprint_allocated(buf) ((buf)->str != (buf)->reserved_internal_buffer)
I think "av_bprint_is_allocated()" is better here; the current name may
suggest it returns the current internally allocated size.
> +
> +static int av_bprint_grow(AVBPrint *buf, unsigned new_size)
> +{
> + char *old_str, *new_str;
> +
> + if (!av_bprint_complete(buf))
> + return -1;
AVERROR_BUG?
> + if (!new_size)
> + new_size = buf->size > buf->size_max / 2 ? buf->size_max :
> + buf->size * 2;
> + old_str = av_bprint_allocated(buf) ? buf->str : NULL;
> + new_str = av_realloc(old_str, new_size);
> + if (!new_str)
> + return -1;
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 || buf->size == buf->size_max)
> + break;
> + if (av_bprint_grow(buf, 0))
> + break;
> + }
> + add = FFMIN(add, UINT_MAX - 5 - buf->len);
> + 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 || buf->size == buf->size_max)
> + 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_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_allocated(buf))
> + av_freep(&buf->str);
> + }
> + buf->size = real_size;
> +}
> +
> #ifdef TEST
>
Adding tests might be welcome; not only for testing, but as an API
demonstration purpose.
> #undef printf
> diff --git a/libavutil/avstring.h b/libavutil/avstring.h
> index f73d6e7..258b394 100644
> --- a/libavutil/avstring.h
> +++ b/libavutil/avstring.h
> @@ -23,6 +23,7 @@
>
> #include <stddef.h>
> #include "attributes.h"
> +#include "common.h"
>
> /**
> * @addtogroup lavu_string
> @@ -203,6 +204,65 @@ int av_strcasecmp(const char *a, const char *b);
> int av_strncasecmp(const char *a, const char *b, size_t n);
>
> /**
> + * Buffer to print data progressively
> + * The string buffer grows as necessary and is always 0-terminated.
> + * The length of the string can go beyond the allocated size: the buffer is
> + * then truncated, but the functions still keep account of the required
> + * size.
> + * 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).
> + */
> +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);
> +
> +/**
> + * Append a formated string to a print buffer
> + */
What happens in case of error?
> +void av_bprintf(AVBPrint *buf, const char *fmt, ...) av_printf_format(2, 3);
> +
> +/**
> + * Append n times char c to a print buffer
> + */
ditto.
> +void av_bprint_chars(AVBPrint *buf, char c, unsigned n);
> +
> +/**
> + * Tests if the print buffer is complete
> + * It may have been truncated due to a memory allocation failure.
> + */
> +static inline int av_bprint_complete(AVBPrint *buf)
> +{
> + return buf->len < buf->size;
> +}
> +
Any reason this function is inline while av_bprint_room() and
av_bprint_allocated() are (unexported) macros?
> +/**
> + * 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);
> +
> +/**
> * @}
> */
>
> diff --git a/libavutil/common.h b/libavutil/common.h
> index 84290c6..d3308ef 100644
> --- a/libavutil/common.h
> +++ b/libavutil/common.h
> @@ -61,6 +61,13 @@
> #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0]))
> #define FFALIGN(x, a) (((x)+(a)-1)&~((a)-1))
>
> +/**
> + * Define a structure with extra padding to a fixed size.
> + */
Maybe you should mention it is mainly for ABI compat purpose?
> +#define FF_PAD_STRUCTURE(size, ...) \
> + __VA_ARGS__ \
> + char reserved_padding[size - sizeof(struct { __VA_ARGS__ })];
> +
> /* misc math functions */
> extern const uint8_t ff_log2_tab[256];
I still like this very much, and will be happy to make use of it in a few
places.
--
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/20120218/d4895b1d/attachment.asc>
More information about the ffmpeg-devel
mailing list