[FFmpeg-devel] [PATCH] lavc/dxv: align to 4x4 blocks instead of 16x16

[Andreas Rheinhardt]

Connor Worley:
> The previous assumption that DXV needs to be aligned to 16x16 was
> erroneous. 4x4 works just as well, and FATE decoder tests pass for all
> texture formats.
> 
> On the encoder side, we should reject input that isn't 4x4 aligned,
> like the HAP encoder does, and stop aligning to 16x16. This both solves
> the uninitialized reads causing current FATE tests to fail and produces
> smaller encoded outputs.

Given that this is a lossy format, one also has to investigate whether
the quality is affected.

> 
> Signed-off-by: Connor Worley <connorbworley at gmail.com>
> ---
>  libavcodec/dxv.c            |  6 +++---
>  libavcodec/dxvenc.c         | 13 ++++++++++---
>  tests/ref/fate/dxv3enc-dxt1 |  2 +-
>  3 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/libavcodec/dxv.c b/libavcodec/dxv.c
> index e1c7cee3e8..9261a5cac1 100644
> --- a/libavcodec/dxv.c
> +++ b/libavcodec/dxv.c
> @@ -1238,9 +1238,9 @@ static int dxv_init(AVCodecContext *avctx)
>          return ret;
>      }
>  
> -    /* Codec requires 16x16 alignment. */
> -    avctx->coded_width  = FFALIGN(avctx->width,  16);
> -    avctx->coded_height = FFALIGN(avctx->height, 16);
> +    /* Since codec is based on 4x4 blocks, size is aligned to 4 */
> +    avctx->coded_width  = FFALIGN(avctx->width,  TEXTURE_BLOCK_W);
> +    avctx->coded_height = FFALIGN(avctx->height, TEXTURE_BLOCK_H);
>  
>      ff_texturedsp_init(&ctx->texdsp);
>  
> diff --git a/libavcodec/dxvenc.c b/libavcodec/dxvenc.c
> index ebc48aace3..3023a6da6c 100644
> --- a/libavcodec/dxvenc.c
> +++ b/libavcodec/dxvenc.c
> @@ -216,6 +216,13 @@ static av_cold int dxv_init(AVCodecContext *avctx)
>          return ret;
>      }
>  
> +    if (avctx->width % TEXTURE_BLOCK_W || avctx->height % TEXTURE_BLOCK_H) {
> +        av_log(avctx, AV_LOG_ERROR, "Video size %dx%d is not multiple of %dx%d.\n",
> +               avctx->width, avctx->height,
> +               TEXTURE_BLOCK_W, TEXTURE_BLOCK_H);

There is really no reason to pass this via arguments.

> +        return AVERROR_INVALIDDATA;
> +    }
> +
>      ff_texturedspenc_init(&texdsp);
>  
>      switch (ctx->tex_fmt) {
> @@ -229,10 +236,10 @@ static av_cold int dxv_init(AVCodecContext *avctx)
>          return AVERROR_INVALIDDATA;
>      }
>      ctx->enc.raw_ratio = 16;
> -    ctx->tex_size = FFALIGN(avctx->width, 16) / TEXTURE_BLOCK_W *
> -                    FFALIGN(avctx->height, 16) / TEXTURE_BLOCK_H *
> +    ctx->tex_size = avctx->width  / TEXTURE_BLOCK_W *
> +                    avctx->height / TEXTURE_BLOCK_H *
>                      ctx->enc.tex_ratio;
> -    ctx->enc.slice_count = av_clip(avctx->thread_count, 1, FFALIGN(avctx->height, 16) / TEXTURE_BLOCK_H);
> +    ctx->enc.slice_count = av_clip(avctx->thread_count, 1, avctx->height / TEXTURE_BLOCK_H);
>  
>      ctx->tex_data = av_malloc(ctx->tex_size);
>      if (!ctx->tex_data) {
> diff --git a/tests/ref/fate/dxv3enc-dxt1 b/tests/ref/fate/dxv3enc-dxt1
> index 3cfd73397e..74849a8031 100644
> --- a/tests/ref/fate/dxv3enc-dxt1
> +++ b/tests/ref/fate/dxv3enc-dxt1
> @@ -3,4 +3,4 @@
>  #codec_id 0: dxv
>  #dimensions 0: 1920x1080
>  #sar 0: 1/1
> -0,          0,          0,        1,    76767, 0x932ecbfa
> +0,          0,          0,        1,    76521, 0xed387a5e