On 7/14/2020 5:49 PM, Michael Niedermayer wrote: > 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
The issue is that most existing functions can't be ported to ptrdiff_t/size_t with a mere type switch after a soname bump, which means to make such a move we'd need to introduce an entire set of new imgutils functions with a different design approach for this purpose. This is why i consider it may be a better idea to define this new single function in a consistent way with the existing API, including types and signature, which is what the first iteration of this set attempted to do, and leave the move to proper types for a rewrite of the module. If not, the v3 posted yesterday is currently the best non-int approach. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
