[FFmpeg-devel] [PATCH] provide AV_BASE64_SIZE() macro

Måns Rullgård mans
Fri Jun 4 00:57:59 CEST 2010


Michael Niedermayer <michaelni at gmx.at> writes:

> On Thu, Jun 03, 2010 at 11:00:49PM +0100, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>> 
>> > On Thu, Jun 03, 2010 at 10:39:52PM +0100, M?ns Rullg?rd wrote:
>> >> Michael Niedermayer <michaelni at gmx.at> writes:
>> >> 
>> >> > On Thu, Jun 03, 2010 at 01:50:06PM -0700, Howard Chu wrote:
>> >> >> Just for convenience...
>> >> >
>> >> > IMHO this is making the code harder to understand
>> >> 
>> >> How so?  It's an obscure calculation repeated multiple times.  That is
>> >> exactly the kind of situation functions and macros were invented for.
>> >
>> > to me the calculation is clear,
>> 
>> Because you happen to know exactly how base64 encoding works.  This is
>> IMO not a reasonable requirement to place on users of the interface.
>
> its documented though, and i think its reasonable to expect users
> to read the dox of what they use.

By extension, one could argue that the base64 encoding is documented,
wherefore we don't need to provide a function for it.

I think a macro like this is good for the same reasons we have FFMAX
and similar macros.

>> Besides, the same rather complex calculation repeated numerous times
>> is a recipe for trouble.  Sooner or later, someone will get it wrong
>> in one place.  Then people will copy it from there, and the error will
>> spread.  It's much better to put it all in a macro.
>
> i dont really see this as a big issue but if people prefer then iam not
> against the macro

People have a remarkable knack for seeking out and copying the most
broken example in the largest of code bases.  I'd rather not take that
risk when it can be so easily avoided.

-- 
M?ns Rullg?rd
mans at mansr.com



More information about the ffmpeg-devel mailing list