[FFmpeg-devel] [PATCH 3/3] cbs: Stop reallocating fragment unit arrays

[Mark Thompson]

On 05/02/2019 20:08, Andreas Rheinhardt wrote:
> This commit changes various places that make use of cbs to keep the
> fragments' unit arrays instead of constantly reallocating them.
> 
> The more units a packet is split into, the bigger the benefit.
> So MPEG-2 benefits the most; for a video coming from an NTSC-DVD
> (usually 32 units per frame) the average cost of cbs_insert_unit (for a
> single unit) went down from 6717 decicycles to 450 decicycles (based
> upon 10 runs with 4194304 runs each); if each packet consists of only
> one unit, it went down from 2425 to 448; for a H.264 video where most
> packets contain nine units, it went from 4431 to 450.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at googlemail.com>
> ---
>  libavcodec/av1_metadata_bsf.c       |  6 ++++--
>  libavcodec/av1_parser.c             |  5 +++--
>  libavcodec/filter_units_bsf.c       | 13 +++++++------
>  libavcodec/h264_metadata_bsf.c      |  6 ++++--
>  libavcodec/h264_redundant_pps_bsf.c |  6 ++++--
>  libavcodec/h265_metadata_bsf.c      |  6 ++++--
>  libavcodec/mpeg2_metadata_bsf.c     |  6 ++++--
>  libavcodec/trace_headers_bsf.c      | 14 ++++++++------
>  libavcodec/vp9_metadata_bsf.c       |  4 +++-
>  9 files changed, 41 insertions(+), 25 deletions(-)
> 
> diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_bsf.c
> index b08a1379e7..5e4dc10b89 100644
> --- a/libavcodec/av1_metadata_bsf.c
> +++ b/libavcodec/av1_metadata_bsf.c
> @@ -170,7 +170,7 @@ static int av1_metadata_filter(AVBSFContext *bsf, AVPacket *out)
>  
>      err = 0;
>  fail:
> -    ff_cbs_fragment_uninit(ctx->cbc, frag, 1);
> +    ff_cbs_fragment_uninit(ctx->cbc, frag, 0);
>  
>      if (err < 0)
>          av_packet_unref(out);
> @@ -215,13 +215,15 @@ static int av1_metadata_init(AVBSFContext *bsf)
>  
>      err = 0;
>  fail:
> -    ff_cbs_fragment_uninit(ctx->cbc, frag, 1);
> +    ff_cbs_fragment_uninit(ctx->cbc, frag, 0);
>      return err;
>  }
>  
>  static void av1_metadata_close(AVBSFContext *bsf)
>  {
>      AV1MetadataContext *ctx = bsf->priv_data;
> +
> +    ff_cbs_fragment_uninit(ctx->cbc, &ctx->access_unit, 1);
>      ff_cbs_close(&ctx->cbc);
>  }

I feel like this patch for each BSF demonstrates nicely that they should be separate functions - one version is called repeatedly in the filter operations, the other only in close.

With suitable names, this all looks good.

Thanks,

- Mark