| Message ID | 20210606134757.18919-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avfilter/vf_ciescope: Handle black as very dark neutral gray | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On Sun, Jun 06, 2021 at 04:50:41PM +0300, Valerii Zapodovnikov wrote:
> I am sorry, what???
If that is intended as a review comment, please compose
a english sentance which represents what you are trying to convey.
That way others know what you mean.
The goal of review is to improve code
Anyway to elaborate on the rather trivial change
the code before the change has undefined behavior, that is a bug
in C per definition of what a bug is pretty much.
the change tried to replace the used value with one thats close
and not undefined.
we could skip black pixels or also use x=y=0 but later is
discontinous on all pathes with non negative color leading to black
so it felt a bit strange. But probably x=y=0 is a more strictly correct
choice, so ill change it to that.
thx
[...]
вс, 6 июн. 2021 г., 18:35 Michael Niedermayer <michael@niedermayer.cc>: > On Sun, Jun 06, 2021 at 04:50:41PM +0300, Valerii Zapodovnikov wrote: > > I am sorry, what??? > > If that is intended as a review comment, please compose > a english sentance which represents what you are trying to convey. > That way others know what you mean. > > The goal of review is to improve code > > Anyway to elaborate on the rather trivial change > the code before the change has undefined behavior, that is a bug > in C per definition of what a bug is pretty much. > the change tried to replace the used value with one thats close > and not undefined. > we could skip black pixels or also use x=y=0 but later is > discontinous on all pathes with non negative color leading to black > so it felt a bit strange. But probably x=y=0 is a more strictly correct > choice, so ill change it to that. > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Many that live deserve death. And some that die deserve life. Can you give > it to them? Then do not be too eager to deal out death in judgement. For > even the very wise cannot see all ends. -- Gandalf > _______________________________________________ > 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". > Nice, please change to that. And actually first link in google says https://stackoverflow.com/questions/42926763/ that it was defined in Annex F of ISO/IEC 9899 and gcc can do it, while clang cannot. I am not sure how one catches such NAN exceptions though. Moreover clang also wants that https://bugs.llvm.org/show_bug.cgi?id=19535#c13 and https://bugs.llvm.org/show_bug.cgi?id=17005, the last one even says and I quote: "The most visible issue caused by this is apparently that using -fsanitize=undefined on programs that make use of floating point division by 0.0 (which is perfectly legal udner IEEE floats) will print "runtime error: division by zero". (Also see: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=720935) It's also scary to think that your program might run slightly incorrectly in corner cases under clang, while it's correct under gcc (at least with -std=c99, gcc intentionally breaks floating point semantics with -std=gnu99)." Another bug in compiler, nothing to worry about IMHO. >
diff --git a/libavfilter/vf_ciescope.c b/libavfilter/vf_ciescope.c index b3b906f517..f8fe554c06 100644 --- a/libavfilter/vf_ciescope.c +++ b/libavfilter/vf_ciescope.c @@ -844,6 +844,9 @@ rgb_to_xy(double rc, { double sum; + if (rc == 0 && gc == 0 && bc == 0) + rc = gc = bc = 0.1/65536; + *x = m[0][0] * rc + m[0][1] * gc + m[0][2] * bc; *y = m[1][0] * rc + m[1][1] * gc + m[1][2] * bc; *z = m[2][0] * rc + m[2][1] * gc + m[2][2] * bc;
Fixes: floating point division by 0 Fixes: undefined behavior in handling NaN Fixes: Ticket 8268 Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavfilter/vf_ciescope.c | 3 +++ 1 file changed, 3 insertions(+)