diff mbox series

[FFmpeg-devel,RFC,2/2] fate: Make it possible to have alternative reference files

Message ID dfd303a8d3ec0976e0abdf6cede1f2dd27b10a05.1729252615.git.eclipse7@gmx.net
State New
Headers show
Series Make fate tests succeed with zlib-ng | expand

Commit Message

Alexander Strasser Oct. 18, 2024, 12:03 p.m. UTC 
Sometimes deps (external from FFmpeg) can cause different results
either because of bugs or because of drop in replacements.

This feature of alternate reference files should only be used
where absolutely necessary because other solutions are not
feasible in practice. Maintaining two reference files is a
burden and updating one or the other can easily be forgotten.

We currently have this problem with zlib-ng. On systems where
the zlib-ng is used the results are different.

In commit bce5855afb25d318e090c2e6c16117f065458356 we avoided the
problem by using compression level 0. That fixed the problem, but
introduced a problem with original zlib. This caused differences
with 2 fate tests depending on the zlib version used.
See zlib commit 8ba393e70d984d902b15b9e6876f4d0d38ae4be8 .

After re-applying bce5855afb25d318e090c2e6c16117f065458356 this
patch fixes the 2 mentioned cases for older zlib versions by
allowing an alternative reference file. The files accommodate
for the bug in the older version.
---
 tests/fate-run.sh                  | 10 +++++++
 tests/ref/fate/mov-cover-image.alt | 42 ++++++++++++++++++++++++++++++
 tests/ref/fate/png-mdcv.alt        | 22 ++++++++++++++++
 3 files changed, 74 insertions(+)
 create mode 100644 tests/ref/fate/mov-cover-image.alt
 create mode 100644 tests/ref/fate/png-mdcv.alt

--

Comments

Hendrik Leppkes Oct. 18, 2024, 12:41 p.m. UTC | #1 
On Fri, Oct 18, 2024 at 2:09 PM Alexander Strasser via ffmpeg-devel
<ffmpeg-devel@ffmpeg.org> wrote:
> This caused differences
> with 2 fate tests depending on the zlib version used.

2 fate tests?
https://fate.ffmpeg.org/report.cgi?time=20241016173824&slot=x86_32-msvc14-dll-md-windows-native

This entire approach feels extremely brittle for the future.

- Hendrik
Lynne Oct. 18, 2024, 2:36 p.m. UTC | #2 
On 18/10/2024 14:41, Hendrik Leppkes wrote:
> On Fri, Oct 18, 2024 at 2:09 PM Alexander Strasser via ffmpeg-devel
> <ffmpeg-devel@ffmpeg.org> wrote:
>> This caused differences
>> with 2 fate tests depending on the zlib version used.
> 
> 2 fate tests?
> https://fate.ffmpeg.org/report.cgi?time=20241016173824&slot=x86_32-msvc14-dll-md-windows-native
> 
> This entire approach feels extremely brittle for the future.
> 
> - Hendrik

Yeah, definitely.
These days, other libraries like miniz are getting used as well, so 
hardcoding each case separately is no better than what we have currently.
Alexander Strasser Oct. 18, 2024, 2:58 p.m. UTC | #3 
On 2024-10-18 16:36 +0200, Lynne via ffmpeg-devel wrote:
> On 18/10/2024 14:41, Hendrik Leppkes wrote:
> > On Fri, Oct 18, 2024 at 2:09 PM Alexander Strasser via ffmpeg-devel
> > <ffmpeg-devel@ffmpeg.org> wrote:
> > > This caused differences
> > > with 2 fate tests depending on the zlib version used.
> >
> > 2 fate tests?
> > https://fate.ffmpeg.org/report.cgi?time=20241016173824&slot=x86_32-msvc14-dll-md-windows-native
> >
> > This entire approach feels extremely brittle for the future.
> >
> > - Hendrik
>
> Yeah, definitely.
> These days, other libraries like miniz are getting used as well, so
> hardcoding each case separately is no better than what we have currently.

Hmm not sure about the build from Hendrik. This was with a build with
zlib and compression level 0?

What I tried to do and describe is to account for a bug in older zlib
versions. I didn't want to have alternative refs for every implementation.
I actually oppose to that!


  Alexander
Hendrik Leppkes Oct. 18, 2024, 4:32 p.m. UTC | #4 
On Fri, Oct 18, 2024 at 5:09 PM Alexander Strasser via ffmpeg-devel
<ffmpeg-devel@ffmpeg.org> wrote:
>
> On 2024-10-18 16:36 +0200, Lynne via ffmpeg-devel wrote:
> > On 18/10/2024 14:41, Hendrik Leppkes wrote:
> > > On Fri, Oct 18, 2024 at 2:09 PM Alexander Strasser via ffmpeg-devel
> > > <ffmpeg-devel@ffmpeg.org> wrote:
> > > > This caused differences
> > > > with 2 fate tests depending on the zlib version used.
> > >
> > > 2 fate tests?
> > > https://fate.ffmpeg.org/report.cgi?time=20241016173824&slot=x86_32-msvc14-dll-md-windows-native
> > >
> > > This entire approach feels extremely brittle for the future.
> > >
> > > - Hendrik
> >
> > Yeah, definitely.
> > These days, other libraries like miniz are getting used as well, so
> > hardcoding each case separately is no better than what we have currently.
>
> Hmm not sure about the build from Hendrik. This was with a build with
> zlib and compression level 0?
>

