diff mbox

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

Message ID 20190205200852.2160-4-andreas.rheinhardt@googlemail.com
State Superseded
Headers show

Commit Message

Andreas Rheinhardt Feb. 5, 2019, 8:08 p.m. UTC 
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@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(-)

Comments

Mark Thompson Feb. 10, 2019, 11:06 p.m. UTC | #1 
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@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
diff mbox

Patch

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);
 }
 
diff --git a/libavcodec/av1_parser.c b/libavcodec/av1_parser.c
index 071227eed6..9e26f6517a 100644
--- a/libavcodec/av1_parser.c
+++ b/libavcodec/av1_parser.c
@@ -72,7 +72,7 @@  static int av1_parser_parse(AVCodecParserContext *ctx,
             goto end;
         }
 
-        ff_cbs_fragment_uninit(s->cbc, td, 1);
+        ff_cbs_fragment_uninit(s->cbc, td, 0);
     }
 
     ret = ff_cbs_read(s->cbc, td, data, size, 1);
@@ -159,7 +159,7 @@  static int av1_parser_parse(AVCodecParserContext *ctx,
     }
 
 end:
-    ff_cbs_fragment_uninit(s->cbc, td, 1);
+    ff_cbs_fragment_uninit(s->cbc, td, 0);
 
     s->cbc->log_ctx = NULL;
 
@@ -193,6 +193,7 @@  static void av1_parser_close(AVCodecParserContext *ctx)
 {
     AV1ParseContext *s = ctx->priv_data;
 
+    ff_cbs_fragment_uninit(s->cbc, &s->temporal_unit, 1);
     ff_cbs_close(&s->cbc);
 }
 
diff --git a/libavcodec/filter_units_bsf.c b/libavcodec/filter_units_bsf.c
index 9980601ce5..440b9b6d58 100644
--- a/libavcodec/filter_units_bsf.c
+++ b/libavcodec/filter_units_bsf.c
@@ -139,7 +139,7 @@  static int filter_units_filter(AVBSFContext *bsf, AVPacket *out)
 
         // Don't return packets with nothing in them.
         av_packet_free(&in);
-        ff_cbs_fragment_uninit(ctx->cbc, frag, 1);
+        ff_cbs_fragment_uninit(ctx->cbc, frag, 0);
     }
 
     err = ff_cbs_write_packet(ctx->cbc, out, frag);
@@ -153,7 +153,7 @@  static int filter_units_filter(AVBSFContext *bsf, AVPacket *out)
         goto fail;
 
 fail:
-    ff_cbs_fragment_uninit(ctx->cbc, frag, 1);
+    ff_cbs_fragment_uninit(ctx->cbc, frag, 0);
     av_packet_free(&in);
 
     return err;
@@ -199,18 +199,18 @@  static int filter_units_init(AVBSFContext *bsf)
     ctx->cbc->nb_decompose_unit_types = 0;
 
     if (bsf->par_in->extradata) {
-        CodedBitstreamFragment ps;
+        CodedBitstreamFragment *frag = &ctx->fragment;
 
-        err = ff_cbs_read_extradata(ctx->cbc, &ps, bsf->par_in, 0);
+        err = ff_cbs_read_extradata(ctx->cbc, frag, bsf->par_in, 1);
         if (err < 0) {
             av_log(bsf, AV_LOG_ERROR, "Failed to read extradata.\n");
         } else {
-            err = ff_cbs_write_extradata(ctx->cbc, bsf->par_out, &ps);
+            err = ff_cbs_write_extradata(ctx->cbc, bsf->par_out, frag);
             if (err < 0)
                 av_log(bsf, AV_LOG_ERROR, "Failed to write extradata.\n");
         }
 
-        ff_cbs_fragment_uninit(ctx->cbc, &ps, 1);
+        ff_cbs_fragment_uninit(ctx->cbc, frag, 0);
     }
 
     return err;
@@ -222,6 +222,7 @@  static void filter_units_close(AVBSFContext *bsf)
 
     av_freep(&ctx->type_list);
 
