[FFmpeg-devel] [PATCH v2 1/3] libavutil/imgutils: add utility to get plane sizes

James Almer jamrial at gmail.com
Tue Jul 14 17:19:54 EEST 2020


On 7/14/2020 9:47 AM, Nicolas George wrote:
> James Almer (12020-07-10):
>> Because my opinion and tastes are not yours. I already said why *i*
>> consider it ugly. It doesn't need to fit *your* conception of ugliness.
> 
> If it is only a matter of taste, then it cannot count as an argument.
> But tastes are frequently a proxy for minor factors. If you can express
> the minor factors behind your tastes, they can count as arguments.
> 
> Which is what I am trying to do about my tastes.
> 
> One of these minor factors: there is frequently a "int ret" variable and
> a "return ret" at the end. If the return value conflates size with the
> error code, ret cannot be used, which means some kind of "if (size < 0)
> ret = size;" need to happen, but would easily be forgotten, initially or
> in refactoring.
> 
>> int av_image_fill_plane_sizes(size_t sizes[4], enum AVPixelFormat
>> pix_fmt, const ptrdiff_t linesizes[4]);
> 
> So the question is now: is the total size useful enough to warrant an
> extra return parameter or not? I have no advice on the question.

Not for an extra parameter (Who doesn't love enterprise-style code full
of function calls filled with NULL arguments, right?), but yes for using
the otherwise undefined >0 scope of the return value, which is only
possible if we keep the sizes as int like the rest of the module, and to
be honest something i would very much like to do seeing almost every
other existing function can't be ported to size_t/ptrdiff_t.
A complete, consistent set of new functions would have to be introduced
for that purpose.

I'd like to convince Michael about it, since he suggested these types,
but if i can't then this is the approach most consistent with existing
size array filling functions.

> 
> Regards,


More information about the ffmpeg-devel mailing list