[FFmpeg-devel] Patchwork FATE Errors

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Andriy
> Gelman
> Sent: Friday, January 21, 2022 5:24 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] Patchwork FATE Errors
> 
> On Fri, 21. Jan 01:44, Soft Works wrote:
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Andreas
> > > Rheinhardt
> > > Sent: Friday, January 21, 2022 2:31 AM
> > > To: ffmpeg-devel at ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] Patchwork FATE Errors
> > >
> > > Soft Works:
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Andreas
> > > >> Rheinhardt
> > > >> Sent: Thursday, January 20, 2022 11:38 PM
> > > >> To: ffmpeg-devel at ffmpeg.org
> > > >> Subject: Re: [FFmpeg-devel] Patchwork FATE Errors
> > > >>
> > > >> Soft Works:
> > > >>>
> > > >>>
> > > >>>> -----Original Message-----
> > > >>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > > Andreas
> > > >>>> Rheinhardt
> > > >>>> Sent: Thursday, January 20, 2022 10:22 PM
> > > >>>> To: ffmpeg-devel at ffmpeg.org
> > > >>>> Subject: Re: [FFmpeg-devel] Patchwork FATE Errors
> > > >>>>
> > > >>>> Soft Works:
> > > >>>>>
> > > >>>>> andriy/make_fate_ppc
> > > >>>>>
> > > >>>>> => Does it possibly need 'make fate-rsync'?
> > > >>>>>
> > > >>>>
> > > >>>> No. The test does not rely on need samples;
> > > >>>
> > > >>> It was just a very quick guess, because yesterday I rebased and
> > > >>> saw the test matroska-dovi-write-config7 failing which was fixed
> > > >>> after fate-rsync - that's why I though it might be the same reason
> > > >>> (with make -jX, it's probably not deterministic, which test will
> > > >>> fail first).
> > > >>>
> > > >>>
> > > >>>> and the other test that uses
> > > >>>> this sample works fine. Some time ago, someone else wrote FATE tests
> for
> > > >>>> AVDOVIDecoderConfigurationRecord in Matroska
> > > >>>>
> > > (https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220101165153.440729-
> > > >> 6-
> > > >>>> tcChlisop0 at gmail.com/).
> > > >>>> These were faulty and one of them relied on a sample that has
> apparently
> > > >>>> never been uploaded (but this test is actually redundant with the
> other
> > > >>>> test), so I investigated and saw that the test (presumably
> > > >>>> unintentially) reencoded audio, so I switched it to a pure copy test
> and
> > > >>>> applied it, believing that codec-copy tests could not possibly for
> some
> > > >>>> arches. That was a mistake and I am deeply sorry for this mess.
> > > >>>
> > > >>> Nevermind - things happen..
> > > >>>
> > > >>>
> > > >>> BTW, I was thinking about submitting a patch for
> libavutil/tests/md5.c
> > > >>>
> > > >>> something like:
> > > >>>
> > > >>> #ifdef __GNUC__
> > > >>> #pragma GCC diagnostic ignored "-Wdiscarded-qualifiers"
> > > >>> #endif
> > > >>>
> > > >>> #ifdef __clang__
> > > >>> #pragma clang diagnostic ignored "-Wdiscarded-qualifiers"
> > > >>> #endif
> > > >>>
> > > >>> Would that make sense?
> > > >>> Those warnings are appearing in every single fate error output on
> > > >> patchwork,
> > > >>> possibly covering up more relevant things.
> > > >>>
> > > >>
> > > >> Instead of pragmas one should limit the volatile to those compilers
> > > >> which miscompile the code without them.
> > > >> (IMO one does not need to find the exact set of compilers that
> > > >> miscompile this; all that matters is that recent versions don't give
> > > >> warnings and old versions don't miscompile. If some compilers of
> medium
> > > >> age still show this warning afterwards without needing the volatile,
> so
> > > >> be it.)
> > > >
> > > > You mean like this?
> > > >
> > > > #if defined(__clang__) && defined(__clang_major__) && __clang_major__ <
> 4
> > > >     volatile uint8_t in[1000]; // volatile to workaround
> > > http://llvm.org/bugs/show_bug.cgi?id=20849
> > > > #else
> > > >     uint8_t in[1000];
> > > > #endif
> > > >
> > >
> > > I would not use an else branch, but only put the volatile and the
> > > comment in the #if branch.
> > >
> > > >
> > > > It was fixed in 3.5.1, so "medium age" would be 3.5.1 to 4.0.0
> > > >
> > >
> > > Fine by me if tested.
> 
> >
> > I do not have a Clang setup locally, I'd assume at least one of the
> > Patchwork VMs uses Clang?
> 
> They're all using gcc at the moment.
> I can try to add extra info about the runners and their status. Maybe to a
> grafana page.

Maybe the effort would be better spent on improving the fate-error output.
Of course in most cases, one can reproduce locally, but not always.

The md5 warnings are gone with my patch.

Before: https://patchwork.ffmpeg.org/check/52439/
After: https://patchwork.ffmpeg.org/check/52456/

What would be nice is when it would output all *.err (and maybe *.diff)
files from the fate run.
Just an idea, though..

sw