Quoting Josh de Kock (2016-05-31 18:31:29)
> x86/hevc: add avx2 dc idct
>
> hevc: separate residu and prediction
>
> x86/hevc_idct: replace old and unused idct functions
>
> hevc: align coeffs to 32byte boundary
> Fixes a segfault related to AVX not aligning to a 32bit boundary
>
> Authors:
> James Almer <[email protected]>
> Michael Niedermayer <[email protected]
> Pierre Edouard <[email protected]>
> ---
> libavcodec/hevc.c | 28 ++--
> libavcodec/hevcdsp.c | 23 ++--
> libavcodec/hevcdsp.h | 14 +-
> libavcodec/hevcdsp_template.c | 313
> ++++++++++++++++++------------------------
> libavcodec/x86/Makefile | 3 +-
> libavcodec/x86/hevc_idct.asm | 106 ++++++++++++++
> libavcodec/x86/hevcdsp_init.c | 57 ++++++++
> libavutil/x86/x86util.asm | 4 +-
> 8 files changed, 347 insertions(+), 201 deletions(-)
> create mode 100644 libavcodec/x86/hevc_idct.asm
I would really prefer if this was split into separate commits for
- SPLATW for avx2
- reorganigizing the C code
- actually adding the new asm + changing coeffs alignment
Also, I wonder if this messing with col_limit has any measurable
effects.
>
> diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
> index 177cf93..babc448 100644
> --- a/libavcodec/hevc.c
> +++ b/libavcodec/hevc.c
> @@ -897,7 +897,7 @@ static void hls_residual_coding(HEVCContext *s, int x0,
> int y0,
> int vshift = s->ps.sps->vshift[c_idx];
> uint8_t *dst = &s->frame->data[c_idx][(y0 >> vshift) * stride +
> ((x0 >> hshift) <<
> s->ps.sps->pixel_shift)];
> - DECLARE_ALIGNED(16, int16_t, coeffs[MAX_TB_SIZE * MAX_TB_SIZE]) = { 0 };
> + DECLARE_ALIGNED(32, int16_t, coeffs[MAX_TB_SIZE * MAX_TB_SIZE]) = { 0 };
> DECLARE_ALIGNED(8, uint8_t, significant_coeff_group_flag[8][8]) = { { 0
> } };
>
> int trafo_size = 1 << log2_trafo_size;
> @@ -1205,17 +1205,29 @@ static void hls_residual_coding(HEVCContext *s, int
> x0, int y0,
> }
> }
>
> - if (lc->cu.cu_transquant_bypass_flag) {
> - s->hevcdsp.transquant_bypass[log2_trafo_size - 2](dst, coeffs,
> stride);
> - } else {
> + if (!lc->cu.cu_transquant_bypass_flag) {
> if (transform_skip_flag)
> - s->hevcdsp.transform_skip(dst, coeffs, stride);
> + s->hevcdsp.transform_skip(coeffs, log2_trafo_size);
> else if (lc->cu.pred_mode == MODE_INTRA && c_idx == 0 &&
> log2_trafo_size == 2)
> - s->hevcdsp.transform_4x4_luma_add(dst, coeffs, stride);
> - else
> - s->hevcdsp.transform_add[log2_trafo_size - 2](dst, coeffs,
> stride);
> + s->hevcdsp.idct_4x4_luma(coeffs);
> + else {
> + int max_xy = FFMAX(last_significant_coeff_x,
> last_significant_coeff_y);
> + if (max_xy == 0)
> + s->hevcdsp.idct_dc[log2_trafo_size-2](coeffs);
> + else {
> + int col_limit = last_significant_coeff_x +
> last_significant_coeff_y + 4;
> + if (max_xy < 4)
> + col_limit = FFMIN(4, col_limit);
> + else if (max_xy < 8)
> + col_limit = FFMIN(8, col_limit);
> + else if (max_xy < 12)
> + col_limit = FFMIN(24, col_limit);
> + s->hevcdsp.idct[log2_trafo_size-2](coeffs, col_limit);
> + }
> + }
> }
> + s->hevcdsp.transform_add[log2_trafo_size-2](dst, coeffs, stride);
> }
>
> static int hls_transform_unit(HEVCContext *s, int x0, int y0,
> diff --git a/libavcodec/hevcdsp.c b/libavcodec/hevcdsp.c
> index 15a712d..6b4b97c 100644
> --- a/libavcodec/hevcdsp.c
> +++ b/libavcodec/hevcdsp.c
> @@ -164,16 +164,21 @@ void ff_hevc_dsp_init(HEVCDSPContext *hevcdsp, int
> bit_depth)
>
> #define HEVC_DSP(depth) \
> hevcdsp->put_pcm = FUNC(put_pcm, depth); \
> - hevcdsp->transquant_bypass[0] = FUNC(transquant_bypass4x4, depth); \
> - hevcdsp->transquant_bypass[1] = FUNC(transquant_bypass8x8, depth); \
> - hevcdsp->transquant_bypass[2] = FUNC(transquant_bypass16x16, depth); \
> - hevcdsp->transquant_bypass[3] = FUNC(transquant_bypass32x32, depth); \
> + hevcdsp->transform_add[0] = FUNC(transform_add4x4, depth); \
> + hevcdsp->transform_add[1] = FUNC(transform_add8x8, depth); \
> + hevcdsp->transform_add[2] = FUNC(transform_add16x16, depth); \
> + hevcdsp->transform_add[3] = FUNC(transform_add32x32, depth); \
> hevcdsp->transform_skip = FUNC(transform_skip, depth); \
> - hevcdsp->transform_4x4_luma_add = FUNC(transform_4x4_luma_add, depth); \
> - hevcdsp->transform_add[0] = FUNC(transform_4x4_add, depth); \
> - hevcdsp->transform_add[1] = FUNC(transform_8x8_add, depth); \
> - hevcdsp->transform_add[2] = FUNC(transform_16x16_add, depth); \
> - hevcdsp->transform_add[3] = FUNC(transform_32x32_add, depth); \
> + hevcdsp->idct_4x4_luma = FUNC(transform_4x4_luma, depth); \
> + hevcdsp->idct[0] = FUNC(idct_4x4, depth); \
> + hevcdsp->idct[1] = FUNC(idct_8x8, depth); \
> + hevcdsp->idct[2] = FUNC(idct_16x16, depth); \
> + hevcdsp->idct[3] = FUNC(idct_32x32, depth); \
> + \
> + hevcdsp->idct_dc[0] = FUNC(idct_4x4_dc, depth); \
> + hevcdsp->idct_dc[1] = FUNC(idct_8x8_dc, depth); \
> + hevcdsp->idct_dc[2] = FUNC(idct_16x16_dc, depth); \
> + hevcdsp->idct_dc[3] = FUNC(idct_32x32_dc, depth); \
> \
> hevcdsp->sao_band_filter[0] = FUNC(sao_band_filter_0, depth); \
> hevcdsp->sao_band_filter[1] = FUNC(sao_band_filter_1, depth); \
> diff --git a/libavcodec/hevcdsp.h b/libavcodec/hevcdsp.h
> index 4097233..1793893 100644
> --- a/libavcodec/hevcdsp.h
> +++ b/libavcodec/hevcdsp.h
> @@ -42,13 +42,15 @@ typedef struct HEVCDSPContext {
> void (*put_pcm)(uint8_t *dst, ptrdiff_t stride, int size,
> GetBitContext *gb, int pcm_bit_depth);
>
> - void (*transquant_bypass[4])(uint8_t *dst, int16_t *coeffs,
> - ptrdiff_t stride);
> + void (*transform_add[4])(uint8_t *_dst, int16_t *coeffs, ptrdiff_t
> _stride);
>
> - void (*transform_skip)(uint8_t *dst, int16_t *coeffs, ptrdiff_t stride);
> - void (*transform_4x4_luma_add)(uint8_t *dst, int16_t *coeffs,
> - ptrdiff_t stride);
> - void (*transform_add[4])(uint8_t *dst, int16_t *coeffs, ptrdiff_t
> stride);
> + void (*transform_skip)(int16_t *coeffs, int16_t log2_size);
> +
> + void (*idct_4x4_luma)(int16_t *coeffs);
> +
> + void (*idct[4])(int16_t *coeffs, int col_limit);
> +
> + void (*idct_dc[4])(int16_t *coeffs);
>
> void (*sao_band_filter[4])(uint8_t *dst, uint8_t *src, ptrdiff_t stride,
> struct SAOParams *sao, int *borders,
> diff --git a/libavcodec/hevcdsp_template.c b/libavcodec/hevcdsp_template.c
> index 31a2e7a..3846327 100644
> --- a/libavcodec/hevcdsp_template.c
> +++ b/libavcodec/hevcdsp_template.c
> @@ -57,48 +57,53 @@ static av_always_inline void
> FUNC(transquant_bypass)(uint8_t *_dst, int16_t *coe
> }
> }
>
> -static void FUNC(transquant_bypass4x4)(uint8_t *_dst, int16_t *coeffs,
> +static void FUNC(transform_add4x4)(uint8_t *_dst, int16_t *coeffs,
> ptrdiff_t stride)
> {
> FUNC(transquant_bypass)(_dst, coeffs, stride, 4);
This function should no longer be called transquant_bypass then.
> }
>
> -static void FUNC(transquant_bypass8x8)(uint8_t *_dst, int16_t *coeffs,
> +static void FUNC(transform_add8x8)(uint8_t *_dst, int16_t *coeffs,
> ptrdiff_t stride)
> {
> FUNC(transquant_bypass)(_dst, coeffs, stride, 8);
> }
>
> -static void FUNC(transquant_bypass16x16)(uint8_t *_dst, int16_t *coeffs,
> +static void FUNC(transform_add16x16)(uint8_t *_dst, int16_t *coeffs,
> ptrdiff_t stride)
> {
> FUNC(transquant_bypass)(_dst, coeffs, stride, 16);
> }
>
> -static void FUNC(transquant_bypass32x32)(uint8_t *_dst, int16_t *coeffs,
> +static void FUNC(transform_add32x32)(uint8_t *_dst, int16_t *coeffs,
> ptrdiff_t stride)
> {
> FUNC(transquant_bypass)(_dst, coeffs, stride, 32);
> }
>
> -static void FUNC(transform_skip)(uint8_t *_dst, int16_t *coeffs,
> - ptrdiff_t stride)
> +static void FUNC(transform_skip)(int16_t *_coeffs, int16_t log2_size)
This function previously operated on fixed size 4x4 blocks, why is it
changed into variable size? The code looked simpler as well.
> {
> - pixel *dst = (pixel *)_dst;
> - int shift = 13 - BIT_DEPTH;
> -#if BIT_DEPTH <= 13
> - int offset = 1 << (shift - 1);
> -#else
> - int offset = 0;
> -#endif
> + int shift = 15 - BIT_DEPTH - log2_size;
> int x, y;
> + int size = 1 << log2_size;
> + int16_t *coeffs = _coeffs;
This assignment is pointless.
>
> - stride /= sizeof(pixel);
>
> - for (y = 0; y < 4 * 4; y += 4) {
> - for (x = 0; x < 4; x++)
> - dst[x] = av_clip_pixel(dst[x] + ((coeffs[y + x] + offset) >>
> shift));
> - dst += stride;
> + if (shift > 0) {
> + int offset = 1 << (shift - 1);
> + for (y = 0; y < size; y++) {
> + for (x = 0; x < size; x++) {
> + *coeffs = (*coeffs + offset) >> shift;
> + coeffs++;
> + }
> + }
> + } else {
> + for (y = 0; y < size; y++) {
> + for (x = 0; x < size; x++) {
> + *coeffs = *coeffs << -shift;
> + coeffs++;
> + }
> + }
> }
> }
>
> @@ -122,17 +127,13 @@ static void FUNC(transform_skip)(uint8_t *_dst, int16_t
> *coeffs,
> assign(dst[3 * step], 55 * c0 + 29 * c2 - c3); \
> } while (0)
>
> -static void FUNC(transform_4x4_luma_add)(uint8_t *_dst, int16_t *coeffs,
> - ptrdiff_t stride)
> +static void FUNC(transform_4x4_luma)(int16_t *coeffs)
> {
> int i;
> - pixel *dst = (pixel *)_dst;
> int shift = 7;
> int add = 1 << (shift - 1);
> int16_t *src = coeffs;
>
> - stride /= sizeof(pixel);
> -
> for (i = 0; i < 4; i++) {
> TR_4x4_LUMA(src, src, 4, SCALE);
> src++;
> @@ -141,180 +142,140 @@ static void FUNC(transform_4x4_luma_add)(uint8_t
> *_dst, int16_t *coeffs,
> shift = 20 - BIT_DEPTH;
> add = 1 << (shift - 1);
> for (i = 0; i < 4; i++) {
> - TR_4x4_LUMA(dst, coeffs, 1, ADD_AND_SCALE);
> + TR_4x4_LUMA(coeffs, coeffs, 1, SCALE);
> coeffs += 4;
> - dst += stride;
> }
> }
>
> #undef TR_4x4_LUMA
>
> -#define TR_4(dst, src, dstep, sstep, assign) \
> - do { \
> - const int e0 = transform[8 * 0][0] * src[0 * sstep] + \
> - transform[8 * 2][0] * src[2 * sstep]; \
> - const int e1 = transform[8 * 0][1] * src[0 * sstep] + \
> - transform[8 * 2][1] * src[2 * sstep]; \
> - const int o0 = transform[8 * 1][0] * src[1 * sstep] + \
> - transform[8 * 3][0] * src[3 * sstep]; \
> - const int o1 = transform[8 * 1][1] * src[1 * sstep] + \
> - transform[8 * 3][1] * src[3 * sstep]; \
> - \
> - assign(dst[0 * dstep], e0 + o0); \
> - assign(dst[1 * dstep], e1 + o1); \
> - assign(dst[2 * dstep], e1 - o1); \
> - assign(dst[3 * dstep], e0 - o0); \
> +#define TR_4(dst, src, dstep, sstep, assign, end)
> \
> + do {
> \
> + const int e0 = 64 * src[0 * sstep] + 64 * src[2 * sstep];
> \
> + const int e1 = 64 * src[0 * sstep] - 64 * src[2 * sstep];
> \
> + const int o0 = 83 * src[1 * sstep] + 36 * src[3 * sstep];
> \
> + const int o1 = 36 * src[1 * sstep] - 83 * src[3 * sstep];
> \
> +
> \
> + assign(dst[0 * dstep], e0 + o0);
> \
> + assign(dst[1 * dstep], e1 + o1);
> \
> + assign(dst[2 * dstep], e1 - o1);
> \
> + assign(dst[3 * dstep], e0 - o0);
> \
> } while (0)
This does a bunch of unrelated things. The only relevant change is
adding the 'end' parameter, everything else should be either dropped or
go into a separate commit.
>
> -static void FUNC(transform_4x4_add)(uint8_t *_dst, int16_t *coeffs,
> - ptrdiff_t stride)
> -{
> - int i;
> - pixel *dst = (pixel *)_dst;
> - int shift = 7;
> - int add = 1 << (shift - 1);
> - int16_t *src = coeffs;
> -
> - stride /= sizeof(pixel);
> -
> - for (i = 0; i < 4; i++) {
> - TR_4(src, src, 4, 4, SCALE);
> - src++;
> - }
> -
> - shift = 20 - BIT_DEPTH;
> - add = 1 << (shift - 1);
> - for (i = 0; i < 4; i++) {
> - TR_4(dst, coeffs, 1, 1, ADD_AND_SCALE);
> - coeffs += 4;
> - dst += stride;
> - }
> -}
> -
> -#define TR_8(dst, src, dstep, sstep, assign) \
> - do { \
> - int i, j; \
> - int e_8[4]; \
> - int o_8[4] = { 0 }; \
> - for (i = 0; i < 4; i++) \
> - for (j = 1; j < 8; j += 2) \
> - o_8[i] += transform[4 * j][i] * src[j * sstep]; \
> - TR_4(e_8, src, 1, 2 * sstep, SET); \
> - \
> - for (i = 0; i < 4; i++) { \
> - assign(dst[i * dstep], e_8[i] + o_8[i]); \
> - assign(dst[(7 - i) * dstep], e_8[i] - o_8[i]); \
> - } \
> +#define TR_8(dst, src, dstep, sstep, assign, end)
> \
> + do {
> \
> + int i, j;
> \
> + int e_8[4];
> \
> + int o_8[4] = { 0 };
> \
> + for (i = 0; i < 4; i++)
> \
> + for (j = 1; j < end; j += 2)
> \
> + o_8[i] += transform[4 * j][i] * src[j * sstep];
> \
> + TR_4(e_8, src, 1, 2 * sstep, SET, 4);
> \
> +
> \
> + for (i = 0; i < 4; i++) {
> \
> + assign(dst[i * dstep], e_8[i] + o_8[i]);
> \
> + assign(dst[(7 - i) * dstep], e_8[i] - o_8[i]);
> \
> + }
> \
The trailing backslashes are reindented for no reason, making the diff
harder to read. Same for the macros below.
And I really have to wonder if this 'end' parameter makes things any
faster.
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
