diff mbox

[FFmpeg-devel,1/4] tests/checkasm: add overflow test for hevc_add_res

Message ID 1576053936-742-1-git-send-email-linjie.fu@intel.com
State New
Headers show

Commit Message

Fu, Linjie Dec. 11, 2019, 8:45 a.m. UTC
Add overflow test for hevc_add_res when int16_t coeff = -32768.
The result of C is good, while ASM is not.

To verify:
    make fate-checkasm-hevc_add_res
    ffmpeg/tests/checkasm/checkasm --test=hevc_add_res

./checkasm --test=hevc_add_res
checkasm: using random seed 679391863
MMXEXT:
    hevc_add_res_4x4_8_mmxext (hevc_add_res.c:69)
  - hevc_add_res.add_residual [FAILED]
SSE2:
    hevc_add_res_8x8_8_sse2 (hevc_add_res.c:69)
    hevc_add_res_16x16_8_sse2 (hevc_add_res.c:69)
    hevc_add_res_32x32_8_sse2 (hevc_add_res.c:69)
  - hevc_add_res.add_residual [FAILED]
AVX:
    hevc_add_res_8x8_8_avx (hevc_add_res.c:69)
    hevc_add_res_16x16_8_avx (hevc_add_res.c:69)
    hevc_add_res_32x32_8_avx (hevc_add_res.c:69)
  - hevc_add_res.add_residual [FAILED]
AVX2:
    hevc_add_res_32x32_8_avx2 (hevc_add_res.c:69)
  - hevc_add_res.add_residual [FAILED]
checkasm: 8 of 14 tests have failed

Signed-off-by: Xu Guangxin <guangxin.xu@intel.com>
Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
 tests/checkasm/hevc_add_res.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Fu, Linjie Dec. 15, 2019, 2:01 a.m. UTC | #1
Hi,

> -----Original Message-----
> From: Fu, Linjie <linjie.fu@intel.com>
> Sent: Wednesday, December 11, 2019 16:46
> To: ffmpeg-devel@ffmpeg.org
> Cc: Fu, Linjie <linjie.fu@intel.com>; Xu, Guangxin <guangxin.xu@intel.com>
> Subject: [PATCH 1/4] tests/checkasm: add overflow test for hevc_add_res
> 
> Add overflow test for hevc_add_res when int16_t coeff = -32768.
> The result of C is good, while ASM is not.
> 
> To verify:
>     make fate-checkasm-hevc_add_res
>     ffmpeg/tests/checkasm/checkasm --test=hevc_add_res
> 
> ./checkasm --test=hevc_add_res
> checkasm: using random seed 679391863
> MMXEXT:
>     hevc_add_res_4x4_8_mmxext (hevc_add_res.c:69)
>   - hevc_add_res.add_residual [FAILED]
> SSE2:
>     hevc_add_res_8x8_8_sse2 (hevc_add_res.c:69)
>     hevc_add_res_16x16_8_sse2 (hevc_add_res.c:69)
>     hevc_add_res_32x32_8_sse2 (hevc_add_res.c:69)
>   - hevc_add_res.add_residual [FAILED]
> AVX:
>     hevc_add_res_8x8_8_avx (hevc_add_res.c:69)
>     hevc_add_res_16x16_8_avx (hevc_add_res.c:69)
>     hevc_add_res_32x32_8_avx (hevc_add_res.c:69)
>   - hevc_add_res.add_residual [FAILED]
> AVX2:
>     hevc_add_res_32x32_8_avx2 (hevc_add_res.c:69)
>   - hevc_add_res.add_residual [FAILED]
> checkasm: 8 of 14 tests have failed
> 
> Signed-off-by: Xu Guangxin <guangxin.xu@intel.com>
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
>  tests/checkasm/hevc_add_res.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/checkasm/hevc_add_res.c
> b/tests/checkasm/hevc_add_res.c
> index e92c6b4..a6e3b8a 100644
> --- a/tests/checkasm/hevc_add_res.c
> +++ b/tests/checkasm/hevc_add_res.c
> @@ -58,6 +58,7 @@ static void check_add_res(HEVCDSPContext h, int
> bit_depth)
> 
>          randomize_buffers(res0, size);
>          randomize_buffers2(dst0, size);
> +        res0[0] = 0x8000;// overflow test
>          memcpy(res1, res0, sizeof(*res0) * size);
>          memcpy(dst1, dst0, sizeof(int16_t) * size);
> 
> --
> 2.7.4
A kindly ping.
Any comments towards this?

- linjie
Fu, Linjie Dec. 18, 2019, 1:55 a.m. UTC | #2
Hi,

> -----Original Message-----

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu,

> Linjie

> Sent: Sunday, December 15, 2019 10:02

