Re: [FFmpeg-devel] [PATCH 6/7] ffv1: use standard syntax for indexing state

[Lynne via ffmpeg-devel]

On 11/10/24 02:36, Michael Niedermayer wrote:

On Sat, Nov 09, 2024 at 08:22:25AM +0100, Lynne via ffmpeg-devel wrote:

It isn't immediately obvious what indexing this array does.
Use standard syntax instead.
---
  libavcodec/ffv1.h             | 2 +-
  libavcodec/ffv1dec_template.c | 2 +-
  libavcodec/ffv1enc_template.c | 4 ++--
  3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h
index 2af457be27..ed71e238e0 100644
--- a/libavcodec/ffv1.h
+++ b/libavcodec/ffv1.h
@@ -63,7 +63,7 @@ typedef struct VlcState {
  typedef struct PlaneContext {
      int quant_table_index;
      int context_count;
-    uint8_t (*state)[CONTEXT_SIZE];
+    uint8_t *state;
      VlcState *vlc_state;
  } PlaneContext;

diff --git a/libavcodec/ffv1dec_template.c b/libavcodec/ffv1dec_template.c

index 2da6bd935d..3c95741b32 100644
--- a/libavcodec/ffv1dec_template.c
+++ b/libavcodec/ffv1dec_template.c
@@ -71,7 +71,7 @@ RENAME(decode_line)(FFV1Context *f, FFV1SliceContext *sc,
          av_assert2(context < p->context_count);

if (ac != AC_GOLOMB_RICE) {

-            diff = get_symbol_inline(c, p->state[context], 1);
+            diff = get_symbol_inline(c, &p->state[32*context], 1);
          } else {
              if (context == 0 && run_mode == 0)
                  run_mode = 1;
diff --git a/libavcodec/ffv1enc_template.c b/libavcodec/ffv1enc_template.c
index bc14926ab9..e17e40a327 100644
--- a/libavcodec/ffv1enc_template.c
+++ b/libavcodec/ffv1enc_template.c
@@ -75,10 +75,10 @@ RENAME(encode_line)(FFV1Context *f, FFV1SliceContext *sc,

if (ac != AC_GOLOMB_RICE) {

              if (pass1) {
-                put_symbol_inline(c, p->state[context], diff, 1, sc->rc_stat,
+                put_symbol_inline(c, &p->state[32*context], diff, 1, 
sc->rc_stat,
                                    
sc->rc_stat2[p->quant_table_index][context]);
              } else {
-                put_symbol_inline(c, p->state[context], diff, 1, NULL, NULL);
+                put_symbol_inline(c, &p->state[32*context], diff, 1, NULL, 
NULL);

I would prefer that a comment is added to "uint8_t (*state)[CONTEXT_SIZE];"
if its unclear what it represents.

Also the "32" is hardcoded and duplicates in the patch which is bad if it
needs to be changed

thx

[...]

Yeah, it was just not syntax I had encountered before.

Consider this patch dropped.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".