[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