> To: FFmpeg development discussions and patches <ffmpeg-

> devel@ffmpeg.org>; jamrial@gmail.com; u@pkh.me; Pierre-

> Edouard.Lepere@insa-rennes.fr

> Subject: Re: [FFmpeg-devel] [PATCH 1/4] tests/checkasm: add overflow test

> for hevc_add_res

> 

> Hi,

> 

> > -----Original Message-----

> > From: Fu, Linjie <linjie.fu@intel.com>

> > Sent: Wednesday, December 11, 2019 16:46

> > To: ffmpeg-devel@ffmpeg.org

> > Cc: Fu, Linjie <linjie.fu@intel.com>; Xu, Guangxin <guangxin.xu@intel.com>

> > Subject: [PATCH 1/4] tests/checkasm: add overflow test for hevc_add_res

> >

> > Add overflow test for hevc_add_res when int16_t coeff = -32768.

> > The result of C is good, while ASM is not.

> >

> > To verify:

> >     make fate-checkasm-hevc_add_res

> >     ffmpeg/tests/checkasm/checkasm --test=hevc_add_res

> >

> > ./checkasm --test=hevc_add_res

> > checkasm: using random seed 679391863

> > MMXEXT:

> >     hevc_add_res_4x4_8_mmxext (hevc_add_res.c:69)

> >   - hevc_add_res.add_residual [FAILED]

> > SSE2:

> >     hevc_add_res_8x8_8_sse2 (hevc_add_res.c:69)

> >     hevc_add_res_16x16_8_sse2 (hevc_add_res.c:69)

> >     hevc_add_res_32x32_8_sse2 (hevc_add_res.c:69)

> >   - hevc_add_res.add_residual [FAILED]

> > AVX:

> >     hevc_add_res_8x8_8_avx (hevc_add_res.c:69)

> >     hevc_add_res_16x16_8_avx (hevc_add_res.c:69)

> >     hevc_add_res_32x32_8_avx (hevc_add_res.c:69)

> >   - hevc_add_res.add_residual [FAILED]

> > AVX2:

> >     hevc_add_res_32x32_8_avx2 (hevc_add_res.c:69)

> >   - hevc_add_res.add_residual [FAILED]

> > checkasm: 8 of 14 tests have failed

> >

> > Signed-off-by: Xu Guangxin <guangxin.xu@intel.com>

> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>

> > ---

> >  tests/checkasm/hevc_add_res.c | 1 +

> >  1 file changed, 1 insertion(+)

> >

> > diff --git a/tests/checkasm/hevc_add_res.c

> > b/tests/checkasm/hevc_add_res.c

> > index e92c6b4..a6e3b8a 100644

> > --- a/tests/checkasm/hevc_add_res.c

> > +++ b/tests/checkasm/hevc_add_res.c

> > @@ -58,6 +58,7 @@ static void check_add_res(HEVCDSPContext h, int

> > bit_depth)

> >

> >          randomize_buffers(res0, size);

> >          randomize_buffers2(dst0, size);

> > +        res0[0] = 0x8000;// overflow test

> >          memcpy(res1, res0, sizeof(*res0) * size);

> >          memcpy(dst1, dst0, sizeof(int16_t) * size);

> >

> > --

> > 2.7.4

> A kindly ping.

> Any comments towards this?

> 

Ping, and looking forward to any feedbacks.

This overflow does affect the decode of the clips with coeff=-32768, and makes it
difficult(unless by --disbale-asm) to use software decoded result as a reference for
comparation with the hardware decoder.

- linjie
Fu, Linjie Dec. 27, 2019, 9:03 a.m. UTC | #3
Hi,

> -----Original Message-----

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu,

> Linjie

> Sent: Wednesday, December 18, 2019 09:55

> To: FFmpeg development discussions and patches <ffmpeg-

> devel@ffmpeg.org>; alexandra@khirnov.net; cus@passwd.hu; Paul B Mahol

> <onemda@gmail.com>; jamrial@gmail.com; u@pkh.me

> Subject: Re: [FFmpeg-devel] [PATCH 1/4] tests/checkasm: add overflow test

> for hevc_add_res

> 

> Hi,

> 

> > -----Original Message-----

> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of

> Fu,

> > Linjie

> > Sent: Sunday, December 15, 2019 10:02

> > To: FFmpeg development discussions and patches <ffmpeg-

> > devel@ffmpeg.org>; jamrial@gmail.com; u@pkh.me; Pierre-

> > Edouard.Lepere@insa-rennes.fr

> > Subject: Re: [FFmpeg-devel] [PATCH 1/4] tests/checkasm: add overflow

> test

> > for hevc_add_res

> >

> > Hi,

> >

> > > -----Original Message-----

> > > From: Fu, Linjie <linjie.fu@intel.com>

> > > Sent: Wednesday, December 11, 2019 16:46

> > > To: ffmpeg-devel@ffmpeg.org

> > > Cc: Fu, Linjie <linjie.fu@intel.com>; Xu, Guangxin

> <guangxin.xu@intel.com>

> > > Subject: [PATCH 1/4] tests/checkasm: add overflow test for

> hevc_add_res

> > >

> > > Add overflow test for hevc_add_res when int16_t coeff = -32768.

> > > The result of C is good, while ASM is not.

> > >

> > > To verify:

> > >     make fate-checkasm-hevc_add_res

> > >     ffmpeg/tests/checkasm/checkasm --test=hevc_add_res

> > >

> > > ./checkasm --test=hevc_add_res

> > > checkasm: using random seed 679391863

> > > MMXEXT:

> > >     hevc_add_res_4x4_8_mmxext (hevc_add_res.c:69)

> > >   - hevc_add_res.add_residual [FAILED]

> > > SSE2:

> > >     hevc_add_res_8x8_8_sse2 (hevc_add_res.c:69)

> > >     hevc_add_res_16x16_8_sse2 (hevc_add_res.c:69)

> > >     hevc_add_res_32x32_8_sse2 (hevc_add_res.c:69)

> > >   - hevc_add_res.add_residual [FAILED]

> > > AVX:

> > >     hevc_add_res_8x8_8_avx (hevc_add_res.c:69)

> > >     hevc_add_res_16x16_8_avx (hevc_add_res.c:69)

> > >     hevc_add_res_32x32_8_avx (hevc_add_res.c:69)

> > >   - hevc_add_res.add_residual [FAILED]

> > > AVX2:

> > >     hevc_add_res_32x32_8_avx2 (hevc_add_res.c:69)

> > >   - hevc_add_res.add_residual [FAILED]

> > > checkasm: 8 of 14 tests have failed

> > >

> > > Signed-off-by: Xu Guangxin <guangxin.xu@intel.com>

> > > Signed-off-by: Linjie Fu <linjie.fu@intel.com>

> > > ---

> > >  tests/checkasm/hevc_add_res.c | 1 +

> > >  1 file changed, 1 insertion(+)

> > >

> > > diff --git a/tests/checkasm/hevc_add_res.c

> > > b/tests/checkasm/hevc_add_res.c

> > > index e92c6b4..a6e3b8a 100644

> > > --- a/tests/checkasm/hevc_add_res.c

> > > +++ b/tests/checkasm/hevc_add_res.c

> > > @@ -58,6 +58,7 @@ static void check_add_res(HEVCDSPContext h, int

> > > bit_depth)

> > >

> > >          randomize_buffers(res0, size);

> > >          randomize_buffers2(dst0, size);

> > > +        res0[0] = 0x8000;// overflow test

> > >          memcpy(res1, res0, sizeof(*res0) * size);

> > >          memcpy(dst1, dst0, sizeof(int16_t) * size);

> > >

> > > --

> > > 2.7.4

> > A kindly ping.

> > Any comments towards this?

> >

> Ping, and looking forward to any feedbacks.

> 

> This overflow does affect the decode of the clips with coeff=-32768, and

> makes it

> difficult(unless by --disbale-asm) to use software decoded result as a

> reference for

> comparation with the hardware decoder.


Ping for review.
Same issue exists in h264_idct.asm and make the decode fail.
IMHO we should get this fixed firstly.

- linjie
Fu, Linjie Dec. 30, 2019, 1:51 a.m. UTC | #4
Hi,

> -----Original Message-----

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu,

> Linjie

> Sent: Friday, December 27, 2019 17:04

> To: FFmpeg development discussions and patches <ffmpeg-

> devel@ffmpeg.org>

> Subject: Re: [FFmpeg-devel] [PATCH 1/4] tests/checkasm: add overflow test

> for hevc_add_res

> 

> Hi,

> 

> > -----Original Message-----

> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of

> Fu,

> > Linjie

> > Sent: Wednesday, December 18, 2019 09:55

> > To: FFmpeg development discussions and patches <ffmpeg-

> > devel@ffmpeg.org>; alexandra@khirnov.net; cus@passwd.hu; Paul B

> Mahol

> > <onemda@gmail.com>; jamrial@gmail.com; u@pkh.me

> > Subject: Re: [FFmpeg-devel] [PATCH 1/4] tests/checkasm: add overflow

> test

> > for hevc_add_res

> >

> > Hi,

> >

> > > -----Original Message-----

> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of

> > Fu,

> > > Linjie

> > > Sent: Sunday, December 15, 2019 10:02

> > > To: FFmpeg development discussions and patches <ffmpeg-