+    ff_cbs_fragment_uninit(ctx->cbc, &ctx->fragment, 1);
     ff_cbs_close(&ctx->cbc);
 }
 
diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index 53ba623b70..37a5c9a9b1 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -604,7 +604,7 @@  static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *out)
 
     err = 0;
 fail:
-    ff_cbs_fragment_uninit(ctx->cbc, au, 1);
+    ff_cbs_fragment_uninit(ctx->cbc, au, 0);
     av_freep(&displaymatrix_side_data);
 
     if (err < 0)
@@ -648,13 +648,15 @@  static int h264_metadata_init(AVBSFContext *bsf)
 
     err = 0;
 fail:
-    ff_cbs_fragment_uninit(ctx->cbc, au, 1);
+    ff_cbs_fragment_uninit(ctx->cbc, au, 0);
     return err;
 }
 
 static void h264_metadata_close(AVBSFContext *bsf)
 {
     H264MetadataContext *ctx = bsf->priv_data;
+
+    ff_cbs_fragment_uninit(ctx->cbc, &ctx->access_unit, 1);
     ff_cbs_close(&ctx->cbc);
 }
 
diff --git a/libavcodec/h264_redundant_pps_bsf.c b/libavcodec/h264_redundant_pps_bsf.c
index 1df93dd121..4c3d020b27 100644
--- a/libavcodec/h264_redundant_pps_bsf.c
+++ b/libavcodec/h264_redundant_pps_bsf.c
@@ -118,7 +118,7 @@  static int h264_redundant_pps_filter(AVBSFContext *bsf, AVPacket *out)
 
     err = 0;
 fail:
-    ff_cbs_fragment_uninit(ctx->output, au, 1);
+    ff_cbs_fragment_uninit(ctx->output, au, 0);
     av_packet_free(&in);
     if (err < 0)
         av_packet_unref(out);
@@ -167,7 +167,7 @@  static int h264_redundant_pps_init(AVBSFContext *bsf)
 
     err = 0;
 fail:
-    ff_cbs_fragment_uninit(ctx->output, au, 1);
+    ff_cbs_fragment_uninit(ctx->output, au, 0);
     return err;
 }
 
@@ -180,6 +180,8 @@  static void h264_redundant_pps_flush(AVBSFContext *bsf)
 static void h264_redundant_pps_close(AVBSFContext *bsf)
 {
     H264RedundantPPSContext *ctx = bsf->priv_data;
+
+    ff_cbs_fragment_uninit(ctx->output, &ctx->access_unit, 1);
     ff_cbs_close(&ctx->input);
     ff_cbs_close(&ctx->output);
 }
diff --git a/libavcodec/h265_metadata_bsf.c b/libavcodec/h265_metadata_bsf.c
index 53d35f0e09..d7c44c90ce 100644
--- a/libavcodec/h265_metadata_bsf.c
+++ b/libavcodec/h265_metadata_bsf.c
@@ -322,7 +322,7 @@  static int h265_metadata_filter(AVBSFContext *bsf, AVPacket *out)
 
     err = 0;
 fail:
-    ff_cbs_fragment_uninit(ctx->cbc, au, 1);
+    ff_cbs_fragment_uninit(ctx->cbc, au, 0);
 
     if (err < 0)
         av_packet_unref(out);
@@ -370,13 +370,15 @@  static int h265_metadata_init(AVBSFContext *bsf)
 
     err = 0;
 fail:
-    ff_cbs_fragment_uninit(ctx->cbc, au, 1);
+    ff_cbs_fragment_uninit(ctx->cbc, au, 0);
     return err;
 }
 
 static void h265_metadata_close(AVBSFContext *bsf)
 {
     H265MetadataContext *ctx = bsf->priv_data;
+
+    ff_cbs_fragment_uninit(ctx->cbc, &ctx->access_unit, 1);
     ff_cbs_close(&ctx->cbc);
 }
 
