[FFmpeg-devel] [PATCH 1/2] lavu: add av_bprintf and related.
nicolas.george at normalesup.org
Sun Feb 19 10:36:12 CET 2012
Le decadi 30 pluviôse, an CCXX, Clément Bœsch a écrit :
> I agree with moving this to a dedicated place.
Done: bprint.c and bprint.h were the obvious choice.
> I think "av_bprint_is_allocated()" is better here; the current name may
> suggest it returns the current internally allocated size.
> > + if (!av_bprint_complete(buf))
> > + return -1;
This is not a bug: the buffer may already be truncated because a previous
memory allocation has already failed. In that case, it will likely fail
again, but even if it could succeed, some data has already been lost.
There was a comment until I used a named function for the test; I put it
I also moved there the check for size vs. size_max, instead of having it
> > + new_str = av_realloc(old_str, new_size);
> > + if (!new_str)
> > + return -1;
The error is never forwarded, so it is a bit moot, but you are right, I
ought to keep good practices and set the example.
> Adding tests might be welcome; not only for testing, but as an API
> demonstration purpose.
I will think about it. I also need to learn how to add stuff to fate.
> What happens in case of error?
If this is a memory allocation failure, the buffer contents is truncated to
the available size, and only the length is updated. I added a length
explanation at the top of the header.
Other printf possible errors are silently ignored, as they are almost
exclusively the result of programming bugs and not environmental conditions.
> Any reason this function is inline while av_bprint_room() and
> av_bprint_allocated() are (unexported) macros?
Macros are tricky, for example they give strange error messages: I like to
avoid them in public headers, especially when their name looks like a
function. For internal code, I trust the programmer more.
> > + * Define a structure with extra padding to a fixed size.
> Maybe you should mention it is mainly for ABI compat purpose?
Done, and moved to bprint.h, as it is the only place it is of use for now.
> I still like this very much, and will be happy to make use of it in a few
I send a new patch shortly; the graphdump part is unchanged apart from the
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 198 bytes
Desc: Digital signature
More information about the ffmpeg-devel