[FFmpeg-devel] [PATCH] [libavutil] Add saturated add/sub operations for int64_t.

[Michael Niedermayer]

On Fri, May 01, 2020 at 01:13:58PM -0700, Dale Curtis wrote:
> On Fri, May 1, 2020 at 12:53 PM Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > On Thu, Apr 30, 2020 at 05:39:43PM -0700, Dale Curtis wrote:
> > > On Thu, Apr 30, 2020 at 5:21 PM James Almer <jamrial at gmail.com> wrote:
> > >
> > > > On 4/30/2020 7:19 PM, Dale Curtis wrote:
> > > > > Many places are using their own custom code for handling overflow
> > > > > around timestamps or other int64_t values. There are enough of these
> > > > > now that having some common saturated math functions seems sound.
> > > > >
> > > > > This adds implementations that just use the builtin functions for
> > > > > recent gcc, clang when available or implements its own version for
> > > > > older compilers.
> > > >
> > > > These look like 64 bit versions of av_sat_add32 and av_sat_sub32, from
> > > > common.h, so you should probably add them there and rename them
> > > > accordingly.
> > > >
> > > >
> > > Ah! I was looking for those, but missed them. Thanks. Done.
> >
> > one disadvantage the av_sat* functions have is the lack of inexact
> > detection
> >
> > In addition to av_sat*
> > In situations where its better to fail than to clip, something that
> > emulates what (+-Inf/)NaN is for float may make sense.
> > That would allow to simply check after a computation if any inexactness
> > occured
> >
> > Such a thing could be usefull in situations where a exact value or an
> > error is wanted.
> >
> >
> The __builtin functions provide exactly this API, we're just hiding it. I
> could add something like:
> int did_overflow = av_checked_sat_(add|sub)64(int64_t a, int64_t b,
> int64_t* result)

this could be done, but iam unsure this API is optimal

Maybe its best to show an example, why iam unsure about the API

lets consider a simple random expression
a*x + b*y

overflow  = av_checked_sat_mul64(a, x, &T0);
overflow |= av_checked_sat_mul64(b, y, &T1);
overflow |= av_checked_sat_add64(T0, T1, &result);
if (overflow)
    ...

vs.
 
int64_t result = av_add_eint64( av_mul_eint64(a, x),
                                av_mul_eint64(b, y) );
if (!av_is_finite_eint64(result))
    ....

To me the 2nd variant looks easier to read, (eint here is supposed to mean
extended integer, that is extended by +/- infinity and NaN with IEEE like semantics)

also the NaN element should have the same value as AV_NOPTS_VALUE, that would
likely be most usefull.
This could also allow the removial of alot of AV_NOPTS_VALUE special casing ...

But this is independant of the pure integer saturation API and should probably
not hold it up when that itself is needed. 

thx


> 
> |result| would still satuate and thus av_sat_(add|sub)64 could use it
> without checking the return value, but those which want to check and abort
> could do so. This is similar to the API shape we expose in Chromium modulo
> the fact we enforce an assert.
> 
> - dale
> _______________________________________________
> 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".

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200504/212e8a69/attachment.sig>