diff mbox

[FFmpeg-devel,v1] Bug #8027 - Wrong result for FFSIGN(0)

Message ID 8f249510-5072-c2b1-a996-a059264da80c@CoSoCo.de
State New
Headers show

Commit Message

Ulf Zibis July 17, 2019, 6:34 a.m. UTC
Again with the patch attached ...

Am 17.07.19 um 08:30 schrieb Ulf Zibis:
> Hi,
>
> I have a patch for bug #8027 <https://trac.ffmpeg.org/ticket/8027> (see
> attachment).
>
> But there is still a problem with -0.0, but FFABS(-0.0) works fine.
>
> Testcode:
>    av_log(NULL, AV_LOG_ERROR, "FFSIGN(0): %d\n", FFSIGN(0));
>     av_log(NULL, AV_LOG_ERROR, "FFSIGN(-0): %d\n", FFSIGN(-0));
>     av_log(NULL, AV_LOG_ERROR, "FFSIGN(0.0D): %d\n", FFSIGN(0.0D));
>     av_log(NULL, AV_LOG_ERROR, "FFSIGN(-0.0D): %d\n", FFSIGN(-0.0D));
>     av_log(NULL, AV_LOG_ERROR, "FFSIGN(-0.0F): %d\n", FFSIGN(-0.0F));
>     av_log(NULL, AV_LOG_ERROR, "FFSIGN(-0.0): %d\n", FFSIGN(-0.0));
>
>     av_log(NULL, AV_LOG_ERROR, "FFABS(0): %d\n", FFABS(0));
>     av_log(NULL, AV_LOG_ERROR, "FFABS(-0): %d\n", FFABS(-0));
>     av_log(NULL, AV_LOG_ERROR, "FFABS(0.0D): %f\n", FFABS(0.0D));
>     av_log(NULL, AV_LOG_ERROR, "FFABS(-0.0D): %f\n", FFABS(-0.0D));
>     av_log(NULL, AV_LOG_ERROR, "FFABS(-0.0F): %f\n", FFABS(-0.0F));
>     av_log(NULL, AV_LOG_ERROR, "FFABS(-0.0): %f\n", FFABS(-0.0));
>
> Results:
> FFSIGN(0): 1
> FFSIGN(-0): 1
> FFSIGN(0.0D): 1
> FFSIGN(-0.0D): 1
> FFSIGN(-0.0F): 1
> FFSIGN(-0.0): 1
> FFABS(0): 0
> FFABS(-0): 0
> FFABS(0.0D): 0.000000
> FFABS(-0.0D): -0.000000
> FFABS(-0.0F): -0.000000
> FFABS(-0.0): -0.000000
>
> -Ulf
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Comments

Ruiling Song July 17, 2019, 6:57 a.m. UTC | #1
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Ulf Zibis

> Sent: Wednesday, July 17, 2019 2:34 PM

> To: ffmpeg-devel@ffmpeg.org

> Subject: Re: [FFmpeg-devel] [PATCH v1] Bug #8027 - Wrong result for

> FFSIGN(0)

> 

> Again with the patch attached ...

> 

> Am 17.07.19 um 08:30 schrieb Ulf Zibis:

> > Hi,

> >

> > I have a patch for bug #8027 <https://trac.ffmpeg.org/ticket/8027> (see

> > attachment).

Why do you think FFSIGN(0.0) should return +1? What issue do you meet?
I think the value of FFSIGN(0) depends on how we define the behavior of FFSIGN().

Thanks!
Ruiling
> >

> > But there is still a problem with -0.0, but FFABS(-0.0) works fine.

> >

> > Testcode:

> >    av_log(NULL, AV_LOG_ERROR, "FFSIGN(0): %d\n", FFSIGN(0));

> >     av_log(NULL, AV_LOG_ERROR, "FFSIGN(-0): %d\n", FFSIGN(-0));

> >     av_log(NULL, AV_LOG_ERROR, "FFSIGN(0.0D): %d\n", FFSIGN(0.0D));

> >     av_log(NULL, AV_LOG_ERROR, "FFSIGN(-0.0D): %d\n", FFSIGN(-0.0D));

> >     av_log(NULL, AV_LOG_ERROR, "FFSIGN(-0.0F): %d\n", FFSIGN(-0.0F));

> >     av_log(NULL, AV_LOG_ERROR, "FFSIGN(-0.0): %d\n", FFSIGN(-0.0));

> >

> >     av_log(NULL, AV_LOG_ERROR, "FFABS(0): %d\n", FFABS(0));

> >     av_log(NULL, AV_LOG_ERROR, "FFABS(-0): %d\n", FFABS(-0));

> >     av_log(NULL, AV_LOG_ERROR, "FFABS(0.0D): %f\n", FFABS(0.0D));

> >     av_log(NULL, AV_LOG_ERROR, "FFABS(-0.0D): %f\n", FFABS(-0.0D));

> >     av_log(NULL, AV_LOG_ERROR, "FFABS(-0.0F): %f\n", FFABS(-0.0F));

> >     av_log(NULL, AV_LOG_ERROR, "FFABS(-0.0): %f\n", FFABS(-0.0));

> >

> > Results:

> > FFSIGN(0): 1

> > FFSIGN(-0): 1

> > FFSIGN(0.0D): 1

> > FFSIGN(-0.0D): 1

> > FFSIGN(-0.0F): 1

> > FFSIGN(-0.0): 1

> > FFABS(0): 0

> > FFABS(-0): 0

> > FFABS(0.0D): 0.000000

> > FFABS(-0.0D): -0.000000

> > FFABS(-0.0F): -0.000000

