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

[Michael Niedermayer]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20241110/f79bca16/attachment.sig>