| Message ID | 20220517115429.2020-1-jdek@itanimul.li |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] checkasm: improve hevc_sao test | expand |
| 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 |
On Tue, 17 May 2022, J. Dekker wrote: > The HEVC decoder can call these functions with smaller widths than the > functions themselves are designed to operate on so we should only check > the relevant output > > Signed-off-by: J. Dekker <jdek@itanimul.li> > --- > tests/checkasm/hevc_sao.c | 51 ++++++++++++++++++++++++--------------- > 1 file changed, 31 insertions(+), 20 deletions(-) > > diff --git a/tests/checkasm/hevc_sao.c b/tests/checkasm/hevc_sao.c > index 6b750758e2..72cdb87dd1 100644 > --- a/tests/checkasm/hevc_sao.c > +++ b/tests/checkasm/hevc_sao.c > @@ -78,20 +78,26 @@ static void check_sao_band(HEVCDSPContext h, int bit_depth) > > for (i = 0; i <= 4; i++) { > int block_size = sao_size[i]; > + int prev_size = i > 0 ? sao_size[i - 1] : 0; > ptrdiff_t stride = PIXEL_STRIDE*SIZEOF_PIXEL; > declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *dst, uint8_t *src, ptrdiff_t dst_stride, ptrdiff_t src_stride, > int16_t *sao_offset_val, int sao_left_class, int width, int height); > > - randomize_buffers(src0, src1, BUF_SIZE); > - randomize_buffers2(offset_val, OFFSET_LENGTH); > - memset(dst0, 0, BUF_SIZE); > - memset(dst1, 0, BUF_SIZE); > - > - if (check_func(h.sao_band_filter[i], "hevc_sao_band_%dx%d_%d", block_size, block_size, bit_depth)) { > - call_ref(dst0, src0, stride, stride, offset_val, left_class, block_size, block_size); > - call_new(dst1, src1, stride, stride, offset_val, left_class, block_size, block_size); > - if (memcmp(dst0, dst1, BUF_SIZE)) > - fail(); > + if (check_func(h.sao_band_filter[i], "hevc_sao_band_%d_%d", block_size, bit_depth)) { > + > + for (int w = prev_size + 4; w <= block_size; w += 4) { > + randomize_buffers(src0, src1, BUF_SIZE); > + randomize_buffers2(offset_val, OFFSET_LENGTH); > + memset(dst0, 0, BUF_SIZE); > + memset(dst1, 0, BUF_SIZE); > + > + call_ref(dst0, src0, stride, stride, offset_val, left_class, w, block_size); > + call_new(dst1, src1, stride, stride, offset_val, left_class, w, block_size); > + for (int j = 0; j < block_size; j++) { > + if (memcmp(dst0 + j*MAX_PB_SIZE*2, dst1 + j*MAX_PB_SIZE*2, w)) I'm not quite sure about the MAX_PB_SIZE*2 part here - shouldn't that be just the 'stride' variable instead? And for the compared length ('w'), shouldn't that be multiplied by SIZEOF_PIXEL? Other than that, this looks good to me! // Martin
On 24 May 2022, at 22:27, Martin Storsjö wrote: > On Tue, 17 May 2022, J. Dekker wrote: > >> The HEVC decoder can call these functions with smaller widths than the >> functions themselves are designed to operate on so we should only check >> the relevant output >> >> Signed-off-by: J. Dekker <jdek@itanimul.li> >> --- >> tests/checkasm/hevc_sao.c | 51 ++++++++++++++++++++++++--------------- >> 1 file changed, 31 insertions(+), 20 deletions(-) >> >> diff --git a/tests/checkasm/hevc_sao.c b/tests/checkasm/hevc_sao.c >> index 6b750758e2..72cdb87dd1 100644 >> --- a/tests/checkasm/hevc_sao.c >> +++ b/tests/checkasm/hevc_sao.c >> @@ -78,20 +78,26 @@ static void check_sao_band(HEVCDSPContext h, int bit_depth) >> >> for (i = 0; i <= 4; i++) { >> int block_size = sao_size[i]; >> + int prev_size = i > 0 ? sao_size[i - 1] : 0; >> ptrdiff_t stride = PIXEL_STRIDE*SIZEOF_PIXEL; >> declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *dst, uint8_t *src, ptrdiff_t dst_stride, ptrdiff_t src_stride, >> int16_t *sao_offset_val, int sao_left_class, int width, int height); >> >> - randomize_buffers(src0, src1, BUF_SIZE); >> - randomize_buffers2(offset_val, OFFSET_LENGTH); >> - memset(dst0, 0, BUF_SIZE); >> - memset(dst1, 0, BUF_SIZE); >> - >> - if (check_func(h.sao_band_filter[i], "hevc_sao_band_%dx%d_%d", block_size, block_size, bit_depth)) { >> - call_ref(dst0, src0, stride, stride, offset_val, left_class, block_size, block_size); >> - call_new(dst1, src1, stride, stride, offset_val, left_class, block_size, block_size); >> - if (memcmp(dst0, dst1, BUF_SIZE)) >> - fail(); >> + if (check_func(h.sao_band_filter[i], "hevc_sao_band_%d_%d", block_size, bit_depth)) { >> + >> + for (int w = prev_size + 4; w <= block_size; w += 4) { >> + randomize_buffers(src0, src1, BUF_SIZE); >> + randomize_buffers2(offset_val, OFFSET_LENGTH); >> + memset(dst0, 0, BUF_SIZE); >> + memset(dst1, 0, BUF_SIZE); >> + >> + call_ref(dst0, src0, stride, stride, offset_val, left_class, w, block_size); >> + call_new(dst1, src1, stride, stride, offset_val, left_class, w, block_size); >> + for (int j = 0; j < block_size; j++) { >> + if (memcmp(dst0 + j*MAX_PB_SIZE*2, dst1 + j*MAX_PB_SIZE*2, w)) > > I'm not quite sure about the MAX_PB_SIZE*2 part here - shouldn't that be just the 'stride' variable instead? And for the compared length ('w'), shouldn't that be multiplied by SIZEOF_PIXEL? > > Other than that, this looks good to me! Pushed with this fix. Rest of the set as-is. I have an extra SAO patch but don't want to delay this set further. Thanks,
On Wed, 25 May 2022, J. Dekker wrote: > > > On 24 May 2022, at 22:27, Martin Storsjö wrote: > >> On Tue, 17 May 2022, J. Dekker wrote: >> >>> The HEVC decoder can call these functions with smaller widths than the >>> functions themselves are designed to operate on so we should only check >>> the relevant output >>> >>> Signed-off-by: J. Dekker <jdek@itanimul.li> >>> --- >>> tests/checkasm/hevc_sao.c | 51 ++++++++++++++++++++++++--------------- >>> 1 file changed, 31 insertions(+), 20 deletions(-) >>> >>> diff --git a/tests/checkasm/hevc_sao.c b/tests/checkasm/hevc_sao.c >>> index 6b750758e2..72cdb87dd1 100644 >>> --- a/tests/checkasm/hevc_sao.c >>> +++ b/tests/checkasm/hevc_sao.c >>> @@ -78,20 +78,26 @@ static void check_sao_band(HEVCDSPContext h, int bit_depth) >>> >>> for (i = 0; i <= 4; i++) { >>> int block_size = sao_size[i]; >>> + int prev_size = i > 0 ? sao_size[i - 1] : 0; >>> ptrdiff_t stride = PIXEL_STRIDE*SIZEOF_PIXEL; >>> declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *dst, uint8_t *src, ptrdiff_t dst_stride, ptrdiff_t src_stride, >>> int16_t *sao_offset_val, int sao_left_class, int width, int height); >>> >>> - randomize_buffers(src0, src1, BUF_SIZE); >>> - randomize_buffers2(offset_val, OFFSET_LENGTH); >>> - memset(dst0, 0, BUF_SIZE); >>> - memset(dst1, 0, BUF_SIZE); >>> - >>> - if (check_func(h.sao_band_filter[i], "hevc_sao_band_%dx%d_%d", block_size, block_size, bit_depth)) { >>> - call_ref(dst0, src0, stride, stride, offset_val, left_class, block_size, block_size); >>> - call_new(dst1, src1, stride, stride, offset_val, left_class, block_size, block_size); >>> - if (memcmp(dst0, dst1, BUF_SIZE)) >>> - fail(); >>> + if (check_func(h.sao_band_filter[i], "hevc_sao_band_%d_%d", block_size, bit_depth)) { >>> + >>> + for (int w = prev_size + 4; w <= block_size; w += 4) { >>> + randomize_buffers(src0, src1, BUF_SIZE); >>> + randomize_buffers2(offset_val, OFFSET_LENGTH); >>> + memset(dst0, 0, BUF_SIZE); >>> + memset(dst1, 0, BUF_SIZE); >>> + >>> + call_ref(dst0, src0, stride, stride, offset_val, left_class, w, block_size); >>> + call_new(dst1, src1, stride, stride, offset_val, left_class, w, block_size); >>> + for (int j = 0; j < block_size; j++) { >>> + if (memcmp(dst0 + j*MAX_PB_SIZE*2, dst1 + j*MAX_PB_SIZE*2, w)) >> >> I'm not quite sure about the MAX_PB_SIZE*2 part here - shouldn't that be just the 'stride' variable instead? And for the compared length ('w'), shouldn't that be multiplied by SIZEOF_PIXEL? >> >> Other than that, this looks good to me! > > Pushed with this fix. Rest of the set as-is. I have an extra SAO patch > but don't want to delay this set further. Thanks! Yes, it's better to land the previously reviewed and good parts, to keep the focus in the reviews. // Martin
diff --git a/tests/checkasm/hevc_sao.c b/tests/checkasm/hevc_sao.c index 6b750758e2..72cdb87dd1 100644 --- a/tests/checkasm/hevc_sao.c +++ b/tests/checkasm/hevc_sao.c @@ -78,20 +78,26 @@ static void check_sao_band(HEVCDSPContext h, int bit_depth) for (i = 0; i <= 4; i++) { int block_size = sao_size[i]; + int prev_size = i > 0 ? sao_size[i - 1] : 0; ptrdiff_t stride = PIXEL_STRIDE*SIZEOF_PIXEL; declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *dst, uint8_t *src, ptrdiff_t dst_stride, ptrdiff_t src_stride, int16_t *sao_offset_val, int sao_left_class, int width, int height); - randomize_buffers(src0, src1, BUF_SIZE); - randomize_buffers2(offset_val, OFFSET_LENGTH); - memset(dst0, 0, BUF_SIZE); - memset(dst1, 0, BUF_SIZE); - - if (check_func(h.sao_band_filter[i], "hevc_sao_band_%dx%d_%d", block_size, block_size, bit_depth)) { - call_ref(dst0, src0, stride, stride, offset_val, left_class, block_size, block_size); - call_new(dst1, src1, stride, stride, offset_val, left_class, block_size, block_size); - if (memcmp(dst0, dst1, BUF_SIZE)) - fail(); + if (check_func(h.sao_band_filter[i], "hevc_sao_band_%d_%d", block_size, bit_depth)) { + + for (int w = prev_size + 4; w <= block_size; w += 4) { + randomize_buffers(src0, src1, BUF_SIZE); + randomize_buffers2(offset_val, OFFSET_LENGTH); + memset(dst0, 0, BUF_SIZE); + memset(dst1, 0, BUF_SIZE); + + call_ref(dst0, src0, stride, stride, offset_val, left_class, w, block_size); + call_new(dst1, src1, stride, stride, offset_val, left_class, w, block_size); + for (int j = 0; j < block_size; j++) { + if (memcmp(dst0 + j*MAX_PB_SIZE*2, dst1 + j*MAX_PB_SIZE*2, w)) + fail(); + } + } bench_new(dst1, src1, stride, stride, offset_val, left_class, block_size, block_size); } } @@ -109,21 +115,26 @@ static void check_sao_edge(HEVCDSPContext h, int bit_depth) for (i = 0; i <= 4; i++) { int block_size = sao_size[i]; + int prev_size = i > 0 ? sao_size[i - 1] : 0; ptrdiff_t stride = PIXEL_STRIDE*SIZEOF_PIXEL; int offset = (AV_INPUT_BUFFER_PADDING_SIZE + PIXEL_STRIDE)*SIZEOF_PIXEL; declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *dst, uint8_t *src, ptrdiff_t stride_dst, int16_t *sao_offset_val, int eo, int width, int height); - randomize_buffers(src0, src1, BUF_SIZE); - randomize_buffers2(offset_val, OFFSET_LENGTH); - memset(dst0, 0, BUF_SIZE); - memset(dst1, 0, BUF_SIZE); - - if (check_func(h.sao_edge_filter[i], "hevc_sao_edge_%dx%d_%d", block_size, block_size, bit_depth)) { - call_ref(dst0, src0 + offset, stride, offset_val, eo, block_size, block_size); - call_new(dst1, src1 + offset, stride, offset_val, eo, block_size, block_size); - if (memcmp(dst0, dst1, BUF_SIZE)) - fail(); + for (int w = prev_size + 4; w <= block_size; w += 4) { + randomize_buffers(src0, src1, BUF_SIZE); + randomize_buffers2(offset_val, OFFSET_LENGTH); + memset(dst0, 0, BUF_SIZE); + memset(dst1, 0, BUF_SIZE); + + if (check_func(h.sao_edge_filter[i], "hevc_sao_edge_%d_%d", block_size, bit_depth)) { + call_ref(dst0, src0 + offset, stride, offset_val, eo, w, block_size); + call_new(dst1, src1 + offset, stride, offset_val, eo, w, block_size); + for (int j = 0; j < block_size; j++) { + if (memcmp(dst0 + j*MAX_PB_SIZE*2, dst1 + j*MAX_PB_SIZE*2, w)) + fail(); + } + } bench_new(dst1, src1 + offset, stride, offset_val, eo, block_size, block_size); } }
The HEVC decoder can call these functions with smaller widths than the functions themselves are designed to operate on so we should only check the relevant output Signed-off-by: J. Dekker <jdek@itanimul.li> --- tests/checkasm/hevc_sao.c | 51 ++++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 20 deletions(-)