[FFmpeg-devel] [PATCH 1/3] lavu: add helper functions for integer lists.
Nicolas George
nicolas.george at normalesup.org
Thu Apr 11 14:21:09 CEST 2013
Le primidi 21 germinal, an CCXXI, Stefano Sabatini a écrit :
> Nit+++: no need to put them on separate lines.
I find alignment better.
> Nit: empty line between brief and first @param is customary and helps
> readability.
>
Ok.
> Docs for elsize are a bit ambiguous, and the list is confusing (only
> those sizes are accepted, also "size of the list elements" could be
> interpreted as "size of (the list elements)" rather than "size of each
> list element".
>
> I suggest:
> size in bytes of each list element
>
> and then specify if it only accepts a limited number of sizes (I still
> didn't read the code though, so I may be wrong)
Ok.
> Maybe av_int_list_get_length_for_size(), but feel free to discard this
> comment if you prefer the short form.
I find overlong names annoying.
> Nit: empty line before @param, here and below
Ok.
> * @param obj an AVClass object to set options on
> name of the binary option, or drop the (...) which may be read as "the
> name must be binary"
Ok.
> what is the correct type?
Updated, hopefully clearer. The point is that it is a macro, and sizeof will
be used.
> Don't you need to count the terminator as well?
If you know the length, you do not need a terminator, that is the whole
point.
> Optionally, you may add an example in opt-test.
I do not see how it can fit in the existing code, and adding this for just
that seems overkill.
Once the lavfi AVOptions frenzy is finished, I will try to remember to
update the API examples.
> Nice idea.
Thanks. Updated patch soon.
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/20130411/e60e67a9/attachment.asc>
More information about the ffmpeg-devel
mailing list