> > FFABS(-0.0): -0.000000

> >

> > -Ulf

> >

> > _______________________________________________

> > ffmpeg-devel mailing list

> > ffmpeg-devel@ffmpeg.org

> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

> >

> > To unsubscribe, visit link above, or email

> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Ulf Zibis July 17, 2019, 7:55 a.m. UTC | #2
Am 17.07.19 um 08:57 schrieb Song, Ruiling:
> Why do you think FFSIGN(0.0) should return +1?
Because anyone I ask including mathematicians, is the opinion that the
sign of 0 is positive.

> What issue do you meet?

I wanted to use the macro in my code, but with the current behaviour it
is worthless.

-Ulf
Paul B Mahol July 17, 2019, 8:11 a.m. UTC | #3
On 7/17/19, Ulf Zibis <Ulf.Zibis@cosoco.de> wrote:
>
> Am 17.07.19 um 08:57 schrieb Song, Ruiling:
>> Why do you think FFSIGN(0.0) should return +1?
> Because anyone I ask including mathematicians, is the opinion that the
> sign of 0 is positive.
>
>> What issue do you meet?
>
> I wanted to use the macro in my code, but with the current behaviour it
> is worthless.

Use copysign(f), see relevant manual page for documentation of this function.
Also, you always can add own macro, in local to your developed filter.

>
> -Ulf
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Hendrik Leppkes July 17, 2019, 8:12 a.m. UTC | #4
On Wed, Jul 17, 2019 at 9:55 AM Ulf Zibis <Ulf.Zibis@cosoco.de> wrote:
>
>
> Am 17.07.19 um 08:57 schrieb Song, Ruiling:
> > Why do you think FFSIGN(0.0) should return +1?
> Because anyone I ask including mathematicians, is the opinion that the
> sign of 0 is positive.

It really doesn't matter what anyone else thinks, the macro was
defined the way it  is, and changing it now has a chance of breaking
random places that rely on the current behavior.

>
> > What issue do you meet?
>
> I wanted to use the macro in my code, but with the current behaviour it
> is worthless.
>

Any macro starting with FF is technically internal to FFmpeg and
should not be relied upon by external software.
Furthermore, this macro is so extremely simple, you can just make your
own with no overhead or issues, if this one does not suit your needs.

- Hendrik
Nicolas George July 17, 2019, 5:21 p.m. UTC | #5
Ulf Zibis (12019-07-17):
> Because anyone I ask including mathematicians, is the opinion that the
> sign of 0 is positive.

They were not very competent mathematicians.

Regards,
Ulf Zibis July 17, 2019, 8:33 p.m. UTC | #6
Am 17.07.19 um 19:21 schrieb Nicolas George:
> Ulf Zibis (12019-07-17):
>> Because anyone I ask including mathematicians, is the opinion that the
>> sign of 0 is positive.
> They were not very competent mathematicians.
In normal arithmetic, yes, sgn(0) is 0, but here we are in floating
point arithmetics where sgn(0) is 1, see:
https://en.wikipedia.org/wiki/Floating-point_arithmetic#Signed_zero

But FFSIGN(0) results as -1, which is never correct.

-Ulf
Ulf Zibis July 18, 2019, 7:17 p.m. UTC | #7
Am 17.07.19 um 10:12 schrieb Hendrik Leppkes:
> ..., the macro was defined the way it is, and changing it now has a
> chance of breaking
> random places that rely on the current behavior.

OK, that would be risky.

Maybe we could add another macro FFSGN. Then maintainers of individual
code parts could check the usage over the time.

#define FFSGN(a) ((a) > 0 ? 1 : (a) < 0 ? -1 : 0)

-Ulf
Nicolas George July 18, 2019, 7:22 p.m. UTC | #8
Ulf Zibis (12019-07-18):
> Maybe we could add another macro FFSGN. Then maintainers of individual
> code parts could check the usage over the time.

Or maybe write robust code that does not need that kind of hair
splitting.

Regards,
Hendrik Leppkes July 18, 2019, 9:44 p.m. UTC | #9
On Thu, Jul 18, 2019 at 9:17 PM Ulf Zibis <Ulf.Zibis@cosoco.de> wrote:
>
>
> Am 17.07.19 um 10:12 schrieb Hendrik Leppkes:
> > ..., the macro was defined the way it is, and changing it now has a
> > chance of breaking
> > random places that rely on the current behavior.
>
> OK, that would be risky.
>
> Maybe we could add another macro FFSGN. Then maintainers of individual
> code parts could check the usage over the time.
>
> #define FFSGN(a) ((a) > 0 ? 1 : (a) < 0 ? -1 : 0)
>

Unless something inside FFmpeg needs such a behavior, that is unlikely.

- Hendrik
diff mbox

Patch

From aa27ca089dafb3bc2f2c5aaa171856b235989ea4 Mon Sep 17 00:00:00 2001
From: Ulf Zibis <Ulf.Zibis@CoSoCo.de>
Date: 16.07.2019, 23:36:51

avutil/common.h: Correct result for FFSIGN(0.0)

diff --git a/libavutil/common.h b/libavutil/common.h
index 8db0291..4b14e7b 100644
--- a/libavutil/common.h
+++ b/libavutil/common.h
@@ -70,7 +70,7 @@ 
  * @see FFNABS()
  */
 #define FFABS(a) ((a) >= 0 ? (a) : (-(a)))
-#define FFSIGN(a) ((a) > 0 ? 1 : -1)
+#define FFSIGN(a) ((a) >= 0 ? 1 : -1)
 
 /**
  * Negative Absolute value.