diff mbox series

[FFmpeg-devel,09/39] lavc/ffv1: move run_index to the per-slice context

Message ID 20240716171155.31838-9-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 
---
 libavcodec/ffv1.h             |  3 ++-
 libavcodec/ffv1dec.c          |  6 +++---
 libavcodec/ffv1dec_template.c | 16 ++++++++--------
 libavcodec/ffv1enc.c          |  6 +++---
 libavcodec/ffv1enc_template.c | 16 ++++++++--------
 5 files changed, 24 insertions(+), 23 deletions(-)

Comments

Michael Niedermayer July 17, 2024, 10:49 p.m. UTC | #1 
On Tue, Jul 16, 2024 at 07:11:24PM +0200, Anton Khirnov wrote:
> ---
>  libavcodec/ffv1.h             |  3 ++-
>  libavcodec/ffv1dec.c          |  6 +++---
>  libavcodec/ffv1dec_template.c | 16 ++++++++--------
>  libavcodec/ffv1enc.c          |  6 +++---
>  libavcodec/ffv1enc_template.c | 16 ++++++++--------
>  5 files changed, 24 insertions(+), 23 deletions(-)

more complex code

> @@ -140,13 +140,13 @@ static int decode_plane(FFV1Context *s, FFV1SliceContext *sc,
>          sample[0][w]  = sample[0][w - 1];
>  
>          if (s->avctx->bits_per_raw_sample <= 8) {
> -            int ret = decode_line(s, w, sample, plane_index, 8);
> +            int ret = decode_line(s, sc, w, sample, plane_index, 8);
>              if (ret < 0)
>                  return ret;
>              for (x = 0; x < w; x++)
>                  src[x*pixel_stride + stride * y] = sample[1][x];
>          } else {
> -            int ret = decode_line(s, w, sample, plane_index, s->avctx->bits_per_raw_sample);
> +            int ret = decode_line(s, sc, w, sample, plane_index, s->avctx->bits_per_raw_sample);
>              if (ret < 0)
>                  return ret;
>              if (s->packed_at_lsb) {

The need for passing an additional pointer around

seems to me a net negative

thx

[...]
Anton Khirnov July 18, 2024, 3:36 p.m. UTC | #2 
Quoting Michael Niedermayer (2024-07-18 00:49:41)
> The need for passing an additional pointer around
> 
> seems to me a net negative

It makes it explicitly clear which data is per-slice, and which
per-frame/global, which is much harder to figure out with current code.
Michael Niedermayer July 18, 2024, 5:41 p.m. UTC | #3 
On Thu, Jul 18, 2024 at 05:36:39PM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2024-07-18 00:49:41)
> > The need for passing an additional pointer around
> > 
> > seems to me a net negative
> 
> It makes it explicitly clear which data is per-slice, and which
> per-frame/global, which is much harder to figure out with current code.

if its not meassurably slower then its ok

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h
index ccb510a483..68d13a2964 100644
--- a/libavcodec/ffv1.h
+++ b/libavcodec/ffv1.h
@@ -77,6 +77,8 @@  typedef struct FFV1SliceContext {
     int slice_height;
     int slice_x;
     int slice_y;
+
+    int run_index;
 } FFV1SliceContext;
 
 typedef struct FFV1Context {
@@ -109,7 +111,6 @@  typedef struct FFV1Context {
     int context_count[MAX_QUANT_TABLES];
     uint8_t state_transition[256];
     uint8_t (*initial_states[MAX_QUANT_TABLES])[32];
-    int run_index;
     int colorspace;
 
     int use32bit;
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index fcf8977a36..a2971d7eea 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -126,7 +126,7 @@  static int decode_plane(FFV1Context *s, FFV1SliceContext *sc,
     sample[0] = sc->sample_buffer + 3;
     sample[1] = sc->sample_buffer + w + 6 + 3;
 
-    s->run_index = 0;
+    sc->run_index = 0;
 
     memset(sc->sample_buffer, 0, 2 * (w + 6) * sizeof(*sc->sample_buffer));
 
@@ -140,13 +140,13 @@  static int decode_plane(FFV1Context *s, FFV1SliceContext *sc,
         sample[0][w]  = sample[0][w - 1];
 
         if (s->avctx->bits_per_raw_sample <= 8) {
-            int ret = decode_line(s, w, sample, plane_index, 8);
+            int ret = decode_line(s, sc, w, sample, plane_index, 8);
             if (ret < 0)
                 return ret;
             for (x = 0; x < w; x++)
                 src[x*pixel_stride + stride * y] = sample[1][x];
         } else {
-            int ret = decode_line(s, w, sample, plane_index, s->avctx->bits_per_raw_sample);
+            int ret = decode_line(s, sc, w, sample, plane_index, s->avctx->bits_per_raw_sample);
             if (ret < 0)
                 return ret;
             if (s->packed_at_lsb) {
diff --git a/libavcodec/ffv1dec_template.c b/libavcodec/ffv1dec_template.c
index b9316e04ca..8e2e38c0b9 100644
--- a/libavcodec/ffv1dec_template.c
+++ b/libavcodec/ffv1dec_template.c
@@ -22,16 +22,16 @@ 
 
 #include "ffv1_template.c"
 
-static av_always_inline int RENAME(decode_line)(FFV1Context *s, int w,
-                                                 TYPE *sample[2],
-                                                 int plane_index, int bits)
+static av_always_inline int
+RENAME(decode_line)(FFV1Context *s, FFV1SliceContext *sc, int w,
+                    TYPE *sample[2], int plane_index, int bits)
 {
     PlaneContext *const p = &s->plane[plane_index];
     RangeCoder *const c   = &s->c;
     int x;
     int run_count = 0;
     int run_mode  = 0;
-    int run_index = s->run_index;
+    int run_index = sc->run_index;
 
     if (is_input_end(s))
         return AVERROR_INVALIDDATA;
@@ -123,7 +123,7 @@  static av_always_inline int RENAME(decode_line)(FFV1Context *s, int w,
 
         sample[1][x] = av_zero_extend(RENAME(predict)(sample[1] + x, sample[0] + x) + (SUINT)diff, bits);
     }
-    s->run_index = run_index;
+    sc->run_index = run_index;
     return 0;
 }
 
@@ -142,7 +142,7 @@  static int RENAME(decode_rgb_frame)(FFV1Context *s, FFV1SliceContext *sc,
         sample[x][1] = RENAME(sc->sample_buffer) + (x * 2 + 1) * (w + 6) + 3;
     }
 
-    s->run_index = 0;
+    sc->run_index = 0;
 
     memset(RENAME(sc->sample_buffer), 0, 8 * (w + 6) * sizeof(*RENAME(sc->sample_buffer)));
 
@@ -157,9 +157,9 @@  static int RENAME(decode_rgb_frame)(FFV1Context *s, FFV1SliceContext *sc,
             sample[p][1][-1]= sample[p][0][0  ];
             sample[p][0][ w]= sample[p][0][w-1];
             if (lbd && s->slice_coding_mode == 0)
-                ret = RENAME(decode_line)(s, w, sample[p], (p + 1)/2, 9);
+                ret = RENAME(decode_line)(s, sc, w, sample[p], (p + 1)/2, 9);
             else
-                ret = RENAME(decode_line)(s, w, sample[p], (p + 1)/2, bits + (s->slice_coding_mode != 1));
+                ret = RENAME(decode_line)(s, sc, w, sample[p], (p + 1)/2, bits + (s->slice_coding_mode != 1));
             if (ret < 0)
                 return ret;
         }
diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c
index f6b1919ee4..8bb9174a1d 100644
--- a/libavcodec/ffv1enc.c
+++ b/libavcodec/ffv1enc.c
@@ -276,7 +276,7 @@  static int encode_plane(FFV1Context *s, FFV1SliceContext *sc,
     int x, y, i, ret;
     const int ring_size = s->context_model ? 3 : 2;
     int16_t *sample[3];
-    s->run_index = 0;
+    sc->run_index = 0;
 
     memset(sc->sample_buffer, 0, ring_size * (w + 6) * sizeof(*sc->sample_buffer));
 
@@ -289,7 +289,7 @@  static int encode_plane(FFV1Context *s, FFV1SliceContext *sc,
         if (s->bits_per_raw_sample <= 8) {
             for (x = 0; x < w; x++)
                 sample[0][x] = src[x * pixel_stride + stride * y];
-            if((ret = encode_line(s, w, sample, plane_index, 8)) < 0)
+            if((ret = encode_line(s, sc, w, sample, plane_index, 8)) < 0)
                 return ret;
         } else {
             if (s->packed_at_lsb) {
@@ -301,7 +301,7 @@  static int encode_plane(FFV1Context *s, FFV1SliceContext *sc,
                     sample[0][x] = ((uint16_t*)(src + stride*y))[x] >> (16 - s->bits_per_raw_sample);
                 }
             }
-            if((ret = encode_line(s, w, sample, plane_index, s->bits_per_raw_sample)) < 0)
+            if((ret = encode_line(s, sc, w, sample, plane_index, s->bits_per_raw_sample)) < 0)
                 return ret;
         }
     }
diff --git a/libavcodec/ffv1enc_template.c b/libavcodec/ffv1enc_template.c
index 85fbac224b..4dccd531a8 100644
--- a/libavcodec/ffv1enc_template.c
+++ b/libavcodec/ffv1enc_template.c
@@ -22,14 +22,14 @@ 
 
 #include "ffv1_template.c"
 
-static av_always_inline int RENAME(encode_line)(FFV1Context *s, int w,
-                                                TYPE *sample[3],
-                                                int plane_index, int bits)
+static av_always_inline int
+RENAME(encode_line)(FFV1Context *s, FFV1SliceContext *sc,
+                    int w, TYPE *sample[3], int plane_index, int bits)
 {
     PlaneContext *const p = &s->plane[plane_index];
     RangeCoder *const c   = &s->c;
     int x;
-    int run_index = s->run_index;
+    int run_index = sc->run_index;
     int run_count = 0;
     int run_mode  = 0;
 
@@ -119,7 +119,7 @@  static av_always_inline int RENAME(encode_line)(FFV1Context *s, int w,
         if (run_count)
             put_bits(&s->pb, 1, 1);
     }
-    s->run_index = run_index;
+    sc->run_index = run_index;
 
     return 0;
 }
@@ -138,7 +138,7 @@  static int RENAME(encode_rgb_frame)(FFV1Context *s, FFV1SliceContext *sc,
     int transparency = s->transparency;
     int packed_size = (3 + transparency)*2;
 
-    s->run_index = 0;
+    sc->run_index = 0;
 
     memset(RENAME(sc->sample_buffer), 0, ring_size * MAX_PLANES *
            (w + 6) * sizeof(*RENAME(sc->sample_buffer)));
@@ -193,9 +193,9 @@  static int RENAME(encode_rgb_frame)(FFV1Context *s, FFV1SliceContext *sc,
             sample[p][0][-1] = sample[p][1][0  ];
             sample[p][1][ w] = sample[p][1][w-1];
             if (lbd && s->slice_coding_mode == 0)
-                ret = RENAME(encode_line)(s, w, sample[p], (p + 1) / 2, 9);
+                ret = RENAME(encode_line)(s, sc, w, sample[p], (p + 1) / 2, 9);
             else
-                ret = RENAME(encode_line)(s, w, sample[p], (p + 1) / 2, bits + (s->slice_coding_mode != 1));
+                ret = RENAME(encode_line)(s, sc, w, sample[p], (p + 1) / 2, bits + (s->slice_coding_mode != 1));
             if (ret < 0)
                 return ret;
         }