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

[Michael Niedermayer]

On Tue, Jul 14, 2020 at 11:19:54AM -0300, James Almer wrote:
> 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.

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

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Whats the most studid thing your enemy could do ? Blow himself up
Whats the most studid thing you could do ? Give up your rights and
freedom because your enemy blew himself up.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200714/6c3d8e11/attachment.sig>