Its a pretty old version of zlib, yes. I never bothered to update the
msvc library I use, since its just for FATE, so its from 2012, and the
build is from 31b5b3badc, a few commits before the compression=0
change was reverted.

- Hendrik
diff mbox series

Patch

diff --git a/tests/fate-run.sh b/tests/fate-run.sh
index 309ab85134..72251ec571 100755
--- a/tests/fate-run.sh
+++ b/tests/fate-run.sh
@@ -652,6 +652,16 @@  if test -e "$ref" || test $cmp = "oneline" || test $cmp = "null" || test $cmp =
         null)   cat               "$outfile"            >$cmpfile ;;
     esac
     cmperr=$?
+    if test -e "$ref".alt && !(test $cmp = "oneline" || test $cmp = "null" || test $cmp = "grep") ; then
+        case $cmp in
+            diff)   diff -u -b "$ref".alt "$outfile"            >$cmpfile ;;
+            rawdiff)diff -u    "$ref".alt "$outfile"            >$cmpfile ;;
+            oneoff) oneoff     "$ref".alt "$outfile"            >$cmpfile ;;
+            stddev) stddev     "$ref".alt "$outfile"            >$cmpfile ;;
+        esac
+        cmperr=$?
+    fi
+
     test $err = 0 && err=$cmperr
     if [ "$report_type" = "ignore" ]; then
         test $err = 0 || echo "IGNORE\t${test}" && err=0 && unset sig
diff --git a/tests/ref/fate/mov-cover-image.alt b/tests/ref/fate/mov-cover-image.alt
new file mode 100644
index 0000000000..bb94b86939
--- /dev/null
+++ b/tests/ref/fate/mov-cover-image.alt
@@ -0,0 +1,42 @@ 
+ad553810e0ce362697a2b21544daebde *tests/data/fate/mov-cover-image.mp4
+2063262 tests/data/fate/mov-cover-image.mp4
+#extradata 0:        2, 0x00340022
+#tb 0: 1/44100
+#media_type 0: audio
+#codec_id 0: aac
+#sample_rate 0: 44100
+#channel_layout_name 0: stereo
+#tb 1: 1/90000
+#media_type 1: video
+#codec_id 1: mjpeg
+#dimensions 1: 600x600
+#sar 1: 96/96
+#tb 2: 1/90000
+#media_type 2: video
+#codec_id 2: png
+#dimensions 2: 600x600
+#sar 2: 1/1
+0,      -2112,      -2112,     1024,        6, 0x027e00e8, F=0x5, S=1,       10
+0,      -1088,      -1088,     1024,        6, 0x027e00e8, F=0x5
+0,        -64,        -64,     1024,        6, 0x027e00e8
+1,          0,          0,        0,    25441, 0xe82503b0
+2,          0,          0,        0,  1084000, 0x70fc8139
+0,        960,        960,     1024,        6, 0x027e00e8
+0,       1984,       1984,     1024,        6, 0x027e00e8
+0,       3008,       3008,     1024,        6, 0x027e00e8
+0,       4032,       4032,     1024,        6, 0x027e00e8
+[STREAM]
+index=0
+codec_name=aac
+DISPOSITION:attached_pic=0
+[/STREAM]
+[STREAM]
+index=1
+codec_name=mjpeg
+DISPOSITION:attached_pic=1
+[/STREAM]
+[STREAM]
+index=2
+codec_name=png
+DISPOSITION:attached_pic=1
+[/STREAM]
diff --git a/tests/ref/fate/png-mdcv.alt b/tests/ref/fate/png-mdcv.alt
new file mode 100644
index 0000000000..1339fe3a46
--- /dev/null
+++ b/tests/ref/fate/png-mdcv.alt
@@ -0,0 +1,22 @@ 
+b674209fd9cd8eff945a6bc1a4b306d3 *tests/data/fate/png-mdcv.image2pipe
+2774240 tests/data/fate/png-mdcv.image2pipe
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 1280x720
+#sar 0: 0/1
+0,          0,          0,        1,  2764800, 0x2bfc7b42
+frames.frame.0.side_data_list.side_data.0.side_data_type="Content light level metadata"
+frames.frame.0.side_data_list.side_data.0.max_content=1000
+frames.frame.0.side_data_list.side_data.0.max_average=200
+frames.frame.0.side_data_list.side_data.1.side_data_type="Mastering display metadata"
+frames.frame.0.side_data_list.side_data.1.red_x="13250/50000"
+frames.frame.0.side_data_list.side_data.1.red_y="7500/50000"
+frames.frame.0.side_data_list.side_data.1.green_x="34000/50000"
+frames.frame.0.side_data_list.side_data.1.green_y="16000/50000"
+frames.frame.0.side_data_list.side_data.1.blue_x="2/50000"
+frames.frame.0.side_data_list.side_data.1.blue_y="0/50000"
+frames.frame.0.side_data_list.side_data.1.white_point_x="15635/50000"
+frames.frame.0.side_data_list.side_data.1.white_point_y="16450/50000"
+frames.frame.0.side_data_list.side_data.1.min_luminance="50/10000"
+frames.frame.0.side_data_list.side_data.1.max_luminance="10000000/10000"