[FFmpeg-devel] [PATCH v2] avcodec/bsf: restructure the internal implementation of the bistream filter API
Marton Balint
cus at passwd.hu
Sun Apr 19 19:01:56 EEST 2020
On Sun, 19 Apr 2020, 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 puts the bsf API more in line with the decoupled decode API, without
> breaking existing using it.
Well, previously it was assumed that av_bsf_send_packet() is never
returning AVERROR_INVALIDDATA, that is no longer the case. That matters
because current code in mux.c ingnores av_bsf_receive_packet() errors but
not av_bsf_send_packet() errors.
Unless there is some benefit I am not seeing, I'd rather keep things as
is. Sorry.
Regards,
Marton
>
> 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);
>
> --
> 2.26.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list