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

2023-07-16 Thread 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 
>> ---
>>   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(>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(>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

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

2023-07-16 Thread 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 
---
  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(>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(>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.

___
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".