diff --git a/libavcodec/mpeg2_metadata_bsf.c b/libavcodec/mpeg2_metadata_bsf.c
index 43e6ac8c15..69e794c0ee 100644
--- a/libavcodec/mpeg2_metadata_bsf.c
+++ b/libavcodec/mpeg2_metadata_bsf.c
@@ -214,7 +214,7 @@  static int mpeg2_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);
@@ -255,13 +255,15 @@  static int mpeg2_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 mpeg2_metadata_close(AVBSFContext *bsf)
 {
     MPEG2MetadataContext *ctx = bsf->priv_data;
+
+    ff_cbs_fragment_uninit(ctx->cbc, &ctx->fragment, 1);
     ff_cbs_close(&ctx->cbc);
 }
 
diff --git a/libavcodec/trace_headers_bsf.c b/libavcodec/trace_headers_bsf.c
index c6a47e6d6d..3062c529c8 100644
--- a/libavcodec/trace_headers_bsf.c
+++ b/libavcodec/trace_headers_bsf.c
@@ -28,6 +28,7 @@ 
 
 typedef struct TraceHeadersContext {
     CodedBitstreamContext *cbc;
+    CodedBitstreamFragment fragment;
 } TraceHeadersContext;
 
 
@@ -44,13 +45,13 @@  static int trace_headers_init(AVBSFContext *bsf)
     ctx->cbc->trace_level  = AV_LOG_INFO;
 
     if (bsf->par_in->extradata) {
-        CodedBitstreamFragment ps;
+        CodedBitstreamFragment *frag = &ctx->fragment;
 
         av_log(bsf, AV_LOG_INFO, "Extradata\n");
 
-        err = ff_cbs_read_extradata(ctx->cbc, &ps, bsf->par_in, 0);
+        err = ff_cbs_read_extradata(ctx->cbc, frag, bsf->par_in, 1);
 
-        ff_cbs_fragment_uninit(ctx->cbc, &ps, 1);
+        ff_cbs_fragment_uninit(ctx->cbc, frag, 0);
     }
 
     return err;
@@ -60,13 +61,14 @@  static void trace_headers_close(AVBSFContext *bsf)
 {
     TraceHeadersContext *ctx = bsf->priv_data;
 
+    ff_cbs_fragment_uninit(ctx->cbc, &ctx->fragment, 1);
     ff_cbs_close(&ctx->cbc);
 }
 
 static int trace_headers(AVBSFContext *bsf, AVPacket *pkt)
 {
     TraceHeadersContext *ctx = bsf->priv_data;
-    CodedBitstreamFragment au;
+    CodedBitstreamFragment *frag = &ctx->fragment;
     char tmp[256] = { 0 };
     int err;
 
@@ -92,9 +94,9 @@  static int trace_headers(AVBSFContext *bsf, AVPacket *pkt)
 
     av_log(bsf, AV_LOG_INFO, "Packet: %d bytes%s.\n", pkt->size, tmp);
 
-    err = ff_cbs_read_packet(ctx->cbc, &au, pkt, 0);
+    err = ff_cbs_read_packet(ctx->cbc, frag, pkt, 1);
 
-    ff_cbs_fragment_uninit(ctx->cbc, &au, 1);
+    ff_cbs_fragment_uninit(ctx->cbc, frag, 0);
 
     if (err < 0)
         av_packet_unref(pkt);
diff --git a/libavcodec/vp9_metadata_bsf.c b/libavcodec/vp9_metadata_bsf.c
index 26f1e65f20..f67f288289 100644
--- a/libavcodec/vp9_metadata_bsf.c
+++ b/libavcodec/vp9_metadata_bsf.c
@@ -86,7 +86,7 @@  static int vp9_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);
@@ -105,6 +105,8 @@  static int vp9_metadata_init(AVBSFContext *bsf)
 static void vp9_metadata_close(AVBSFContext *bsf)
 {
     VP9MetadataContext *ctx = bsf->priv_data;
+
+    ff_cbs_fragment_uninit(ctx->cbc, &ctx->fragment, 1);
     ff_cbs_close(&ctx->cbc);
 }