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

[Nicolas George]

Le duodi 22 ventôse, an CCXX, Stefano Sabatini a écrit :
> Nit+++: maybe you can remove some of these includes, not that I care much...

Removed stddef.

> Nit++: maybe asize -> a more descriptive name, such as available_size
> or automatic_size or a feasible abbreviation.

size_auto seems fine.

> You may add another example for max_size = 0:
> 
>    av_bprint_init(&b, 0, 0);
>    bprint_pascal(&b, 25);
>    printf("Long text in empty buffer: %zu/%u\n", strlen(b.str), b.len);
>    av_bprint_finalize(&b, NULL);

Done, but finalize if not necessary.

> > +void av_bprint_init(AVBPrint *buf, unsigned size_init, unsigned size_max);
> Nit: init_size and max_size sound more correct to my
> non-native-English-speacker ears.

They are, but I want to see size_ as a kind of namespace prefix.

> Elaborating more on what I told the other time, using symbolic
> constants may help readability:
> 
> av_bprint_init(&b, 0, -1) -> av_bprint_init(&b, 0, AV_BPRINT_UNLIMITED_SIZE)
> av_bprint_init(&b, 0,  1) -> av_bprint_init(&b, 0, AV_BPRINT_AUTOMATIC_SIZE)
> av_bprint_init(&b, 0,  0) -> av_bprint_init(&b, 0, AV_BPRINT_NULL_SIZE)
> 
> so people won't have to check the docs.
> 
> I agree that using a distinct parameter for the "buffering/appending
> mode" may increase API complexity too much, so I'm not sure that was a
> good idea.

I added them as convenience macros, but left the numeric values in the
example.

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/20120311/cfe5fef6/attachment.asc>