| Message ID | 20241108012049.63632-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] fate/pixfmt: disable dithering in the scale filter | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Thu, 7 Nov 2024, James Almer wrote: > Should fix fate failures across different systems. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > This only hides the bug in the dither path for unscaled planar yuv 8bit to > hbd and vice-versa. > Someone more familiar with the code should check what exactly is going on. LGTM, thanks! Good job in hunting this one down - it seems odd as the exact output value seems stable on each machine (and valgrind doesn't find any nondeterminism), but the output is different on most machines. // Martin
On 11/8/2024 5:15 AM, Martin Storsjö wrote: > On Thu, 7 Nov 2024, James Almer wrote: > >> Should fix fate failures across different systems. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> This only hides the bug in the dither path for unscaled planar yuv >> 8bit to >> hbd and vice-versa. >> Someone more familiar with the code should check what exactly is going >> on. > > LGTM, thanks! Good job in hunting this one down - it seems odd as the > exact output value seems stable on each machine (and valgrind doesn't > find any nondeterminism), but the output is different on most machines. The only tests that fail are those using the DITHER_COPY method in planarCopyWrapper() when dither is used, and the only difference compared to the no dither path is accessing the "dithers" static const array defined in the same file. Maybe compilers are doing something weird when aligning it, or accessing it?
On Fri, Nov 08, 2024 at 04:56:38PM -0300, James Almer wrote: > On 11/8/2024 5:15 AM, Martin Storsjö wrote: > > On Thu, 7 Nov 2024, James Almer wrote: > > > > > Should fix fate failures across different systems. > > > > > > Signed-off-by: James Almer <jamrial@gmail.com> > > > --- > > > This only hides the bug in the dither path for unscaled planar yuv > > > 8bit to > > > hbd and vice-versa. > > > Someone more familiar with the code should check what exactly is > > > going on. > > > > LGTM, thanks! Good job in hunting this one down - it seems odd as the > > exact output value seems stable on each machine (and valgrind doesn't > > find any nondeterminism), but the output is different on most machines. > > The only tests that fail are those using the DITHER_COPY method in > planarCopyWrapper() when dither is used, and the only difference compared to > the no dither path is accessing the "dithers" static const array defined in > the same file. > Maybe compilers are doing something weird when aligning it, or accessing it? Not sure i fully understand but the brute force way to debug this is maybe just add a printf() to dump every value then diff between to see where differences start. thx [...]
diff --git a/tests/fate-run.sh b/tests/fate-run.sh index ef9c7ef064..c61a4dc992 100755 --- a/tests/fate-run.sh +++ b/tests/fate-run.sh @@ -513,7 +513,7 @@ pixfmt_conversion_ext(){ outdir="tests/data/pixfmt" file=${outdir}/${color_range}-${conversion}.yuv cleanfiles="$cleanfiles $file" - do_avconv $file $DEC_OPTS -lavfi ${prefix}testsrc=s=352x288,format=${color_range},scale=flags=$SCALE_FLAGS,format=$conversion \ + do_avconv $file $DEC_OPTS -lavfi ${prefix}testsrc=s=352x288,format=${color_range},scale=flags=$SCALE_FLAGS:sws_dither=none,format=$conversion \ $ENC_OPTS -t 1 -f rawvideo -s 352x288 -pix_fmt ${color_range}${suffix} -color_range mpeg } diff --git a/tests/ref/pixfmt/yuv444p10-yuv444p b/tests/ref/pixfmt/yuv444p10-yuv444p index c39314523a..44cd8a40a6 100644 --- a/tests/ref/pixfmt/yuv444p10-yuv444p +++ b/tests/ref/pixfmt/yuv444p10-yuv444p @@ -1,2 +1,2 @@ -67d5749f3b40b343008f9a62db1b2abd *tests/data/pixfmt/yuv444p10-yuv444p.yuv +1b783c5b90f43945b91b0b5be254aded *tests/data/pixfmt/yuv444p10-yuv444p.yuv 15206400 tests/data/pixfmt/yuv444p10-yuv444p.yuv diff --git a/tests/ref/pixfmt/yuv444p12-yuv444p b/tests/ref/pixfmt/yuv444p12-yuv444p index a59328f3c0..fc5e8e53ad 100644 --- a/tests/ref/pixfmt/yuv444p12-yuv444p +++ b/tests/ref/pixfmt/yuv444p12-yuv444p @@ -1,2 +1,2 @@ -1ece60e7894b113b09c86575c66a2ec8 *tests/data/pixfmt/yuv444p12-yuv444p.yuv +9339fb7fd327f4131639c3b718eeccce *tests/data/pixfmt/yuv444p12-yuv444p.yuv 15206400 tests/data/pixfmt/yuv444p12-yuv444p.yuv diff --git a/tests/ref/pixfmt/yuv444p12-yuv444p10be b/tests/ref/pixfmt/yuv444p12-yuv444p10be index 90740e0467..9785443799 100644 --- a/tests/ref/pixfmt/yuv444p12-yuv444p10be +++ b/tests/ref/pixfmt/yuv444p12-yuv444p10be @@ -1,2 +1,2 @@ -284113a13f5c2378e87ff5e53aff23cb *tests/data/pixfmt/yuv444p12-yuv444p10be.yuv +6f9fb7022d197765457c7a39645d8a3f *tests/data/pixfmt/yuv444p12-yuv444p10be.yuv 15206400 tests/data/pixfmt/yuv444p12-yuv444p10be.yuv diff --git a/tests/ref/pixfmt/yuv444p12-yuv444p10le b/tests/ref/pixfmt/yuv444p12-yuv444p10le index 317094ed20..829a350417 100644 --- a/tests/ref/pixfmt/yuv444p12-yuv444p10le +++ b/tests/ref/pixfmt/yuv444p12-yuv444p10le @@ -1,2 +1,2 @@ -284113a13f5c2378e87ff5e53aff23cb *tests/data/pixfmt/yuv444p12-yuv444p10le.yuv +6f9fb7022d197765457c7a39645d8a3f *tests/data/pixfmt/yuv444p12-yuv444p10le.yuv 15206400 tests/data/pixfmt/yuv444p12-yuv444p10le.yuv
Should fix fate failures across different systems. Signed-off-by: James Almer <jamrial@gmail.com> --- This only hides the bug in the dither path for unscaled planar yuv 8bit to hbd and vice-versa. Someone more familiar with the code should check what exactly is going on. tests/fate-run.sh | 2 +- tests/ref/pixfmt/yuv444p10-yuv444p | 2 +- tests/ref/pixfmt/yuv444p12-yuv444p | 2 +- tests/ref/pixfmt/yuv444p12-yuv444p10be | 2 +- tests/ref/pixfmt/yuv444p12-yuv444p10le | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-)