Message ID | ce9afa74-f16d-6a69-b1d7-6e2b7ba99fdb@jkqxz.net |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,v2] cbs_h2645: Implement replace-PS with a table rather than many functions |
Related | show |
Context | Check | Description |
---|---|---|
andriy/configure | warning | Failed to apply patch |
andriy/configure | warning | Failed to apply patch |
On Tue, Feb 16, 2021 at 7:20 AM Mark Thompson <sw@jkqxz.net> wrote: > While this is mode source code, it generates less binary code and can be > more cleanly extended to H.266. > --- > On 14/02/2021 19:45, Andreas Rheinhardt wrote: > > Nuo Mi: > >> From: Mark Thompson <sw@jkqxz.net> > > Hi Andreas & Mark, thanks for the suggestion. Hi Mark, vvc's xps does not need active_offset. vvc's picture header does not need id_offset. see [1], [2] for details How about we change the patch like this: $git diff diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c index 6c44e781fa..57c419aa05 100644 --- a/libavcodec/cbs_h2645.c +++ b/libavcodec/cbs_h2645.c @@ -690,20 +690,24 @@ static int cbs_h2645_replace_ps(CodedBitstreamContext *ctx, size_t active_offset; } PSType; -#define H2645_PS_TYPE(codec, nal, cname, uname, count, id_name, active_field) { \ +#define H26456_PS_TYPE(codec, nal, cname, uname, count, id_off, active_off) { \ .codec_id = AV_CODEC_ID_ ## codec, \ .nal_unit_type = nal, \ .name = #cname, \ .id_count = count, \ - .id_offset = offsetof(codec ## Raw ## cname, \ - id_name ## _parameter_set_id), \ + .id_offset = id_off, \ .ref_array_offset = offsetof(CodedBitstream ## codec ## Context, \ uname ## _ref), \ .ptr_array_offset = offsetof(CodedBitstream ## codec ## Context, \ uname), \ - .active_offset = offsetof(CodedBitstream ## codec ## Context, \ - active_field), \ + .active_offset = active_off, \ } + +#define H2645_PS_TYPE(codec, nal, cname, uname, count, id_name, active_field) \ + H26456_PS_TYPE(codec, nal, cname, uname, count, \ + offsetof(codec ## Raw ## cname, id_name ## _parameter_set_id), \ + offsetof(CodedBitstream ## codec ## Context, active_field)) + #define H264_PS_TYPE(cname, uname, id_name) \ H2645_PS_TYPE(H264, H264_NAL_ ## cname, cname, uname, \ H264_MAX_ ## cname ## _COUNT, \ @@ -724,7 +728,7 @@ static int cbs_h2645_replace_ps(CodedBitstreamContext *ctx, const PSType *ps_type; AVBufferRef **ref_array; void **ptr_array; - int err, id, i; + int err, id = 0, i; ps_type = NULL; for (i = 0; i < FF_ARRAY_ELEMS(ps_types); i++) { @@ -736,12 +740,14 @@ static int cbs_h2645_replace_ps(CodedBitstreamContext *ctx, } av_assert0(ps_type); - id = *((uint8_t*)unit->content + ps_type->id_offset); + if (ps_type->id_offset) { + id = *((uint8_t*)unit->content + ps_type->id_offset); - if (id >= ps_type->id_count) { - av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid %s id: %d.\n", - ps_type->name, id); - return AVERROR_INVALIDDATA; + if (id >= ps_type->id_count) { + av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid %s id: %d.\n", + ps_type->name, id); + return AVERROR_INVALIDDATA; + } } err = ff_cbs_make_unit_refcounted(ctx, unit); [1]: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210211143611.5663-5-nuomi2021@gmail.com/ [2] h266_ps_types in https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210211143611.5663-6-nuomi2021@gmail.com/
On 2/16/2021 12:16 PM, Nuo Mi wrote: > On Tue, Feb 16, 2021 at 7:20 AM Mark Thompson <sw@jkqxz.net> wrote: > >> While this is mode source code, it generates less binary code and can be >> more cleanly extended to H.266. >> --- >> On 14/02/2021 19:45, Andreas Rheinhardt wrote: >>> Nuo Mi: >>>> From: Mark Thompson <sw@jkqxz.net> >> >> Hi Andreas & Mark, > thanks for the suggestion. > > Hi Mark, > vvc's xps does not need active_offset. The change you suggest here is not needed to handle this, as you only need to define a H266_PS_TYPE macro that does not set .active_offset (When using designated initializers, fields that are not set will be zeroed). But at the end of the day, it's a cosmetic choice, so whatever you two prefer is fine. > vvc's picture header does not need id_offset. see [1], [2] for details This one most likely needs the change you suggested below, though. > How about we change the patch like this: > > $git diff > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c > index 6c44e781fa..57c419aa05 100644 > --- a/libavcodec/cbs_h2645.c > +++ b/libavcodec/cbs_h2645.c > @@ -690,20 +690,24 @@ static int cbs_h2645_replace_ps(CodedBitstreamContext > *ctx, > size_t active_offset; > } PSType; > > -#define H2645_PS_TYPE(codec, nal, cname, uname, count, id_name, > active_field) { \ > +#define H26456_PS_TYPE(codec, nal, cname, uname, count, id_off, > active_off) { \ > .codec_id = AV_CODEC_ID_ ## codec, \ > .nal_unit_type = nal, \ > .name = #cname, \ > .id_count = count, \ > - .id_offset = offsetof(codec ## Raw ## cname, \ > - id_name ## _parameter_set_id), \ > + .id_offset = id_off, \ > .ref_array_offset = offsetof(CodedBitstream ## codec ## Context, \ > uname ## _ref), \ > .ptr_array_offset = offsetof(CodedBitstream ## codec ## Context, \ > uname), \ > - .active_offset = offsetof(CodedBitstream ## codec ## Context, \ > - active_field), \ > + .active_offset = active_off, \ > } > + > +#define H2645_PS_TYPE(codec, nal, cname, uname, count, id_name, > active_field) \ > + H26456_PS_TYPE(codec, nal, cname, uname, count, \ > + offsetof(codec ## Raw ## cname, id_name ## _parameter_set_id), > \ > + offsetof(CodedBitstream ## codec ## Context, active_field)) > + > #define H264_PS_TYPE(cname, uname, id_name) \ > H2645_PS_TYPE(H264, H264_NAL_ ## cname, cname, uname, \ > H264_MAX_ ## cname ## _COUNT, \ > @@ -724,7 +728,7 @@ static int cbs_h2645_replace_ps(CodedBitstreamContext > *ctx, > const PSType *ps_type; > AVBufferRef **ref_array; > void **ptr_array; > - int err, id, i; > + int err, id = 0, i; > > ps_type = NULL; > for (i = 0; i < FF_ARRAY_ELEMS(ps_types); i++) { > @@ -736,12 +740,14 @@ static int cbs_h2645_replace_ps(CodedBitstreamContext > *ctx, > } > av_assert0(ps_type); > > - id = *((uint8_t*)unit->content + ps_type->id_offset); > + if (ps_type->id_offset) { > + id = *((uint8_t*)unit->content + ps_type->id_offset); > > - if (id >= ps_type->id_count) { > - av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid %s id: %d.\n", > - ps_type->name, id); > - return AVERROR_INVALIDDATA; > + if (id >= ps_type->id_count) { > + av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid %s id: %d.\n", > + ps_type->name, id); > + return AVERROR_INVALIDDATA; > + } > } > > err = ff_cbs_make_unit_refcounted(ctx, unit); > > > [1]: > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210211143611.5663-5-nuomi2021@gmail.com/ > [2] h266_ps_types in > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210211143611.5663-6-nuomi2021@gmail.com/ > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c index 6005d46e0d..6c44e781fa 100644 --- a/libavcodec/cbs_h2645.c +++ b/libavcodec/cbs_h2645.c @@ -661,38 +661,117 @@ 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; \ - if (id >= FF_ARRAY_ELEMS(priv->ps_var)) { \ - av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid " #ps_name \ - " id : %d.\n", id); \ - return AVERROR_INVALIDDATA; \ - } \ - 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; \ -} +static int cbs_h2645_replace_ps(CodedBitstreamContext *ctx, + CodedBitstreamUnit *unit) +{ + typedef struct { + // Codec this parameter set exists in. + enum AVCodecID codec_id; + // The NAL unit type corresponding to this parameter set type. + int nal_unit_type; + // Name of the parameter set. This field is large enough to + // contain a string of the form "XPS". + char name[4]; + // The maximum number of this type of parameter set which might + // be stored. The greatest valid id is also one less than this. + int id_count; + // Offset of the ID field (uint8_t) in the decomposed raw + // parameter set structure. + size_t id_offset; + // Offset of the reference array (AVBufferRef*[]) in the codec + // private context. + size_t ref_array_offset; + // Offset of the pointer array (CodecRawXPS*[]) in the codec + // private context. + size_t ptr_array_offset; + // Offset of the active field (const CodecRawXPS*) in the codec + // private context, or zero if this codec does not have active + // parameter sets. + size_t active_offset; + } PSType; + +#define H2645_PS_TYPE(codec, nal, cname, uname, count, id_name, active_field) { \ + .codec_id = AV_CODEC_ID_ ## codec, \ + .nal_unit_type = nal, \ + .name = #cname, \ + .id_count = count, \ + .id_offset = offsetof(codec ## Raw ## cname, \ + id_name ## _parameter_set_id), \ + .ref_array_offset = offsetof(CodedBitstream ## codec ## Context, \ + uname ## _ref), \ + .ptr_array_offset = offsetof(CodedBitstream ## codec ## Context, \ + uname), \ + .active_offset = offsetof(CodedBitstream ## codec ## Context, \ + active_field), \ + } +#define H264_PS_TYPE(cname, uname, id_name) \ + H2645_PS_TYPE(H264, H264_NAL_ ## cname, cname, uname, \ + H264_MAX_ ## cname ## _COUNT, \ + id_name, active_ ## uname) +#define H265_PS_TYPE(cname, uname, id_name) \ + H2645_PS_TYPE(H265, HEVC_NAL_ ## cname, cname, uname, \ + HEVC_MAX_ ## cname ## _COUNT, \ + uname ## _ ## id_name, active_ ## uname) + + static const PSType ps_types[] = { + H264_PS_TYPE(SPS, sps, seq), + H264_PS_TYPE(PPS, pps, pic), + H265_PS_TYPE(VPS, vps, video), + H265_PS_TYPE(SPS, sps, seq), + H265_PS_TYPE(PPS, pps, pic), + }; + + const PSType *ps_type; + AVBufferRef **ref_array; + void **ptr_array; + int err, id, i; + + ps_type = NULL; + for (i = 0; i < FF_ARRAY_ELEMS(ps_types); i++) { + if (ps_types[i].codec_id == ctx->codec->codec_id && + ps_types[i].nal_unit_type == unit->type) { + ps_type = &ps_types[i]; + break; + } + } + av_assert0(ps_type); + + id = *((uint8_t*)unit->content + ps_type->id_offset); + + if (id >= ps_type->id_count) { + av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid %s id: %d.\n", + ps_type->name, id); + return AVERROR_INVALIDDATA; + } + + err = ff_cbs_make_unit_refcounted(ctx, unit); + if (err < 0) + return err; + + ref_array = (AVBufferRef**)((uint8_t*)ctx->priv_data + + ps_type->ref_array_offset); + ptr_array = (void**) ((uint8_t*)ctx->priv_data + + ps_type->ptr_array_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) + if (ps_type->active_offset) { + void **active = (void**)((uint8_t*)ctx->priv_data + + ps_type->active_offset); + + if (ptr_array[id] == *active) { + // The old active parameter set is being overwritten, so it can't + // be active after this point. + *active = NULL; + } + } + av_buffer_unref(&ref_array[id]); + + ref_array[id] = av_buffer_ref(unit->content_ref); + if (!ref_array[id]) + return AVERROR(ENOMEM); + ptr_array[id] = ref_array[id]->data; + + return 0; +} static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, CodedBitstreamUnit *unit) @@ -717,7 +796,7 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h264_replace_sps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -739,7 +818,7 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h264_replace_pps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -836,7 +915,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_vps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -849,7 +928,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_sps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -863,7 +942,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_pps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -1007,7 +1086,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h264_replace_sps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -1031,7 +1110,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h264_replace_pps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -1124,7 +1203,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_vps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -1138,7 +1217,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_sps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -1152,7 +1231,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_pps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; }