Thank you for your review. > On Apr 17, 2018, at 15:29, Shengbin Meng <shengbinm...@gmail.com > <mailto:shengbinm...@gmail.com>> wrote: > > > >> On Apr 9, 2018, at 10:12, Yingming Fan <yingming...@gmail.com >> <mailto:yingming...@gmail.com>> wrote: >> >> From: Yingming Fan <yingming...@gmail.com <mailto:yingming...@gmail.com>> >> >> --- >> Hi, there. >> I plane to submit our arm32 neon codes for qpel and epel. >> While before this i will submit hevc_mc checkasm codes. >> This hevc_mc checkasm codes check every qpel and epel function, including 8 >> 10 and 12 bit. >> Passed test by using 'checkasm --test=hevc_mc' under Linux x86_64 MacOS >> x86_64 and Linux arm64 platform. >> Also passed FATE test. >> >> tests/checkasm/Makefile | 2 +- >> tests/checkasm/checkasm.c | 1 + >> tests/checkasm/checkasm.h | 1 + >> tests/checkasm/hevc_mc.c | 547 >> ++++++++++++++++++++++++++++++++++++++++++++++ >> tests/fate/checkasm.mak | 1 + >> 5 files changed, 551 insertions(+), 1 deletion(-) >> create mode 100644 tests/checkasm/hevc_mc.c >> >> diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile >> index 0233d2f989..e6c94cd676 100644 >> --- a/tests/checkasm/Makefile >> +++ b/tests/checkasm/Makefile >> @@ -23,7 +23,7 @@ AVCODECOBJS-$(CONFIG_EXR_DECODER) += exrdsp.o >> AVCODECOBJS-$(CONFIG_HUFFYUV_DECODER) += huffyuvdsp.o >> AVCODECOBJS-$(CONFIG_JPEG2000_DECODER) += jpeg2000dsp.o >> AVCODECOBJS-$(CONFIG_PIXBLOCKDSP) += pixblockdsp.o >> -AVCODECOBJS-$(CONFIG_HEVC_DECODER) += hevc_add_res.o hevc_idct.o >> hevc_sao.o >> +AVCODECOBJS-$(CONFIG_HEVC_DECODER) += hevc_add_res.o hevc_idct.o >> hevc_sao.o hevc_mc.o >> AVCODECOBJS-$(CONFIG_UTVIDEO_DECODER) += utvideodsp.o >> AVCODECOBJS-$(CONFIG_V210_ENCODER) += v210enc.o >> AVCODECOBJS-$(CONFIG_VP9_DECODER) += vp9dsp.o >> diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c >> index 20ce56932f..b95efc674d 100644 >> --- a/tests/checkasm/checkasm.c >> +++ b/tests/checkasm/checkasm.c >> @@ -117,6 +117,7 @@ static const struct { >> { "hevc_add_res", checkasm_check_hevc_add_res }, >> { "hevc_idct", checkasm_check_hevc_idct }, >> { "hevc_sao", checkasm_check_hevc_sao }, >> + { "hevc_mc", checkasm_check_hevc_mc }, >> #endif >> #if CONFIG_HUFFYUV_DECODER >> { "huffyuvdsp", checkasm_check_huffyuvdsp }, >> diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h >> index dcab74de06..5a4a612da7 100644 >> --- a/tests/checkasm/checkasm.h >> +++ b/tests/checkasm/checkasm.h >> @@ -58,6 +58,7 @@ void checkasm_check_h264qpel(void); >> void checkasm_check_hevc_add_res(void); >> void checkasm_check_hevc_idct(void); >> void checkasm_check_hevc_sao(void); >> +void checkasm_check_hevc_mc(void); >> void checkasm_check_huffyuvdsp(void); >> void checkasm_check_jpeg2000dsp(void); >> void checkasm_check_llviddsp(void); >> diff --git a/tests/checkasm/hevc_mc.c b/tests/checkasm/hevc_mc.c >> new file mode 100644 >> index 0000000000..018f322c11 >> --- /dev/null >> +++ b/tests/checkasm/hevc_mc.c >> @@ -0,0 +1,547 @@ >> +/* >> + * Copyright (c) 2018 Yingming Fan <yingming...@gmail.com >> <mailto:yingming...@gmail.com>> >> + * >> + * This file is part of FFmpeg. >> + * >> + * FFmpeg is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * FFmpeg is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License along >> + * with FFmpeg; if not, write to the Free Software Foundation, Inc., >> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. >> + */ >> + >> +#include <string.h> >> + >> +#include "libavutil/intreadwrite.h" >> + >> +#include "libavcodec/avcodec.h" >> + >> +#include "libavcodec/hevcdsp.h" >> + >> +#include "checkasm.h" >> + >> +static const uint32_t pixel_mask[3] = { 0xffffffff, 0x03ff03ff, 0x0fff0fff >> }; >> +static const uint32_t idx_width_map[8][2] = {{1, 4}, {3, 8}, {4, 12}, {5, >> 16}, {6, 24}, {7, 32}, {8, 48}, {9, 64}}; > Why not include block width 2 and 6? I notice that there are already some > optimization code for that.
I find there are no width 2 and 6 in x86 asm codes, so i remove 2 and 6. After your remind i find neon codes have 2 and 6. I will fix this in patch v2. > >> +#define SIZEOF_PIXEL ((bit_depth + 7) / 8) >> +#define PIXEL_STRIDE (AV_INPUT_BUFFER_PADDING_SIZE + MAX_PB_SIZE + >> AV_INPUT_BUFFER_PADDING_SIZE) >> +#define BUF_SIZE ((MAX_PB_SIZE+4+4) * PIXEL_STRIDE * 2) >> + >> +#define randomize_buffers(buf0, buf1, size) \ >> + do { \ >> + uint32_t mask = pixel_mask[(bit_depth - 8) >> 1]; \ >> + int k; \ >> + for (k = 0; k < size; k += 4) { \ >> + uint32_t r = rnd() & mask; \ >> + AV_WN32A(buf0 + k, r); \ >> + AV_WN32A(buf1 + k, r); \ >> + } \ >> + } while (0) >> + >> +#define randomize_buffers2(buf0, buf1, size) \ >> + do { \ >> + uint32_t mask = 0x1fff1fff; \ >> + int k; \ >> + for (k = 0; k < size; k += 4) { \ >> + uint32_t r = rnd() & mask; \ >> + AV_WN32A(buf0 + k, r); \ >> + AV_WN32A(buf1 + k, r); \ >> + } \ >> + } while (0) >> + >> +static int get_random(min, max, mod) > Function parameters should have types. agree >> +{ >> + int value = rnd()%mod; >> + int sign = rnd()&0x1 == 0 ? 1 : -1; >> + return av_clip(sign*value, min, max); >> +} >> + >> +static void check_put_hevc_qpel(HEVCDSPContext h, int bit_depth) >> +{ >> + int i, x, y; >> + LOCAL_ALIGNED_32(uint8_t, dst0, [BUF_SIZE]); >> + LOCAL_ALIGNED_32(uint8_t, dst1, [BUF_SIZE]); >> + LOCAL_ALIGNED_32(uint8_t, src0, [BUF_SIZE]); >> + LOCAL_ALIGNED_32(uint8_t, src1, [BUF_SIZE]); >> + >> + for (i = 0; i < FF_ARRAY_ELEMS(idx_width_map); i++) { >> + for (y = 0; y < 4; y++) { >> + for (x = 0; x < 4; x++) { >> + int idx = idx_width_map[i][0]; >> + int width = idx_width_map[i][1]; >> + int height= width; > So we are only testing the case of height == width? It would be better to > consider those non-square blocks too. I think square size is enough. Because square and non-square will have the same result, all height in mc asm codes implemented by loop. > >> + ptrdiff_t stride = PIXEL_STRIDE*SIZEOF_PIXEL; >> + int offset = >> (AV_INPUT_BUFFER_PADDING_SIZE+4*PIXEL_STRIDE)*SIZEOF_PIXEL; >> + declare_func_emms(AV_CPU_FLAG_MMX, void, int16_t *dst, >> uint8_t *src, ptrdiff_t srcstride, >> + int height, intptr_t mx, intptr_t my, int >> width); >> + >> + randomize_buffers(src0, src1, BUF_SIZE); >> + memset(dst0, 0, BUF_SIZE); >> + memset(dst1, 0, BUF_SIZE); >> + >> + if (check_func(h.put_hevc_qpel[idx][!!y][!!x], >> "put_hevc_qpel_%dx%d_%d_%d_%d", width, height, >> + x, y, bit_depth)) { >> + call_ref((int16_t *)dst0, src0 + offset, stride, >> height, x, y, width); >> + call_new((int16_t *)dst1, src1 + offset, stride, >> height, x, y, width); >> + if (memcmp(dst0, dst1, BUF_SIZE)) >> + fail(); >> + bench_new((int16_t *)dst1, src1 + offset, stride, >> height, x, y, width); >> + } >> + } >> + } >> + } >> +} >> + > > All these check_put_hevc_* functions look very similar. Can you reduce > duplicate code in some way? Using macros for example. Agree, good idea. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel