[FFmpeg-devel] fate/pixfmt: disable dithering in the scale filter
Checks
| 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
|
Commit Message
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(-)
Comments
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
[...]
On 11/8/2024 9:03 PM, Michael Niedermayer wrote:
> 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.
I can't even reproduce the issue locally, so...
On Sat, Nov 09, 2024 at 11:05:32AM -0300, James Almer wrote:
> On 11/8/2024 9:03 PM, Michael Niedermayer wrote:
> > 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.
>
> I can't even reproduce the issue locally, so...
is this a threads >1 issue ?
thx
[...]
On 11/9/2024 1:19 PM, Michael Niedermayer wrote:
> On Sat, Nov 09, 2024 at 11:05:32AM -0300, James Almer wrote:
>> On 11/8/2024 9:03 PM, Michael Niedermayer wrote:
>>> 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.
>>
>> I can't even reproduce the issue locally, so...
>
> is this a threads >1 issue ?
Doesn't look like, it happens on fate clients running the default THREADS=1.
@@ -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
}
@@ -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
@@ -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
@@ -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
@@ -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