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

James Almer jamrial at gmail.com
Tue Jul 21 03:21:11 EEST 2020


On 7/20/2020 6:32 PM, Brian Kim wrote:
> Just wanted to check if there was any consensus on what we wanted to do
> with these changes. Are we holding off until a future module wide change?

No, i'll push v3 soon if my argumentation below was not enough to
convince Nicolas or Michael. My intention is to use ints for the new
function, not to postpone committing it in any form indefinitely.

> 
> On Wed, Jul 15, 2020 at 8:09 AM James Almer <jamrial at gmail.com> wrote:
> 
>> 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,
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 



More information about the ffmpeg-devel mailing list