[14/14] avcodec/cbs_h2645: use AVBufferRef to store list of active parameter sets

Message ID 20190504121427.9638-15-lu_zero@gentoo.org
State New
Headers show
Series
  • [01/14] maint: Ignore dot dirs
Related show

Commit Message

Luca Barbato May 4, 2019, 12:14 p.m.
From: James Almer <jamrial@gmail.com>

Removes unnecessary data copies, and partially fixes potential issues
with dangling references held in said lists.

Reviewed-by: Mark Thompson <sw@jkqxz.net>
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/cbs_h264.h  |  2 ++
 libavcodec/cbs_h2645.c | 46 ++++++++++++++++++++++++++--------------------
 libavcodec/cbs_h265.h  |  3 +++
 3 files changed, 31 insertions(+), 20 deletions(-)

Comments

Alexandra Hájková May 13, 2019, 8:56 a.m. | #1
On Sat, May 4, 2019 at 2:16 PM Luca Barbato <lu_zero@gentoo.org> wrote:
>
> From: James Almer <jamrial@gmail.com>
>
> Removes unnecessary data copies, and partially fixes potential issues
> with dangling references held in said lists.
>
> Reviewed-by: Mark Thompson <sw@jkqxz.net>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/cbs_h264.h  |  2 ++
>  libavcodec/cbs_h2645.c | 46 ++++++++++++++++++++++++++--------------------
>  libavcodec/cbs_h265.h  |  3 +++
>  3 files changed, 31 insertions(+), 20 deletions(-)
>
> diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h
> index 5a7dc27698..8e68595614 100644
> --- a/libavcodec/cbs_h264.h
> +++ b/libavcodec/cbs_h264.h
> @@ -421,6 +421,8 @@ typedef struct CodedBitstreamH264Context {
>
>      // All currently available parameter sets.  These are updated when
>      // any parameter set NAL unit is read/written with this context.
> +    AVBufferRef *sps_ref[H264_MAX_SPS_COUNT];
> +    AVBufferRef *pps_ref[H264_MAX_PPS_COUNT];
>      H264RawSPS *sps[H264_MAX_SPS_COUNT];
>      H264RawPPS *pps[H264_MAX_PPS_COUNT];
>
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index fab8bb7749..c05b347b1c 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -677,9 +677,10 @@ static int cbs_h2645_split_fragment(CodedBitstreamContext *ctx,
>
>  #define cbs_h2645_replace_ps(h26n, ps_name, ps_var, id_element) \
>  static int cbs_h26 ## h26n ## _replace_ ## ps_var(CodedBitstreamContext *ctx, \
> -                                                  const H26 ## h26n ## Raw ## ps_name *ps_var)  \
> +                                                  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; \
>      if (id > FF_ARRAY_ELEMS(priv->ps_var)) { \
>          av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid " #ps_name \
> @@ -688,11 +689,16 @@ static int cbs_h26 ## h26n ## _replace_ ## ps_var(CodedBitstreamContext *ctx, \
>      } \
>      if (priv->ps_var[id] == priv->active_ ## ps_var) \
>          priv->active_ ## ps_var = NULL ; \
> -    av_freep(&priv->ps_var[id]); \
> -    priv->ps_var[id] = av_malloc(sizeof(*ps_var)); \
> -    if (!priv->ps_var[id]) \
> +    av_buffer_unref(&priv->ps_var ## _ref[id]); \
> +    if (unit->content_ref) \
> +        priv->ps_var ## _ref[id] = av_buffer_ref(unit->content_ref); \
> +    else \
> +        priv->ps_var ## _ref[id] = av_buffer_alloc(sizeof(*ps_var)); \
> +    if (!priv->ps_var ## _ref[id]) \
>          return AVERROR(ENOMEM); \
> -    memcpy(priv->ps_var[id], ps_var, sizeof(*ps_var)); \
> +    priv->ps_var[id] = (H26 ## h26n ## Raw ## ps_name *)priv->ps_var ## _ref[id]->data; \
> +    if (!unit->content_ref) \
> +        memcpy(priv->ps_var[id], ps_var, sizeof(*ps_var)); \
>      return 0; \
>  }
>
> @@ -726,7 +732,7 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx,
>              if (err < 0)
>                  return err;
>
> -            err = cbs_h264_replace_sps(ctx, sps);
> +            err = cbs_h264_replace_sps(ctx, unit);
>              if (err < 0)
>                  return err;
>          }
> @@ -760,7 +766,7 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx,
>              if (err < 0)
>                  return err;
>
> -            err = cbs_h264_replace_pps(ctx, pps);
> +            err = cbs_h264_replace_pps(ctx, unit);
>              if (err < 0)
>                  return err;
>          }
> @@ -873,7 +879,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx,
>              if (err < 0)
>                  return err;
>
> -            err = cbs_h265_replace_vps(ctx, vps);
> +            err = cbs_h265_replace_vps(ctx, unit);
>              if (err < 0)
>                  return err;
>          }
> @@ -892,7 +898,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx,
>              if (err < 0)
>                  return err;
>
> -            err = cbs_h265_replace_sps(ctx, sps);
> +            err = cbs_h265_replace_sps(ctx, unit);
>              if (err < 0)
>                  return err;
>          }
> @@ -912,7 +918,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx,
>              if (err < 0)
>                  return err;
>
> -            err = cbs_h265_replace_pps(ctx, pps);
> +            err = cbs_h265_replace_pps(ctx, unit);
>              if (err < 0)
>                  return err;
>          }
> @@ -1002,7 +1008,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx,
>              if (err < 0)
>                  return err;
>
> -            err = cbs_h264_replace_sps(ctx, sps);
> +            err = cbs_h264_replace_sps(ctx, unit);
>              if (err < 0)
>                  return err;
>          }
> @@ -1026,7 +1032,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx,
>              if (err < 0)
>                  return err;
>
> -            err = cbs_h264_replace_pps(ctx, pps);
> +            err = cbs_h264_replace_pps(ctx, unit);
>              if (err < 0)
>                  return err;
>          }
> @@ -1123,7 +1129,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx,
>              if (err < 0)
>                  return err;
>
> -            err = cbs_h265_replace_vps(ctx, vps);
> +            err = cbs_h265_replace_vps(ctx, unit);
>              if (err < 0)
>                  return err;
>          }
> @@ -1137,7 +1143,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx,
>              if (err < 0)
>                  return err;
>
> -            err = cbs_h265_replace_sps(ctx, sps);
> +            err = cbs_h265_replace_sps(ctx, unit);
>              if (err < 0)
>                  return err;
>          }
> @@ -1151,7 +1157,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx,
>              if (err < 0)
>                  return err;
>
> -            err = cbs_h265_replace_pps(ctx, pps);
> +            err = cbs_h265_replace_pps(ctx, unit);
>              if (err < 0)
>                  return err;
>          }
> @@ -1385,9 +1391,9 @@ static void cbs_h264_close(CodedBitstreamContext *ctx)
>      av_freep(&h264->common.write_buffer);
>
>      for (i = 0; i < FF_ARRAY_ELEMS(h264->sps); i++)
> -        av_freep(&h264->sps[i]);
> +        av_buffer_unref(&h264->sps_ref[i]);
>      for (i = 0; i < FF_ARRAY_ELEMS(h264->pps); i++)
> -        av_freep(&h264->pps[i]);
> +        av_buffer_unref(&h264->pps_ref[i]);
>  }
>
>  static void cbs_h265_close(CodedBitstreamContext *ctx)
> @@ -1400,11 +1406,11 @@ static void cbs_h265_close(CodedBitstreamContext *ctx)
>      av_freep(&h265->common.write_buffer);
>
>      for (i = 0; i < FF_ARRAY_ELEMS(h265->vps); i++)
> -        av_freep(&h265->vps[i]);
> +        av_buffer_unref(&h265->vps_ref[i]);
>      for (i = 0; i < FF_ARRAY_ELEMS(h265->sps); i++)
> -        av_freep(&h265->sps[i]);
> +        av_buffer_unref(&h265->sps_ref[i]);
>      for (i = 0; i < FF_ARRAY_ELEMS(h265->pps); i++)
> -        av_freep(&h265->pps[i]);
> +        av_buffer_unref(&h265->pps_ref[i]);
>  }
>
>  const CodedBitstreamType ff_cbs_type_h264 = {
> diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h
> index 0628748f18..f894c9982f 100644
> --- a/libavcodec/cbs_h265.h
> +++ b/libavcodec/cbs_h265.h
> @@ -523,6 +523,9 @@ typedef struct CodedBitstreamH265Context {
>
>      // All currently available parameter sets.  These are updated when
>      // any parameter set NAL unit is read/written with this context.
> +    AVBufferRef *vps_ref[HEVC_MAX_VPS_COUNT];
> +    AVBufferRef *sps_ref[HEVC_MAX_SPS_COUNT];
> +    AVBufferRef *pps_ref[HEVC_MAX_PPS_COUNT];
>      H265RawVPS *vps[HEVC_MAX_VPS_COUNT];
>      H265RawSPS *sps[HEVC_MAX_SPS_COUNT];
>      H265RawPPS *pps[HEVC_MAX_PPS_COUNT];
> --
> 2.12.2
>
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel

OK

Patch

diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h
index 5a7dc27698..8e68595614 100644
--- a/libavcodec/cbs_h264.h
+++ b/libavcodec/cbs_h264.h
@@ -421,6 +421,8 @@  typedef struct CodedBitstreamH264Context {
 
     // All currently available parameter sets.  These are updated when
     // any parameter set NAL unit is read/written with this context.
+    AVBufferRef *sps_ref[H264_MAX_SPS_COUNT];
+    AVBufferRef *pps_ref[H264_MAX_PPS_COUNT];
     H264RawSPS *sps[H264_MAX_SPS_COUNT];
     H264RawPPS *pps[H264_MAX_PPS_COUNT];
 
diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index fab8bb7749..c05b347b1c 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -677,9 +677,10 @@  static int cbs_h2645_split_fragment(CodedBitstreamContext *ctx,
 
 #define cbs_h2645_replace_ps(h26n, ps_name, ps_var, id_element) \
 static int cbs_h26 ## h26n ## _replace_ ## ps_var(CodedBitstreamContext *ctx, \
-                                                  const H26 ## h26n ## Raw ## ps_name *ps_var)  \
+                                                  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; \
     if (id > FF_ARRAY_ELEMS(priv->ps_var)) { \
         av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid " #ps_name \
@@ -688,11 +689,16 @@  static int cbs_h26 ## h26n ## _replace_ ## ps_var(CodedBitstreamContext *ctx, \
     } \
     if (priv->ps_var[id] == priv->active_ ## ps_var) \
         priv->active_ ## ps_var = NULL ; \
-    av_freep(&priv->ps_var[id]); \
-    priv->ps_var[id] = av_malloc(sizeof(*ps_var)); \
-    if (!priv->ps_var[id]) \
+    av_buffer_unref(&priv->ps_var ## _ref[id]); \
+    if (unit->content_ref) \
+        priv->ps_var ## _ref[id] = av_buffer_ref(unit->content_ref); \
+    else \
+        priv->ps_var ## _ref[id] = av_buffer_alloc(sizeof(*ps_var)); \
+    if (!priv->ps_var ## _ref[id]) \
         return AVERROR(ENOMEM); \
-    memcpy(priv->ps_var[id], ps_var, sizeof(*ps_var)); \
+    priv->ps_var[id] = (H26 ## h26n ## Raw ## ps_name *)priv->ps_var ## _ref[id]->data; \
+    if (!unit->content_ref) \
+        memcpy(priv->ps_var[id], ps_var, sizeof(*ps_var)); \
     return 0; \
 }
 
@@ -726,7 +732,7 @@  static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx,
             if (err < 0)
                 return err;
 
-            err = cbs_h264_replace_sps(ctx, sps);
+            err = cbs_h264_replace_sps(ctx, unit);
             if (err < 0)
                 return err;
         }
@@ -760,7 +766,7 @@  static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx,
             if (err < 0)
                 return err;
 
-            err = cbs_h264_replace_pps(ctx, pps);
+            err = cbs_h264_replace_pps(ctx, unit);
             if (err < 0)
                 return err;
         }
@@ -873,7 +879,7 @@  static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx,
             if (err < 0)
                 return err;
 
-            err = cbs_h265_replace_vps(ctx, vps);
+            err = cbs_h265_replace_vps(ctx, unit);
             if (err < 0)
                 return err;
         }
@@ -892,7 +898,7 @@  static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx,
             if (err < 0)
                 return err;
 
-            err = cbs_h265_replace_sps(ctx, sps);
+            err = cbs_h265_replace_sps(ctx, unit);
             if (err < 0)
                 return err;
         }
@@ -912,7 +918,7 @@  static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx,
             if (err < 0)
                 return err;
 
-            err = cbs_h265_replace_pps(ctx, pps);
+            err = cbs_h265_replace_pps(ctx, unit);
             if (err < 0)
                 return err;
         }
