diff mbox series

[FFmpeg-devel,35/36] avcodec/vp9_superframe_bsf: Only store what is needed later

Message ID 20200530160541.29517-35-andreas.rheinhardt@gmail.com
State New
Headers show
Series [FFmpeg-devel,01/36] avcodec/vp9_superframe_bsf: Check for existence of data before reading it | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt May 30, 2020, 4:05 p.m. UTC
This commit ends storing all the packets for later use. Instead the only
part that is actually needed later is kept: The buffer. This avoids
copies of packet structures and also makes the init function
superfluous.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/vp9_superframe_bsf.c | 45 +++++++++++++--------------------
 1 file changed, 17 insertions(+), 28 deletions(-)

Comments

James Almer May 30, 2020, 4:58 p.m. UTC | #1
On 5/30/2020 1:05 PM, Andreas Rheinhardt wrote:
> This commit ends storing all the packets for later use. Instead the only
> part that is actually needed later is kept: The buffer. This avoids
> copies of packet structures and also makes the init function
> superfluous.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/vp9_superframe_bsf.c | 45 +++++++++++++--------------------
>  1 file changed, 17 insertions(+), 28 deletions(-)
> 
> diff --git a/libavcodec/vp9_superframe_bsf.c b/libavcodec/vp9_superframe_bsf.c
> index 29d1c759c7..6d6b2c57de 100644
> --- a/libavcodec/vp9_superframe_bsf.c
> +++ b/libavcodec/vp9_superframe_bsf.c
> @@ -27,8 +27,8 @@
>  
>  #define MAX_CACHE 8
>  typedef struct VP9BSFContext {
> +    AVBufferRef *cache[MAX_CACHE];
>      int n_cache;
> -    AVPacket *cache[MAX_CACHE];
>  } VP9BSFContext;
>  
>  static void vp9_superframe_flush(AVBSFContext *ctx)
> @@ -37,11 +37,11 @@ static void vp9_superframe_flush(AVBSFContext *ctx)
>  
>      // unref cached data
>      for (int n = 0; n < s->n_cache; n++)
> -        av_packet_unref(s->cache[n]);
> +        av_buffer_unref(&s->cache[n]);
>      s->n_cache = 0;
>  }
>  
> -static void stats(AVPacket * const *in, int n_in,
> +static void stats(AVBufferRef * const *in, int n_in,
>                    unsigned *_max, uint64_t *_sum)
>  {
>      int n;
> @@ -60,10 +60,11 @@ static void stats(AVPacket * const *in, int n_in,
>      *_sum = sum;
>  }
>  
> -static int merge_superframe(AVPacket * const *in, int n_in, AVPacket *out)
> +static int merge_superframe(AVBufferRef * const *in, int n_in, AVPacket *pkt)
>  {
>      unsigned max, mag, marker, n;
>      uint64_t sum;
> +    AVBufferRef *out = NULL;
>      uint8_t *ptr;
>      int res;
>  
> @@ -73,7 +74,7 @@ static int merge_superframe(AVPacket * const *in, int n_in, AVPacket *out)
>      sum += 2 + (mag + 1) * n_in;
>      if (sum > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE)
>          return AVERROR(ERANGE);
> -    res = av_new_packet(out, sum);
> +    res = ff_buffer_padded_realloc(&out, sum);
>      if (res < 0)
>          return res;
>      ptr = out->data;
> @@ -107,7 +108,9 @@ static int merge_superframe(AVPacket * const *in, int n_in, AVPacket *out)
>          break;
>      }
>      *ptr++ = marker;
> -    av_assert0(ptr == &out->data[out->size]);
> +
> +    ff_packet_replace_buffer(pkt, out);
> +    av_assert0(ptr == &pkt->data[pkt->size]);
>  
>      return 0;
>  }
> @@ -166,19 +169,21 @@ static int vp9_superframe_filter(AVBSFContext *ctx, AVPacket *pkt)
>  
>      av_assert0(s->n_cache < MAX_CACHE);
>  
> -    av_packet_move_ref(s->cache[s->n_cache++], pkt);
> +    // move the packet's buffer into the cache
> +    pkt->buf->data = pkt->data;
> +    pkt->buf->size = pkt->size;

Manual manipulation of an AVBufferRef like this seems awfully hacky.

> +    s->cache[s->n_cache++] = pkt->buf;
> +    pkt->buf = NULL;
>  
>      if (invisible) {
> -        return AVERROR(EAGAIN);
> +        res = AVERROR(EAGAIN);
> +        goto done;
>      }
>      av_assert0(s->n_cache > 0);
>  
>      // build superframe
>      res = merge_superframe(s->cache, s->n_cache, pkt);
>  
> -    if (res >= 0)
> -        res = av_packet_copy_props(pkt, s->cache[s->n_cache - 1]);
> -
>      vp9_superframe_flush(ctx);
>  
>  done:
> @@ -187,21 +192,6 @@ done:
>      return res;
>  }
>  
> -static int vp9_superframe_init(AVBSFContext *ctx)
> -{
> -    VP9BSFContext *s = ctx->priv_data;
> -    int n;
> -
> -    // alloc cache packets
> -    for (n = 0; n < MAX_CACHE; n++) {
> -        s->cache[n] = av_packet_alloc();
> -        if (!s->cache[n])
> -            return AVERROR(ENOMEM);
> -    }
> -
> -    return 0;
> -}
> -
>  static void vp9_superframe_close(AVBSFContext *ctx)
>  {
>      VP9BSFContext *s = ctx->priv_data;
> @@ -209,7 +199,7 @@ static void vp9_superframe_close(AVBSFContext *ctx)
>  
>      // free cached data
>      for (n = 0; n < MAX_CACHE; n++)
> -        av_packet_free(&s->cache[n]);
> +        av_buffer_unref(&s->cache[n]);
>  }
>  
>  static const enum AVCodecID codec_ids[] = {
> @@ -220,7 +210,6 @@ const AVBitStreamFilter ff_vp9_superframe_bsf = {
>      .name           = "vp9_superframe",
>      .priv_data_size = sizeof(VP9BSFContext),
>      .filter         = vp9_superframe_filter,
> -    .init           = vp9_superframe_init,
>      .flush          = vp9_superframe_flush,
>      .close          = vp9_superframe_close,
>      .codec_ids      = codec_ids,
>
diff mbox series

Patch

diff --git a/libavcodec/vp9_superframe_bsf.c b/libavcodec/vp9_superframe_bsf.c
index 29d1c759c7..6d6b2c57de 100644
--- a/libavcodec/vp9_superframe_bsf.c
+++ b/libavcodec/vp9_superframe_bsf.c
@@ -27,8 +27,8 @@ 
 
 #define MAX_CACHE 8
 typedef struct VP9BSFContext {
+    AVBufferRef *cache[MAX_CACHE];
     int n_cache;
-    AVPacket *cache[MAX_CACHE];
 } VP9BSFContext;
 
 static void vp9_superframe_flush(AVBSFContext *ctx)
@@ -37,11 +37,11 @@  static void vp9_superframe_flush(AVBSFContext *ctx)
 
     // unref cached data
     for (int n = 0; n < s->n_cache; n++)
-        av_packet_unref(s->cache[n]);
+        av_buffer_unref(&s->cache[n]);
     s->n_cache = 0;
 }
 
-static void stats(AVPacket * const *in, int n_in,
+static void stats(AVBufferRef * const *in, int n_in,
                   unsigned *_max, uint64_t *_sum)
 {
     int n;
@@ -60,10 +60,11 @@  static void stats(AVPacket * const *in, int n_in,
     *_sum = sum;
 }
 
-static int merge_superframe(AVPacket * const *in, int n_in, AVPacket *out)
+static int merge_superframe(AVBufferRef * const *in, int n_in, AVPacket *pkt)
 {
     unsigned max, mag, marker, n;
     uint64_t sum;
+    AVBufferRef *out = NULL;
     uint8_t *ptr;
     int res;
 
@@ -73,7 +74,7 @@  static int merge_superframe(AVPacket * const *in, int n_in, AVPacket *out)
     sum += 2 + (mag + 1) * n_in;
     if (sum > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE)
         return AVERROR(ERANGE);
-    res = av_new_packet(out, sum);
+    res = ff_buffer_padded_realloc(&out, sum);
     if (res < 0)
         return res;
     ptr = out->data;
@@ -107,7 +108,9 @@  static int merge_superframe(AVPacket * const *in, int n_in, AVPacket *out)
         break;
     }
     *ptr++ = marker;
-    av_assert0(ptr == &out->data[out->size]);
+
+    ff_packet_replace_buffer(pkt, out);
+    av_assert0(ptr == &pkt->data[pkt->size]);
 
     return 0;
 }
@@ -166,19 +169,21 @@  static int vp9_superframe_filter(AVBSFContext *ctx, AVPacket *pkt)
 
     av_assert0(s->n_cache < MAX_CACHE);
 
-    av_packet_move_ref(s->cache[s->n_cache++], pkt);
+    // move the packet's buffer into the cache
+    pkt->buf->data = pkt->data;
+    pkt->buf->size = pkt->size;
+    s->cache[s->n_cache++] = pkt->buf;
+    pkt->buf = NULL;
 
     if (invisible) {
-        return AVERROR(EAGAIN);
+        res = AVERROR(EAGAIN);
+        goto done;
     }
     av_assert0(s->n_cache > 0);
 
     // build superframe
     res = merge_superframe(s->cache, s->n_cache, pkt);
 
-    if (res >= 0)
-        res = av_packet_copy_props(pkt, s->cache[s->n_cache - 1]);
-
     vp9_superframe_flush(ctx);
 
 done:
@@ -187,21 +192,6 @@  done:
     return res;
 }
 
-static int vp9_superframe_init(AVBSFContext *ctx)
-{
-    VP9BSFContext *s = ctx->priv_data;
-    int n;
-
-    // alloc cache packets
-    for (n = 0; n < MAX_CACHE; n++) {
-        s->cache[n] = av_packet_alloc();
-        if (!s->cache[n])
-            return AVERROR(ENOMEM);
-    }
-
-    return 0;
-}
-
 static void vp9_superframe_close(AVBSFContext *ctx)
 {
     VP9BSFContext *s = ctx->priv_data;
@@ -209,7 +199,7 @@  static void vp9_superframe_close(AVBSFContext *ctx)
 
     // free cached data
     for (n = 0; n < MAX_CACHE; n++)
-        av_packet_free(&s->cache[n]);
+        av_buffer_unref(&s->cache[n]);
 }
 
 static const enum AVCodecID codec_ids[] = {
@@ -220,7 +210,6 @@  const AVBitStreamFilter ff_vp9_superframe_bsf = {
     .name           = "vp9_superframe",
     .priv_data_size = sizeof(VP9BSFContext),
     .filter         = vp9_superframe_filter,
-    .init           = vp9_superframe_init,
     .flush          = vp9_superframe_flush,
     .close          = vp9_superframe_close,
     .codec_ids      = codec_ids,