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

[Lynne]

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.