[FFmpeg-devel] [PATCH 0/2] first steps to resolving float to int undefined behavior

[Michael Niedermayer]

On Sun, Nov 01, 2015 at 07:59:08PM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Sun, Nov 1, 2015 at 7:21 PM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> 
> > On Sun, Nov 1, 2015 at 7:15 PM, Michael Niedermayer
> > <michael at niedermayer.cc> wrote:
> > > On Sun, Nov 01, 2015 at 06:51:19PM -0500, Ganesh Ajjanagadde wrote:
> > >> On Sun, Nov 1, 2015 at 6:25 PM, Nicolas George <george at nsup.org> wrote:
> > >> > Le primidi 11 brumaire, an CCXXIV, Ronald S. Bultje a écrit :
> > >> >> So, is this a bug in llrint, or is this a failure to use llrint, or
> > is this
> > >> >> different from llrint? It sounds to me that llrint should be used,
> > not our
> > >> >> own alternative.
> > >> >
> > >> > Is there a sized version of the function? int64rint? Otherwise, these
> > >> > functions for the native types are as useless as the native types
> > >> > themselves.
> > >>
> > >> No, not in ISO C, or even libc AFAIK. ISO C tends to favor the
> > >> "non-sized" types in their signatures. The reason (I believe) stems
> > >> from the fact that an implementation is free to not even define the
> > >> sized types:
> > >> 7.20.1.1, point 3 - "These types are optional. However, if an
> > >> implementation provides integer types with widths of 8, 16, 32, or 64
> > >> bits, no padding bits, and (for the signed types) that have a two’s
> > >> complement representation [all platforms supported by the project], it
> > >> shall define the corresponding typedef names." -
> > >> Thus they want to limit their scope to mostly (or perharps only?)
> > stdint.h.
> > >>
> > >> Even otherwise, these functions are somewhat useless due to the
> > >> undefined behavior outside the range. All they really do is get
> > >> rounding done correctly according to the current FPU environment and
> > >> associated rounding modes, which can be manipulated in C99/C11.
> > >
> > > quite some of the undefined behavior also makes more optimizations
> > > possible for advanced compilers
> > > random silly example
> > >
> > > min = 0;
> > > for(i=0; i<N; i++) {
> > >     float c = whatever()
> > >     min = fmin(min, c);
> > >     out[i] = llrint(c);
> > > }
> > >
> > > here the compiler can remove any and all handling of NaN and +-Inf
> > > from fmin() because llrint(c) implies c is within the range
> > > represetable of the integer types
> > >
> > > with a llrint() equivalent that is defined for all cases this is not
> > > possible anymore
> >
> > Yes, which is why I have mentioned that we need to use a safe version
> > only when we need to guarantee safety, and are dealing with arbitrary
> > doubles. But such cases are there, such as the ffserver_config patch I
> > posted.
> 
> 
> OK, so let me sum up my current thoughts:
> - I don't really care about ffserver, so I have no opinion on whether the
> patch is appropriate. It may well be. Let's assume it should.
> - I think the name of this function is confusing. The av_clip* namespace
> seems reserved for int to int clips, so using it for a float to int
> conversion with clip is kind of weird. I would use a different namespace
> for it.
> - I'm not sure the code should live in this header. av_clip* is internal
> API (afaiu), and ffserver should not use internal api. I know it does, but
> it shouldn't, so adding new api specifically so ffserver can use it is ...
> all upside-down. Maybe the code should (for now) live in ffserver, or in an
> installed header, so it's ok for apps (like ffserver) to use it, if that's
> the intention.

common.h is a installed header
see /usr/include/libavutil/common.h


> 
> So, finally, maybe I'm being pedantic now, but it seems one of these usages
> overflows if we go over 2^64 = 16000 petabytes. Is that right? Do we really
> care? I mean, we're talking ffserver here, ffserver has much bigger issues
> than this.
> 
> Ronald
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151102/0d5a3b4a/attachment.sig>