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

[James Almer]

On 7/15/2020 4:06 AM, Nicolas George wrote:
> Michael Niedermayer (12020-07-14):
>> Let me phrase my suggestion in a more high level sense
>>
>> Currently the functions use int for lots of cases where they should not
>> ultimately we want the functions to use more correct types for linesize
>> sizes and so on.
>>
>> We need to add these function(s) because we fix a bug.
>> Can we take a step toward more correct types while doing this in a
>> way that makes sense ?
>>
>> if so that should be done.
>>
>> If not (as in its more mess than if its done later in some other way)
>> then we should not try to do this now
>>
>> The idea is just to take a step towards how these functions/API should look
>> ideally. Its not to make some inconvenient mess
> 
> I strongly agree.
> 
> If we use ints now, then the next time the same question comes this
> function will be one more argument for using ints again. And the next
> time, and the next time, all this making that much work for when we
> cannot wait any longer to make the change, instead of that much less.

I don't share the sentiment, since as i said an entire new set of
functions will have to be added for a module-wide type switch. The way
this function is designed here will not increase or reduce any future
work in any way whatsoever. It's meant to be a solution today for a
problem in the current state of the imgutils module.

For all we know, by the time ints start being a real issue or the need
to replace the current functions arise, the new set of functions might
have to be designed in a way this one wont be reusable. For example,
will AVPixelFormat have been replaced by then with an alternative that
removes the need to have 20 entries to cover all bitdepths, chroma
variants, endianness, and plane presence/order, for what's ultimately a
single format?

Sure, at first glance using the proper types here will seem like making
a step forward, but we may instead be getting ahead of ourselves for no
real gain. av_image_check_size() is truly future proof, as is
av_image_check_sar(), but most others aren't, and that includes this one
regardless of the data type we choose.

> 
> Consistency is not a end in itself, it is a means to an end. And it is
> one of the weakest arguments. If there are no other reason for doing things
> one way or another, then sure, by all means let us choose the way that
> looks the same at the rest of the code. But if there is a reason, if one
> way is more efficient, or more convenient, or more future proof, or more
> compatible, etc., then we choose this way, and too bad for consistency.
> 
> Regards,