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

[Nicolas George]

Le nonidi 19 ventôse, an CCXX, Stefano Sabatini a écrit :
> > +    add = FFMIN(add, UINT_MAX - 5 - buf->len);
> why 5?

I added a comment:
/* arbitrary margin to avoid small overflows */

> IIRC doxygen links function names ending with ().

Fixed.

> I have a bad feeling about "abusing" the size_max int for such
> purposes,

This is not really abusing: size_max = 1 makes really no sense at all by
itself, while size_max = 0 and size_max >= 2 make sense by themselves
(although 2 <= size_max < ~8 is rather absurd).

>	    maybe we should rather use a separate enum for specifying
> the appending mode.

What do you mean "appending mode"?

>		      This should also improve readability.

I am not sure: too long lines are also painful to read.

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

I hope it can be used as much everywhere as possible.

The API is intended to be very simple for the user. Can you tell me what
makes it look complicated? In particular, look at the examples in the test
code.

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

Fixed.

> Append char c n times to a print buffer

Fixed.

> Tests -> Test

Fixed.

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

Done; I thought the two lines below would be enough.

> Maybe you could return an error code here.

Done.

Another change: added av_bprint_finalize to the examples; makes Valgrind
happy.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120308/173ec03d/attachment.asc>