@@ -1002,7 +1008,7 @@  static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx,
             if (err < 0)
                 return err;
 
-            err = cbs_h264_replace_sps(ctx, sps);
+            err = cbs_h264_replace_sps(ctx, unit);
             if (err < 0)
                 return err;
         }
@@ -1026,7 +1032,7 @@  static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx,
             if (err < 0)
                 return err;
 
-            err = cbs_h264_replace_pps(ctx, pps);
+            err = cbs_h264_replace_pps(ctx, unit);
             if (err < 0)
                 return err;
         }
@@ -1123,7 +1129,7 @@  static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx,
             if (err < 0)
                 return err;
 
-            err = cbs_h265_replace_vps(ctx, vps);
+            err = cbs_h265_replace_vps(ctx, unit);
             if (err < 0)
                 return err;
         }
@@ -1137,7 +1143,7 @@  static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx,
             if (err < 0)
                 return err;
 
-            err = cbs_h265_replace_sps(ctx, sps);
+            err = cbs_h265_replace_sps(ctx, unit);
             if (err < 0)
                 return err;
         }
@@ -1151,7 +1157,7 @@  static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx,
             if (err < 0)
                 return err;
 
-            err = cbs_h265_replace_pps(ctx, pps);
+            err = cbs_h265_replace_pps(ctx, unit);
             if (err < 0)
                 return err;
         }
