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

[James Almer]

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 at 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,
>