[FFmpeg-devel] [PATCH] avcodec/cbs_h2645: Replace parameter sets generically

[Andreas Rheinhardt]

James Almer:
> On 7/16/2023 6:18 PM, Andreas Rheinhardt wrote:
>> This avoids having a function for each type of parameter set
>> for each of the modules.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>> ---
>>   libavcodec/cbs_h2645.c | 134 ++++++++++++++++++++++-------------------
>>   1 file changed, 72 insertions(+), 62 deletions(-)
>>
>> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
>> index 34c5d1d372..5bbd74b977 100644
>> --- a/libavcodec/cbs_h2645.c
>> +++ b/libavcodec/cbs_h2645.c
>> @@ -736,55 +736,65 @@ static int
>> cbs_h2645_split_fragment(CodedBitstreamContext *ctx,
>>       return 0;
>>   }
>>   -#define cbs_h2645_replace_ps(h26n, ps_name, ps_var, id_element) \
>> -static int cbs_h26 ## h26n ## _replace_ ##
>> ps_var(CodedBitstreamContext *ctx, \
>> -                                                  CodedBitstreamUnit
>> *unit)  \
>> -{ \
>> -    CodedBitstreamH26 ## h26n ## Context *priv = ctx->priv_data; \
>> -    H26 ## h26n ## Raw ## ps_name *ps_var = unit->content; \
>> -    unsigned int id = ps_var->id_element; \
>> -    int err = ff_cbs_make_unit_refcounted(ctx, unit); \
>> -    if (err < 0) \
>> -        return err; \
>> -    if (priv->ps_var[id] == priv->active_ ## ps_var) \
>> -        priv->active_ ## ps_var = NULL ; \
>> -    av_buffer_unref(&priv->ps_var ## _ref[id]); \
>> -    av_assert0(unit->content_ref); \
>> -    priv->ps_var ## _ref[id] = av_buffer_ref(unit->content_ref); \
>> -    if (!priv->ps_var ## _ref[id]) \
>> -        return AVERROR(ENOMEM); \
>> -    priv->ps_var[id] = (H26 ## h26n ## Raw ## ps_name *)priv->ps_var
>> ## _ref[id]->data; \
>> -    return 0; \
>> +typedef struct PSOffsets {
>> +    unsigned id_element_offset;    //< relative to the raw parameter
>> set struct; must refer to uint8_t
>> +    unsigned ps_array_offset;      //< relative to
>> CodedBitstreamH26*Context
>> +    unsigned ps_ref_array_offset;  //< relative to
>> CodedBitstreamH26*Context
>> +    unsigned active_ps_offset;     //< Zero means that active PS is
>> unused
>> +} PSOffsets;
>> +
>> +#define CBS_H26456_OFFSET_TABLE(MODULE, module, PS_NAME, ps_var,
>> id_element, _active_ps_offset)   \
>> +static const PSOffsets module ## _ ## ps_var ## _offsets =
>> {                              \
>> +    .id_element_offset   = offsetof(MODULE ## Raw ## PS_NAME,
>> id_element),                \
>> +    .ps_array_offset     = offsetof(CodedBitstream ## MODULE ##
>> Context, ps_var),         \
>> +    .ps_ref_array_offset = offsetof(CodedBitstream ## MODULE ##
>> Context, ps_var ## _ref), \
>> +    .active_ps_offset    =
>> _active_ps_offset,                                             \
>>   }
>>   -cbs_h2645_replace_ps(4, SPS, sps, seq_parameter_set_id)
>> -cbs_h2645_replace_ps(4, PPS, pps, pic_parameter_set_id)
>> -cbs_h2645_replace_ps(5, VPS, vps, vps_video_parameter_set_id)
>> -cbs_h2645_replace_ps(5, SPS, sps, sps_seq_parameter_set_id)
>> -cbs_h2645_replace_ps(5, PPS, pps, pps_pic_parameter_set_id)
>> -
>> -#define cbs_h266_replace_ps(h26n, ps_name, ps_var, id_element) \
>> -static int cbs_h26 ## h26n ## _replace_ ##
>> ps_var(CodedBitstreamContext *ctx, \
>> -                                                  CodedBitstreamUnit
>> *unit)  \
>> -{ \
>> -    CodedBitstreamH26 ## h26n ## Context *priv = ctx->priv_data; \
>> -    H26 ## h26n ## Raw ## ps_name *ps_var = unit->content; \
>> -    unsigned int id = ps_var->id_element; \
>> -    int err = ff_cbs_make_unit_refcounted(ctx, unit); \
>> -    if (err < 0) \
>> -        return err; \
>> -    av_buffer_unref(&priv->ps_var ## _ref[id]); \
>> -    av_assert0(unit->content_ref); \
>> -    priv->ps_var ## _ref[id] = av_buffer_ref(unit->content_ref); \
>> -    if (!priv->ps_var ## _ref[id]) \
>> -        return AVERROR(ENOMEM); \
>> -    priv->ps_var[id] = (H26 ## h26n ## Raw ## ps_name *)priv->ps_var
>> ## _ref[id]->data; \
>> -    return 0; \
>> -}
>> +#define CBS_H2645_OFFSET_TABLE(h26n, PS_NAME, ps_var, id_element)    \
>> +        CBS_H26456_OFFSET_TABLE(H26 ## h26n, h26 ## h26n, PS_NAME,
>> ps_var, id_element,   \
>> +                                offsetof(CodedBitstreamH26 ## h26n ##
>> Context, active_ ## ps_var))
>> +
>> +CBS_H2645_OFFSET_TABLE(4, SPS, sps, seq_parameter_set_id);
>> +CBS_H2645_OFFSET_TABLE(4, PPS, pps, pic_parameter_set_id);
>> +CBS_H2645_OFFSET_TABLE(5, VPS, vps, vps_video_parameter_set_id);
>> +CBS_H2645_OFFSET_TABLE(5, SPS, sps, sps_seq_parameter_set_id);
>> +CBS_H2645_OFFSET_TABLE(5, PPS, pps, pps_pic_parameter_set_id);
>> +
>> +#define CBS_H266_OFFSET_TABLE(PS_NAME, ps_var, id_element)     \
>> +        CBS_H26456_OFFSET_TABLE(H266, h266, PS_NAME, ps_var,
>> id_element, 0)
>> +
>> +CBS_H266_OFFSET_TABLE(VPS, vps, vps_video_parameter_set_id);
>> +CBS_H266_OFFSET_TABLE(SPS, sps, sps_seq_parameter_set_id);
>> +CBS_H266_OFFSET_TABLE(PPS, pps, pps_pic_parameter_set_id);
>> +
>> +static int cbs_h26456_replace_ps(CodedBitstreamContext *ctx,
>> CodedBitstreamUnit *unit,
> 
> nit: it might be a good idea to rename the module (and h2645_parse) to
> h26x. Or alternatively, nal.

I amended my patch to use h26x in the parts that I touch. Will rename
the other stuff in other patches.

- Andreas