@@ -1385,9 +1391,9 @@  static void cbs_h264_close(CodedBitstreamContext *ctx)
     av_freep(&h264->common.write_buffer);
 
     for (i = 0; i < FF_ARRAY_ELEMS(h264->sps); i++)
-        av_freep(&h264->sps[i]);
+        av_buffer_unref(&h264->sps_ref[i]);
     for (i = 0; i < FF_ARRAY_ELEMS(h264->pps); i++)
-        av_freep(&h264->pps[i]);
+        av_buffer_unref(&h264->pps_ref[i]);
 }
 
 static void cbs_h265_close(CodedBitstreamContext *ctx)
@@ -1400,11 +1406,11 @@  static void cbs_h265_close(CodedBitstreamContext *ctx)
     av_freep(&h265->common.write_buffer);
 
     for (i = 0; i < FF_ARRAY_ELEMS(h265->vps); i++)
-        av_freep(&h265->vps[i]);
+        av_buffer_unref(&h265->vps_ref[i]);
     for (i = 0; i < FF_ARRAY_ELEMS(h265->sps); i++)
-        av_freep(&h265->sps[i]);
+        av_buffer_unref(&h265->sps_ref[i]);
     for (i = 0; i < FF_ARRAY_ELEMS(h265->pps); i++)
-        av_freep(&h265->pps[i]);
+        av_buffer_unref(&h265->pps_ref[i]);
 }
 
 const CodedBitstreamType ff_cbs_type_h264 = {
diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h
index 0628748f18..f894c9982f 100644
--- a/libavcodec/cbs_h265.h
+++ b/libavcodec/cbs_h265.h
@@ -523,6 +523,9 @@  typedef struct CodedBitstreamH265Context {
 
     // All currently available parameter sets.  These are updated when
     // any parameter set NAL unit is read/written with this context.
+    AVBufferRef *vps_ref[HEVC_MAX_VPS_COUNT];
+    AVBufferRef *sps_ref[HEVC_MAX_SPS_COUNT];
+    AVBufferRef *pps_ref[HEVC_MAX_PPS_COUNT];
     H265RawVPS *vps[HEVC_MAX_VPS_COUNT];
     H265RawSPS *sps[HEVC_MAX_SPS_COUNT];
     H265RawPPS *pps[HEVC_MAX_PPS_COUNT];