diff mbox series

[FFmpeg-devel,07/39] lavc/ffv1: add a per-slice context

Message ID 20240716171155.31838-7-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,01/39] tests/fate/vcodec: add vsynth tests for FFV1 version 2 | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov July 16, 2024, 5:11 p.m. UTC
FFV1 decoder and encoder currently use the same struct - FFV1Context -
both as codec private data and per-slice context. For this purpose
FFV1Context contains an array of pointers to per-slice FFV1Context
instances.

This pattern is highly confusing, as it is not clear which fields are
per-slice and which per-codec.

Address this by adding a new struct storing only per-slice data. Start
by moving slice_{x,y,width,height} to it.
---
 libavcodec/ffv1.c    | 15 ++++++---
 libavcodec/ffv1.h    | 13 +++++---
 libavcodec/ffv1dec.c | 76 ++++++++++++++++++++++++--------------------
 libavcodec/ffv1enc.c | 25 ++++++++-------
 4 files changed, 76 insertions(+), 53 deletions(-)

Comments

Michael Niedermayer July 24, 2024, 7:01 p.m. UTC | #1
On Tue, Jul 16, 2024 at 07:11:22PM +0200, Anton Khirnov wrote:
> FFV1 decoder and encoder currently use the same struct - FFV1Context -
> both as codec private data and per-slice context. For this purpose
> FFV1Context contains an array of pointers to per-slice FFV1Context
> instances.
> 
> This pattern is highly confusing, as it is not clear which fields are
> per-slice and which per-codec.
> 
> Address this by adding a new struct storing only per-slice data. Start
> by moving slice_{x,y,width,height} to it.
> ---
>  libavcodec/ffv1.c    | 15 ++++++---
>  libavcodec/ffv1.h    | 13 +++++---
>  libavcodec/ffv1dec.c | 76 ++++++++++++++++++++++++--------------------
>  libavcodec/ffv1enc.c | 25 ++++++++-------
>  4 files changed, 76 insertions(+), 53 deletions(-)

LGTM

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/ffv1.c b/libavcodec/ffv1.c
index 6ec24fed4a..25f28287c0 100644
--- a/libavcodec/ffv1.c
+++ b/libavcodec/ffv1.c
@@ -108,7 +108,12 @@  av_cold int ff_ffv1_init_slice_contexts(FFV1Context *f)
 
     av_assert0(max_slice_count > 0);
 
+    f->slices = av_calloc(max_slice_count, sizeof(*f->slices));
+    if (!f->slices)
+        return AVERROR(ENOMEM);
+
     for (i = 0; i < max_slice_count;) {
+        FFV1SliceContext *sc = &f->slices[i];
         int sx          = i % f->num_h_slices;
         int sy          = i / f->num_h_slices;
         int sxs         = f->avctx->width  *  sx      / f->num_h_slices;
@@ -124,10 +129,10 @@  av_cold int ff_ffv1_init_slice_contexts(FFV1Context *f)
         memcpy(fs, f, sizeof(*fs));
         memset(fs->rc_stat2, 0, sizeof(fs->rc_stat2));
 
-        fs->slice_width  = sxe - sxs;
-        fs->slice_height = sye - sys;
-        fs->slice_x      = sxs;
-        fs->slice_y      = sys;
+        sc->slice_width  = sxe - sxs;
+        sc->slice_height = sye - sys;
+        sc->slice_x      = sxs;
+        sc->slice_y      = sys;
 
         fs->sample_buffer = av_malloc_array((fs->width + 6), 3 * MAX_PLANES *
                                       sizeof(*fs->sample_buffer));
@@ -217,5 +222,7 @@  av_cold int ff_ffv1_close(AVCodecContext *avctx)
     for (i = 0; i < s->max_slice_count; i++)
         av_freep(&s->slice_context[i]);
 
+    av_freep(&s->slices);
+
     return 0;
 }
diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h
index d99367ce81..256904b283 100644
--- a/libavcodec/ffv1.h
+++ b/libavcodec/ffv1.h
@@ -69,6 +69,13 @@  typedef struct PlaneContext {
 
 #define MAX_SLICES 1024
 
+typedef struct FFV1SliceContext {
+    int slice_width;
+    int slice_height;
+    int slice_x;
+    int slice_y;
+} FFV1SliceContext;
+
 typedef struct FFV1Context {
     AVClass *class;
     AVCodecContext *avctx;
@@ -123,14 +130,12 @@  typedef struct FFV1Context {
     int max_slice_count;
     int num_v_slices;
     int num_h_slices;
-    int slice_width;
-    int slice_height;
-    int slice_x;
-    int slice_y;
     int slice_reset_contexts;
     int slice_coding_mode;
     int slice_rct_by_coef;
     int slice_rct_ry_coef;
+
+    FFV1SliceContext *slices;
 } FFV1Context;
 
 int ff_ffv1_common_init(AVCodecContext *avctx);
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index 6d59355c23..28e4a05b21 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -164,7 +164,7 @@  static int decode_plane(FFV1Context *s, uint8_t *src,
 }
 
 static int decode_slice_header(const FFV1Context *f, FFV1Context *fs,
-                               AVFrame *frame)
+                               FFV1SliceContext *sc, AVFrame *frame)
 {
     RangeCoder *c = &fs->c;
     uint8_t state[CONTEXT_SIZE];
@@ -185,17 +185,17 @@  static int decode_slice_header(const FFV1Context *f, FFV1Context *fs,
     if (sx > f->num_h_slices - sw || sy > f->num_v_slices - sh)
         return AVERROR_INVALIDDATA;
 
-    fs->slice_x      =  sx       * (int64_t)f->width  / f->num_h_slices;
-    fs->slice_y      =  sy       * (int64_t)f->height / f->num_v_slices;
-    fs->slice_width  = (sx + sw) * (int64_t)f->width  / f->num_h_slices - fs->slice_x;
-    fs->slice_height = (sy + sh) * (int64_t)f->height / f->num_v_slices - fs->slice_y;
+    sc->slice_x      =  sx       * (int64_t)f->width  / f->num_h_slices;
+    sc->slice_y      =  sy       * (int64_t)f->height / f->num_v_slices;
+    sc->slice_width  = (sx + sw) * (int64_t)f->width  / f->num_h_slices - sc->slice_x;
+    sc->slice_height = (sy + sh) * (int64_t)f->height / f->num_v_slices - sc->slice_y;
 
-    av_assert0((unsigned)fs->slice_width  <= f->width &&
-                (unsigned)fs->slice_height <= f->height);
-    av_assert0 (   (unsigned)fs->slice_x + (uint64_t)fs->slice_width  <= f->width
-                && (unsigned)fs->slice_y + (uint64_t)fs->slice_height <= f->height);
+    av_assert0((unsigned)sc->slice_width  <= f->width &&
+                (unsigned)sc->slice_height <= f->height);
+    av_assert0 (   (unsigned)sc->slice_x + (uint64_t)sc->slice_width  <= f->width
+                && (unsigned)sc->slice_y + (uint64_t)sc->slice_height <= f->height);
 
-    if (fs->ac == AC_GOLOMB_RICE && fs->slice_width >= (1<<23))
+    if (fs->ac == AC_GOLOMB_RICE && sc->slice_width >= (1<<23))
         return AVERROR_INVALIDDATA;
 
     for (unsigned i = 0; i < f->plane_count; i++) {
@@ -261,6 +261,7 @@  static int decode_slice(AVCodecContext *c, void *arg)
     const int ps      = av_pix_fmt_desc_get(c->pix_fmt)->comp[0].step;
     AVFrame * const p = f->picture.f;
     const int      si = (FFV1Context**)arg - f->slice_context;
+    FFV1SliceContext *sc = &f->slices[si];
 
     if (f->fsrc && !(p->flags & AV_FRAME_FLAG_KEY) && f->last_picture.f)
         ff_progress_frame_await(&f->last_picture, si);
@@ -298,8 +299,8 @@  static int decode_slice(AVCodecContext *c, void *arg)
     if (f->version > 2) {
         if (ff_ffv1_init_slice_state(f, fs) < 0)
             return AVERROR(ENOMEM);
-        if (decode_slice_header(f, fs, p) < 0) {
-            fs->slice_x = fs->slice_y = fs->slice_height = fs->slice_width = 0;
+        if (decode_slice_header(f, fs, sc, p) < 0) {
+            sc->slice_x = sc->slice_y = sc->slice_height = sc->slice_width = 0;
             fs->slice_damaged = 1;
             return AVERROR_INVALIDDATA;
         }
@@ -312,10 +313,10 @@  static int decode_slice(AVCodecContext *c, void *arg)
         return AVERROR_INVALIDDATA;
     }
 
-    width  = fs->slice_width;
-    height = fs->slice_height;
-    x      = fs->slice_x;
-    y      = fs->slice_y;
+    width  = sc->slice_width;
+    height = sc->slice_height;
+    x      = sc->slice_x;
+    y      = sc->slice_y;
 
     if (fs->ac == AC_GOLOMB_RICE) {
         if (f->version == 3 && f->micro_version > 1 || f->version > 3)
@@ -788,6 +789,7 @@  static int read_header(FFV1Context *f)
 
     for (int j = 0; j < f->slice_count; j++) {
         FFV1Context *fs = f->slice_context[j];
+        FFV1SliceContext *sc = &f->slices[j];
         fs->ac            = f->ac;
         fs->packed_at_lsb = f->packed_at_lsb;
 
@@ -804,15 +806,15 @@  static int read_header(FFV1Context *f)
             if (sx > f->num_h_slices - sw || sy > f->num_v_slices - sh)
                 return AVERROR_INVALIDDATA;
 
-            fs->slice_x      =  sx       * (int64_t)f->width  / f->num_h_slices;
-            fs->slice_y      =  sy       * (int64_t)f->height / f->num_v_slices;
-            fs->slice_width  = (sx + sw) * (int64_t)f->width  / f->num_h_slices - fs->slice_x;
-            fs->slice_height = (sy + sh) * (int64_t)f->height / f->num_v_slices - fs->slice_y;
+            sc->slice_x      =  sx       * (int64_t)f->width  / f->num_h_slices;
+            sc->slice_y      =  sy       * (int64_t)f->height / f->num_v_slices;
+            sc->slice_width  = (sx + sw) * (int64_t)f->width  / f->num_h_slices - sc->slice_x;
+            sc->slice_height = (sy + sh) * (int64_t)f->height / f->num_v_slices - sc->slice_y;
 
-            av_assert0((unsigned)fs->slice_width  <= f->width &&
-                       (unsigned)fs->slice_height <= f->height);
-            av_assert0 (   (unsigned)fs->slice_x + (uint64_t)fs->slice_width  <= f->width
-                        && (unsigned)fs->slice_y + (uint64_t)fs->slice_height <= f->height);
+            av_assert0((unsigned)sc->slice_width  <= f->width &&
+                       (unsigned)sc->slice_height <= f->height);
+            av_assert0 (   (unsigned)sc->slice_x + (uint64_t)sc->slice_width  <= f->width
+                        && (unsigned)sc->slice_y + (uint64_t)sc->slice_height <= f->height);
         }
 
         for (int i = 0; i < f->plane_count; i++) {
@@ -990,6 +992,7 @@  static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
 
     for (int i = f->slice_count - 1; i >= 0; i--) {
         FFV1Context *fs = f->slice_context[i];
+        FFV1SliceContext *sc = &f->slices[i];
         if (fs->slice_damaged && f->last_picture.f) {
             const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(avctx->pix_fmt);
             const uint8_t *src[4];
@@ -1000,9 +1003,9 @@  static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
                 int sh = (j == 1 || j == 2) ? f->chroma_h_shift : 0;
                 int sv = (j == 1 || j == 2) ? f->chroma_v_shift : 0;
                 dst[j] = p->data[j] + p->linesize[j] *
-                         (fs->slice_y >> sv) + ((fs->slice_x >> sh) << pixshift);
+                         (sc->slice_y >> sv) + ((sc->slice_x >> sh) << pixshift);
                 src[j] = f->last_picture.f->data[j] + f->last_picture.f->linesize[j] *
-                         (fs->slice_y >> sv) + ((fs->slice_x >> sh) << pixshift);
+                         (sc->slice_y >> sv) + ((sc->slice_x >> sh) << pixshift);
 
             }
             if (desc->flags & AV_PIX_FMT_FLAG_PAL) {
@@ -1012,8 +1015,8 @@  static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
             av_image_copy(dst, p->linesize, src,
                           f->last_picture.f->linesize,
                           avctx->pix_fmt,
-                          fs->slice_width,
-                          fs->slice_height);
+                          sc->slice_width,
+                          sc->slice_height);
         }
     }
     ff_progress_frame_report(&f->picture, INT_MAX);
@@ -1048,12 +1051,6 @@  static void copy_fields(FFV1Context *fsdst, const FFV1Context *fssrc,
 
     fsdst->packed_at_lsb       = fsrc->packed_at_lsb;
     fsdst->slice_count         = fsrc->slice_count;
-    if (fsrc->version<3){
-        fsdst->slice_x             = fssrc->slice_x;
-        fsdst->slice_y             = fssrc->slice_y;
-        fsdst->slice_width         = fssrc->slice_width;
-        fsdst->slice_height        = fssrc->slice_height;
-    }
 }
 
 static int update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
@@ -1073,7 +1070,18 @@  static int update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
     for (int i = 0; i < fdst->num_h_slices * fdst->num_v_slices; i++) {
         FFV1Context *fssrc = fsrc->slice_context[i];
         FFV1Context *fsdst = fdst->slice_context[i];
+
+        FFV1SliceContext       *sc  = &fdst->slices[i];
+        const FFV1SliceContext *sc0 = &fsrc->slices[i];
+
         copy_fields(fsdst, fssrc, fsrc);
+
+        if (fsrc->version < 3) {
+            sc->slice_x             = sc0->slice_x;
+            sc->slice_y             = sc0->slice_y;
+            sc->slice_width         = sc0->slice_width;
+            sc->slice_height        = sc0->slice_height;
+        }
     }
     av_assert0(!fdst->plane[0].state);
     av_assert0(!fdst->sample_buffer);
diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c
index 94d9215acd..c46df15b0c 100644
--- a/libavcodec/ffv1enc.c
+++ b/libavcodec/ffv1enc.c
@@ -370,7 +370,7 @@  static void write_header(FFV1Context *f)
     } else if (f->version < 3) {
         put_symbol(c, state, f->slice_count, 0);
         for (i = 0; i < f->slice_count; i++) {
-            FFV1Context *fs = f->slice_context[i];
+            FFV1SliceContext *fs = &f->slices[i];
             put_symbol(c, state,
                        (fs->slice_x      + 1) * f->num_h_slices / f->width, 0);
             put_symbol(c, state,
@@ -904,17 +904,18 @@  slices_ok:
     return 0;
 }
 
-static void encode_slice_header(FFV1Context *f, FFV1Context *fs)
+static void encode_slice_header(FFV1Context *f, FFV1Context *fs,
+                                FFV1SliceContext *sc)
 {
     RangeCoder *c = &fs->c;
     uint8_t state[CONTEXT_SIZE];
     int j;
     memset(state, 128, sizeof(state));
 
-    put_symbol(c, state, (fs->slice_x     +1)*f->num_h_slices / f->width   , 0);
-    put_symbol(c, state, (fs->slice_y     +1)*f->num_v_slices / f->height  , 0);
-    put_symbol(c, state, (fs->slice_width +1)*f->num_h_slices / f->width -1, 0);
-    put_symbol(c, state, (fs->slice_height+1)*f->num_v_slices / f->height-1, 0);
+    put_symbol(c, state, (sc->slice_x     +1)*f->num_h_slices / f->width   , 0);
+    put_symbol(c, state, (sc->slice_y     +1)*f->num_v_slices / f->height  , 0);
+    put_symbol(c, state, (sc->slice_width +1)*f->num_h_slices / f->width -1, 0);
+    put_symbol(c, state, (sc->slice_height+1)*f->num_v_slices / f->height-1, 0);
     for (j=0; j<f->plane_count; j++) {
         put_symbol(c, state, f->plane[j].quant_table_index, 0);
         av_assert0(f->plane[j].quant_table_index == f->context_model);
@@ -1023,10 +1024,12 @@  static int encode_slice(AVCodecContext *c, void *arg)
 {
     FFV1Context *fs  = *(void **)arg;
     FFV1Context *f   = fs->avctx->priv_data;
-    int width        = fs->slice_width;
-    int height       = fs->slice_height;
-    int x            = fs->slice_x;
-    int y            = fs->slice_y;
+    const int     si = (FFV1Context**)arg - f->slice_context;
+    FFV1SliceContext *sc = &f->slices[si];
+    int width        = sc->slice_width;
+    int height       = sc->slice_height;
+    int x            = sc->slice_x;
+    int y            = sc->slice_y;
     const AVFrame *const p = f->cur_enc_frame;
     const int ps     = av_pix_fmt_desc_get(c->pix_fmt)->comp[0].step;
     int ret;
@@ -1048,7 +1051,7 @@  retry:
     if (f->key_frame)
         ff_ffv1_clear_slice_state(f, fs);
     if (f->version > 2) {
-        encode_slice_header(f, fs);
+        encode_slice_header(f, fs, sc);
     }
     if (fs->ac == AC_GOLOMB_RICE) {
         fs->ac_byte_count = f->version > 2 || (!x && !y) ? ff_rac_terminate(&fs->c, f->version > 2) : 0;