[FFmpeg-devel] [PATCH v2] avcodec/bsf: restructure the internal implementation of the bistream filter API

[James Almer]

On 4/19/2020 11:34 AM, James Almer wrote:
> Process input data as soon as it's fed to av_bsf_send_packet(), instead of
> storing a single packet and expecting the user to call av_bsf_receive_packet()
> in order to trigger the decoding process before they are allowed to feed more
> data.

This is no longer the case in v2, sorry for the confusion.

av_bsf_send_packet() will trigger filtering only when called more than
one time in a row (When it can't consume the new packet without trying
to filter the previous one first), to keep behavior for existing API
users intact.

> 
> This puts the bsf API more in line with the decoupled decode API, without
> breaking existing using it.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
> Now without creating a new reference for the input packet, and behaving the
> exact same as before for current users that were following the doxy right down
> to the letter.
> 
> Also didn't rename buffer_pkt to reduce the size of the diff and easily find
> the actual changes and additions.
> 
>  libavcodec/avcodec.h |  6 +-----
>  libavcodec/bsf.c     | 42 +++++++++++++++++++++++++++++++-----------
>  2 files changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index b79b025e53..af2a1b0e90 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -4735,10 +4735,6 @@ int av_bsf_init(AVBSFContext *ctx);
>  /**
>   * Submit a packet for filtering.
>   *
> - * After sending each packet, the filter must be completely drained by calling
> - * av_bsf_receive_packet() repeatedly until it returns AVERROR(EAGAIN) or
> - * AVERROR_EOF.
> - *
>   * @param pkt the packet to filter. The bitstream filter will take ownership of
>   * the packet and reset the contents of pkt. pkt is not touched if an error occurs.
>   * If pkt is empty (i.e. NULL, or pkt->data is NULL and pkt->side_data_elems zero),
> @@ -4770,7 +4766,7 @@ int av_bsf_send_packet(AVBSFContext *ctx, AVPacket *pkt);
>   * an error occurs.
>   *
>   * @note one input packet may result in several output packets, so after sending
> - * a packet with av_bsf_send_packet(), this function needs to be called
> + * a packet with av_bsf_send_packet(), this function may need to be called
>   * repeatedly until it stops returning 0. It is also possible for a filter to
>   * output fewer packets than were sent to it, so this function may return
>   * AVERROR(EAGAIN) immediately after a successful av_bsf_send_packet() call.
> diff --git a/libavcodec/bsf.c b/libavcodec/bsf.c
> index b9fc771a88..c79a8ebbdf 100644
> --- a/libavcodec/bsf.c
> +++ b/libavcodec/bsf.c
> @@ -30,6 +30,7 @@
>  
>  struct AVBSFInternal {
>      AVPacket *buffer_pkt;
> +    AVPacket *out_pkt;
>      int eof;
>  };
>  
> @@ -46,8 +47,10 @@ void av_bsf_free(AVBSFContext **pctx)
>      if (ctx->filter->priv_class && ctx->priv_data)
>          av_opt_free(ctx->priv_data);
>  
> -    if (ctx->internal)
> +    if (ctx->internal) {
>          av_packet_free(&ctx->internal->buffer_pkt);
> +        av_packet_free(&ctx->internal->out_pkt);
> +    }
>      av_freep(&ctx->internal);
>      av_freep(&ctx->priv_data);
>  
> @@ -112,7 +115,8 @@ int av_bsf_alloc(const AVBitStreamFilter *filter, AVBSFContext **pctx)
>      ctx->internal = bsfi;
>  
>      bsfi->buffer_pkt = av_packet_alloc();
> -    if (!bsfi->buffer_pkt) {
> +    bsfi->out_pkt = av_packet_alloc();
> +    if (!bsfi->buffer_pkt || !bsfi->out_pkt) {
>          ret = AVERROR(ENOMEM);
>          goto fail;
>      }
> @@ -185,6 +189,7 @@ void av_bsf_flush(AVBSFContext *ctx)
>      bsfi->eof = 0;
>  
>      av_packet_unref(bsfi->buffer_pkt);
> +    av_packet_unref(bsfi->out_pkt);
>  
>      if (ctx->filter->flush)
>          ctx->filter->flush(ctx);
> @@ -205,10 +210,21 @@ int av_bsf_send_packet(AVBSFContext *ctx, AVPacket *pkt)
>          return AVERROR(EINVAL);
>      }
>  
> +    if (!bsfi->buffer_pkt->data &&
> +        !bsfi->buffer_pkt->side_data_elems)
> +        goto end;
> +
> +    if (!bsfi->out_pkt->data && !bsfi->out_pkt->side_data_elems) {
> +        ret = ctx->filter->filter(ctx, bsfi->out_pkt);
> +        if (ret < 0 && ret != AVERROR(EAGAIN) && ret != AVERROR_EOF)
> +            return ret;
> +    }
> +
>      if (bsfi->buffer_pkt->data ||
>          bsfi->buffer_pkt->side_data_elems)
>          return AVERROR(EAGAIN);
>  
> +end:
>      ret = av_packet_make_refcounted(pkt);
>      if (ret < 0)
>          return ret;
> @@ -219,7 +235,17 @@ int av_bsf_send_packet(AVBSFContext *ctx, AVPacket *pkt)
>  
>  int av_bsf_receive_packet(AVBSFContext *ctx, AVPacket *pkt)
>  {
> -    return ctx->filter->filter(ctx, pkt);
> +    AVBSFInternal *bsfi = ctx->internal;
> +    int ret;
> +
> +    if (!bsfi->out_pkt->data && !bsfi->out_pkt->side_data_elems) {
> +        ret = ctx->filter->filter(ctx, bsfi->out_pkt);
> +        if (ret < 0)
> +            return ret;
> +    }
> +    av_packet_move_ref(pkt, bsfi->out_pkt);
> +
> +    return 0;
>  }
>  
>  int ff_bsf_get_packet(AVBSFContext *ctx, AVPacket **pkt)
> @@ -227,12 +253,9 @@ int ff_bsf_get_packet(AVBSFContext *ctx, AVPacket **pkt)
>      AVBSFInternal *bsfi = ctx->internal;
>      AVPacket *tmp_pkt;
>  
> -    if (bsfi->eof)
> -        return AVERROR_EOF;
> -
>      if (!bsfi->buffer_pkt->data &&
>          !bsfi->buffer_pkt->side_data_elems)
> -        return AVERROR(EAGAIN);
> +        return bsfi->eof ? AVERROR_EOF : AVERROR(EAGAIN);
>  
>      tmp_pkt = av_packet_alloc();
>      if (!tmp_pkt)
> @@ -248,12 +271,9 @@ int ff_bsf_get_packet_ref(AVBSFContext *ctx, AVPacket *pkt)
>  {
>      AVBSFInternal *bsfi = ctx->internal;
>  
> -    if (bsfi->eof)
> -        return AVERROR_EOF;
> -
>      if (!bsfi->buffer_pkt->data &&
>          !bsfi->buffer_pkt->side_data_elems)
> -        return AVERROR(EAGAIN);
> +        return bsfi->eof ? AVERROR_EOF : AVERROR(EAGAIN);
>  
>      av_packet_move_ref(pkt, bsfi->buffer_pkt);
>  
>