> > > devel@ffmpeg.org>; jamrial@gmail.com; u@pkh.me; Pierre-

> > > Edouard.Lepere@insa-rennes.fr

> > > Subject: Re: [FFmpeg-devel] [PATCH 1/4] tests/checkasm: add overflow

> > test

> > > for hevc_add_res

> > >

> > > Hi,

> > >

> > > > -----Original Message-----

> > > > From: Fu, Linjie <linjie.fu@intel.com>

> > > > Sent: Wednesday, December 11, 2019 16:46

> > > > To: ffmpeg-devel@ffmpeg.org

> > > > Cc: Fu, Linjie <linjie.fu@intel.com>; Xu, Guangxin

> > <guangxin.xu@intel.com>

> > > > Subject: [PATCH 1/4] tests/checkasm: add overflow test for

> > hevc_add_res

> > > >

> > > > Add overflow test for hevc_add_res when int16_t coeff = -32768.

> > > > The result of C is good, while ASM is not.

> > > >

> > > > To verify:

> > > >     make fate-checkasm-hevc_add_res

> > > >     ffmpeg/tests/checkasm/checkasm --test=hevc_add_res

> > > >

> > > > ./checkasm --test=hevc_add_res

> > > > checkasm: using random seed 679391863

> > > > MMXEXT:

> > > >     hevc_add_res_4x4_8_mmxext (hevc_add_res.c:69)

> > > >   - hevc_add_res.add_residual [FAILED]

> > > > SSE2:

> > > >     hevc_add_res_8x8_8_sse2 (hevc_add_res.c:69)

> > > >     hevc_add_res_16x16_8_sse2 (hevc_add_res.c:69)

> > > >     hevc_add_res_32x32_8_sse2 (hevc_add_res.c:69)

> > > >   - hevc_add_res.add_residual [FAILED]

> > > > AVX:

> > > >     hevc_add_res_8x8_8_avx (hevc_add_res.c:69)

> > > >     hevc_add_res_16x16_8_avx (hevc_add_res.c:69)

> > > >     hevc_add_res_32x32_8_avx (hevc_add_res.c:69)

> > > >   - hevc_add_res.add_residual [FAILED]

> > > > AVX2:

> > > >     hevc_add_res_32x32_8_avx2 (hevc_add_res.c:69)

> > > >   - hevc_add_res.add_residual [FAILED]

> > > > checkasm: 8 of 14 tests have failed

> > > >

> > > > Signed-off-by: Xu Guangxin <guangxin.xu@intel.com>

> > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com>

> > > > ---

> > > >  tests/checkasm/hevc_add_res.c | 1 +

> > > >  1 file changed, 1 insertion(+)

> > > >

> > > > diff --git a/tests/checkasm/hevc_add_res.c

> > > > b/tests/checkasm/hevc_add_res.c

> > > > index e92c6b4..a6e3b8a 100644

> > > > --- a/tests/checkasm/hevc_add_res.c

> > > > +++ b/tests/checkasm/hevc_add_res.c

> > > > @@ -58,6 +58,7 @@ static void check_add_res(HEVCDSPContext h, int

> > > > bit_depth)

> > > >

> > > >          randomize_buffers(res0, size);

> > > >          randomize_buffers2(dst0, size);

> > > > +        res0[0] = 0x8000;// overflow test

> > > >          memcpy(res1, res0, sizeof(*res0) * size);

> > > >          memcpy(dst1, dst0, sizeof(int16_t) * size);

> > > >

> > > > --

> > > > 2.7.4

> > > A kindly ping.

> > > Any comments towards this?

> > >

> > Ping, and looking forward to any feedbacks.

> >

> > This overflow does affect the decode of the clips with coeff=-32768, and

> > makes it

> > difficult(unless by --disbale-asm) to use software decoded result as a

> > reference for

> > comparation with the hardware decoder.

> 

> Ping for review.

> Same issue exists in h264_idct.asm and make the decode fail.

> IMHO we should get this fixed firstly.

> 

> - linjie

> 

Ping.

- linjie
diff mbox

Patch

diff --git a/tests/checkasm/hevc_add_res.c b/tests/checkasm/hevc_add_res.c
index e92c6b4..a6e3b8a 100644
--- a/tests/checkasm/hevc_add_res.c
+++ b/tests/checkasm/hevc_add_res.c
@@ -58,6 +58,7 @@  static void check_add_res(HEVCDSPContext h, int bit_depth)
 
         randomize_buffers(res0, size);
         randomize_buffers2(dst0, size);
+        res0[0] = 0x8000;// overflow test
         memcpy(res1, res0, sizeof(*res0) * size);
         memcpy(dst1, dst0, sizeof(int16_t) * size);