| Message ID | 20240209112649.16556-1-connorbworley@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2] lavc/dxv: align to 4x4 blocks instead of 16x16 | 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 Fri, 9 Feb 2024, Connor Worley wrote: > 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. > > With regard to correctness, I've checked the decoding path by encoding a > real-world sample with git master, and decoding it with > ffmpeg -i dxt1-master.mov -c:v rawvideo -f framecrc - > The results are exactly the same between master and this patch. > > On the encoding side, I've encoded a real-world sample with both master > and this patch, and decoded both versions with > ffmpeg -i dxt1-{master,patch}.mov -c:v rawvideo -f framecrc - > Under this patch, results for both inputs are exactly the same. > > In other words, the extra padding gained by 16x16 alignment over 4x4 > alignment has no impact on decoded video. > > Signed-off-by: Connor Worley <connorbworley@gmail.com> > --- > libavcodec/dxv.c | 6 +++--- > libavcodec/dxvenc.c | 14 +++++++++++--- > tests/ref/fate/dxv3enc-dxt1 | 2 +- > 3 files changed, 15 insertions(+), 7 deletions(-) LGTM, will push soon to get FATE back to green again. // Martin
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 b274175689..33a18d53d8 100644 --- a/libavcodec/dxvenc.c +++ b/libavcodec/dxvenc.c @@ -275,6 +275,14 @@ 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 "AV_STRINGIFY(TEXTURE_BLOCK_W)"x"AV_STRINGIFY(TEXTURE_BLOCK_H)".\n", + avctx->width, avctx->height); + return AVERROR_INVALIDDATA; + } + ff_texturedspenc_init(&texdsp); switch (ctx->tex_fmt) { @@ -288,10 +296,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
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. With regard to correctness, I've checked the decoding path by encoding a real-world sample with git master, and decoding it with ffmpeg -i dxt1-master.mov -c:v rawvideo -f framecrc - The results are exactly the same between master and this patch. On the encoding side, I've encoded a real-world sample with both master and this patch, and decoded both versions with ffmpeg -i dxt1-{master,patch}.mov -c:v rawvideo -f framecrc - Under this patch, results for both inputs are exactly the same. In other words, the extra padding gained by 16x16 alignment over 4x4 alignment has no impact on decoded video. Signed-off-by: Connor Worley <connorbworley@gmail.com> --- libavcodec/dxv.c | 6 +++--- libavcodec/dxvenc.c | 14 +++++++++++--- tests/ref/fate/dxv3enc-dxt1 | 2 +- 3 files changed, 15 insertions(+), 7 deletions(-) -- 2.40.1