[FFmpeg-devel] [PATCH 1/4] lavu: add simple array implementation
lukasz.m.luki at gmail.com
Thu Mar 6 23:03:09 CET 2014
On 06.03.2014 03:06, Michael Niedermayer wrote:
> On Wed, Mar 05, 2014 at 07:34:35PM +0100, Nicolas George wrote:
>> Le quintidi 15 ventôse, an CCXXII, wm4 a écrit :
>>> Unfortunately, this is pretty horrible. GET_UTF8 style.
>> That is your personal taste, it is not corroborated by any argument.
> compiler errors to such macros are cryptic and cost more time
> to fix. get the "code passed as argument" wrong and the compiler
> will likely have a hard time to figure out what is wrong, it really
> cant know if your macro or the code using it is at fault if the
> result is a syntax error
> This doesnt occur with functions
>> And the beauty of that kind of API is: if you don't like it, don't use it.
>>> It could be simplified a bit. First off, you don't need to put the code
>>> for "success" as macro parameter. You could call the macro something
>>> like AV_DYNARRAY_GROW, and put the "success" code after it.
>> That means the failure case has to be some kind a jump, or it needs an extra
>> condition. That seems to make the macro less usable for no apparent reason
>> except alleged "simplification".
>>> Second, you could drop the type for the size. You'll only ever use
>>> size_t or int as index, so it's better to duplicate the macro for those
>>> two types, instead of wasting 2 macro parameters on them.
>> Until someone is forced by some reason or another to use a variable of
>> another type, for example to interact with a library that uses long.
>> It is this kind of lack of foresight that leads to having two
>> implementations where there could be just one for the start.
> An API thats designed with
> "And the beauty of that kind of API is: if you don't like it, don't use it."
> will probably not improve that
> Its important that the developers like the API because if they end
> up not using it then no matter how great it is in theory it wont
> help them in practice
>> Also, a type parameter does not cost anything in a macro, there is just no
>> reason to omit it.
> printf(future extension, we might use this, 10234, 9999, 1234, yes,
> no, reserved, hello_world_string, goto error /* this cant fail */);
> i find the later more readable, that is needing less of my time.
So, to summarize, should I fix last commit (change return error code) or
replace it with macro?
If macro, I would really remove success part from it (referring to
Royale with Cheese.
More information about the ffmpeg-devel