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

[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.