mbox series

[FFmpeg-devel,RFC,0/2] Make fate tests succeed with zlib-ng

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

Message

Alexander Strasser Oct. 18, 2024, 12:02 p.m. UTC 
This is as subject notes an RFC. I wanted to send it out quickly.
Didn't actually test on zlib systems yet (old and new).

So it is not that heavily tested yet and maybe has rough edges I
didn't notice.

I hope this would help to unbreak zlib-ng systems and not disturb
systems with older zlib versions.

The strategy is

1. re-apply Ramiro's patch to use compression level 0
2. fix the 2 tests that fail with older zlib with extra ref file

The mechanism should only be used when other solutions are not
available. The alternative files could be dropped in the future
e.g. when no more fate clients fail because of it.


Best regards,
  Alexander


Alexander Strasser (2):
  Reapply "tests/fate: disable compression for zlib-based codecs"
  fate: Make it possible to have alternative reference files

 tests/fate-run.sh                         | 10 ++++++
 tests/fate/cover-art.mak                  |  6 ++--
 tests/fate/image.mak                      |  4 +--
 tests/fate/lavf-image.mak                 |  5 +--
 tests/fate/lavf-video.mak                 |  4 +--
 tests/fate/mov.mak                        |  2 +-
 tests/fate/vcodec.mak                     |  4 ++-
 tests/ref/fate/copy-apng                  |  4 +--
 tests/ref/fate/cover-art-aiff-id3v2-remux |  6 ++--
 tests/ref/fate/cover-art-flac-remux       |  6 ++--
 tests/ref/fate/cover-art-mp3-id3v2-remux  |  6 ++--
 tests/ref/fate/mov-cover-image            |  6 ++--
 tests/ref/fate/mov-cover-image.alt        | 42 +++++++++++++++++++++++
 tests/ref/fate/png-icc                    |  6 ++--
 tests/ref/fate/png-mdcv                   |  4 +--
 tests/ref/fate/png-mdcv.alt               | 22 ++++++++++++
 tests/ref/lavf/apng                       |  4 +--
 tests/ref/lavf/apng.png                   |  4 +--
 tests/ref/lavf/gray16be.png               |  4 +--
 tests/ref/lavf/png                        |  4 +--
 tests/ref/lavf/rgb48be.png                |  4 +--
 tests/ref/seek/vsynth_lena-flashsv        | 40 ++++++++++-----------
 tests/ref/vsynth/vsynth1-flashsv          |  4 +--
 tests/ref/vsynth/vsynth1-mpng             |  4 +--
 tests/ref/vsynth/vsynth1-zlib             |  4 +--
 tests/ref/vsynth/vsynth2-flashsv          |  4 +--
 tests/ref/vsynth/vsynth2-mpng             |  4 +--
 tests/ref/vsynth/vsynth2-zlib             |  4 +--
 tests/ref/vsynth/vsynth3-flashsv          |  4 +--
 tests/ref/vsynth/vsynth3-mpng             |  4 +--
 tests/ref/vsynth/vsynth3-zlib             |  4 +--
 tests/ref/vsynth/vsynth_lena-flashsv      |  4 +--
 tests/ref/vsynth/vsynth_lena-mpng         |  4 +--
 tests/ref/vsynth/vsynth_lena-zlib         |  4 +--
 34 files changed, 161 insertions(+), 84 deletions(-)
 create mode 100644 tests/ref/fate/mov-cover-image.alt
 create mode 100644 tests/ref/fate/png-mdcv.alt

--

Comments

Derek Buitenhuis Oct. 18, 2024, 2:55 p.m. UTC | #1 
On 10/18/2024 1:02 PM, Alexander Strasser via ffmpeg-devel wrote:
> Alexander Strasser (2):
>   Reapply "tests/fate: disable compression for zlib-based codecs"
>   fate: Make it possible to have alternative reference files

I want to add this here, since I brought it up on IRC.

I think this entire saga has been pretty silly, and a workaround for
what is fundementally a bad test. Testing the exact contents and size
of encoded output here is fundementally the wrong thing to test and
provides no value as a test. I'd argue it provides negative value.

For a lossles encoder, especially one that uses external deps, but this
applies to internal ones too, almost no value is provided by ensuring
that its output is bit-exact - I'd actually consider it negative value.
It's testing the wrong thing (that output remains correct, decodable,
metadata, etc.) and only ensures no changes to anything can ever be
made without a reference update.

- Derek
Anton Khirnov Oct. 18, 2024, 5:20 p.m. UTC | #2 
Quoting Derek Buitenhuis (2024-10-18 16:55:58)
> On 10/18/2024 1:02 PM, Alexander Strasser via ffmpeg-devel wrote:
> > Alexander Strasser (2):
> >   Reapply "tests/fate: disable compression for zlib-based codecs"
> >   fate: Make it possible to have alternative reference files
> 
> I want to add this here, since I brought it up on IRC.
> 
> I think this entire saga has been pretty silly, and a workaround for
> what is fundementally a bad test. Testing the exact contents and size
> of encoded output here is fundementally the wrong thing to test and
> provides no value as a test. I'd argue it provides negative value.
> 
> For a lossles encoder, especially one that uses external deps, but this
> applies to internal ones too, almost no value is provided by ensuring
> that its output is bit-exact - I'd actually consider it negative value.
> It's testing the wrong thing (that output remains correct, decodable,
> metadata, etc.) and only ensures no changes to anything can ever be
> made without a reference update.

+1