[FFmpeg-devel] [PATCH 2/3] avcodec/x86: disable hevc 12b luma deblock

[Martin Storsjö]

On Sat, 24 Feb 2024, J. Dekker wrote:

>
> Nuo Mi <nuomi2021 at gmail.com> writes:
>
>> On Wed, Feb 21, 2024 at 7:10 PM J. Dekker <jdek at itanimul.li> wrote:
>>
>>> Over/underflow in some cases.
>>>
>>> Signed-off-by: J. Dekker <jdek at itanimul.li>
>>> ---
>>>  libavcodec/x86/hevcdsp_init.c | 9 +++++----
>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavcodec/x86/hevcdsp_init.c b/libavcodec/x86/hevcdsp_init.c
>>> index 31e81eb11f..11cb1b3bfd 100644
>>> --- a/libavcodec/x86/hevcdsp_init.c
>>> +++ b/libavcodec/x86/hevcdsp_init.c
>>> @@ -1205,10 +1205,11 @@ void ff_hevc_dsp_init_x86(HEVCDSPContext *c, const
>>> int bit_depth)
>>>          if (EXTERNAL_SSE2(cpu_flags)) {
>>>              c->hevc_v_loop_filter_chroma =
>>> ff_hevc_v_loop_filter_chroma_12_sse2;
>>>              c->hevc_h_loop_filter_chroma =
>>> ff_hevc_h_loop_filter_chroma_12_sse2;
>>> -            if (ARCH_X86_64) {
>>> -                c->hevc_v_loop_filter_luma =
>>> ff_hevc_v_loop_filter_luma_12_sse2;
>>> -                c->hevc_h_loop_filter_luma =
>>> ff_hevc_h_loop_filter_luma_12_sse2;
>>> -            }
>>> +            // FIXME: 12-bit luma deblock over/underflows in some cases
>>> +            // if (ARCH_X86_64) {
>>> +            //     c->hevc_v_loop_filter_luma =
>>> ff_hevc_v_loop_filter_luma_12_sse2;
>>> +            //     c->hevc_h_loop_filter_luma =
>>> ff_hevc_h_loop_filter_luma_12_sse2;
>>> +            // }
>>>              SAO_BAND_INIT(12, sse2);
>>>              SAO_EDGE_INIT(12, sse2);
>>>
>> Hi Dekker,
>> VVC will utilize this function as well.
>> Could you please share the HEVC clip or data that caused the overflow?
>> We'll make efforts to address it during the VVC porting
>>
>
> You can just run ./tests/checkasm/checkasm --test=hevc_deblock to
> find a failing case.

To clarify, this is with the new checkasm test added in this patchset, not 
currently in git master - otherwise fate would be failing for everybody on 
x86.

> My guess is that delta0 overflows before the right
> shift, see the ARM64 asm which specfically widens this calculation on 12
> bit variant but I'm not 100%, I don't know x86 asm.

Are you sure the input is within valid range? It's always possible that 
checkasm produces inputs that the real decoder wouldn't - but it's also 
possible that this is a real decoder bug that just hasn't been triggered 
by any other test yet.

// Martin