[FFmpeg-devel,36/39] lavc: add private container FIFO API

It provides a FIFO for "container" objects like AVFrame/AVPacket and
features an integrated FFRefStructPool-based pool to avoid allocating an
freeing them repeatedly.
---
 libavcodec/container_fifo.c | 195 ++++++++++++++++++++++++++++++++++++
 libavcodec/container_fifo.h |  87 ++++++++++++++++
 2 files changed, 282 insertions(+)
 create mode 100644 libavcodec/container_fifo.c
 create mode 100644 libavcodec/container_fifo.h

Anton Khirnov:
> It provides a FIFO for "container" objects like AVFrame/AVPacket and
> features an integrated FFRefStructPool-based pool to avoid allocating an
> freeing them repeatedly.
> ---
>  libavcodec/container_fifo.c | 195 ++++++++++++++++++++++++++++++++++++
>  libavcodec/container_fifo.h |  87 ++++++++++++++++
>  2 files changed, 282 insertions(+)
>  create mode 100644 libavcodec/container_fifo.c
>  create mode 100644 libavcodec/container_fifo.h
> 
> diff --git a/libavcodec/container_fifo.c b/libavcodec/container_fifo.c
> new file mode 100644
> index 0000000000..86ed15627b
> --- /dev/null
> +++ b/libavcodec/container_fifo.c
> @@ -0,0 +1,195 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "libavutil/error.h"
> +#include "libavutil/fifo.h"
> +#include "libavutil/frame.h"
> +#include "libavutil/mem.h"
> +
> +#include "container_fifo.h"
> +#include "refstruct.h"
> +
> +struct ContainerFifo {
> +    AVFifo             *fifo;
> +    FFRefStructPool    *pool;
> +
> +    void*             (*container_alloc)(void);
> +    void              (*container_reset)(void *obj);
> +    void              (*container_free) (void *obj);
> +    int               (*fifo_write)     (void *dst, void *src);
> +    int               (*fifo_read)      (void *dst, void *src);
> +
> +};
> +
> +static int container_fifo_init_entry(FFRefStructOpaque opaque, void *obj)
> +{
> +    ContainerFifo *cf = opaque.nc;
> +    void **pobj = obj;
> +
> +    *pobj = cf->container_alloc();
> +    if (!*pobj)
> +        return AVERROR(ENOMEM);
> +
> +    return 0;
> +}
> +
> +static void container_fifo_reset_entry(FFRefStructOpaque opaque, void *obj)
> +{
> +    ContainerFifo *cf = opaque.nc;
> +    cf->container_reset(*(void**)obj);
> +}
> +
> +static void container_fifo_free_entry(FFRefStructOpaque opaque, void *obj)
> +{
> +    ContainerFifo *cf = opaque.nc;
> +    cf->container_free(*(void**)obj);
> +}

container_fifo_(init|reset|free)_entry seem unnecessary if you simply
expected the user to already provide a RefStruct-compatible callback.

> +
> +ContainerFifo*
> +ff_container_fifo_alloc(void* (*container_alloc)(void),
> +                        void  (*container_reset)(void *obj),
> +                        void  (*container_free) (void *obj),
> +                        int   (*fifo_write)     (void *dst, void *src),
> +                        int   (*fifo_read)      (void *dst, void *src))
> +{
> +    ContainerFifo *cf;
> +
> +    cf = av_mallocz(sizeof(*cf));
> +    if (!cf)
> +        return NULL;
> +
> +    cf->container_alloc = container_alloc;
> +    cf->container_reset = container_reset;
> +    cf->container_free  = container_free;
> +    cf->fifo_write      = fifo_write;
> +    cf->fifo_read       = fifo_read;
> +
> +    cf->fifo = av_fifo_alloc2(1, sizeof(void*), AV_FIFO_FLAG_AUTO_GROW);
> +    if (!cf->fifo)
> +        goto fail;
> +
> +    cf->pool = ff_refstruct_pool_alloc_ext(sizeof(void*), 0, cf,
> +                                           container_fifo_init_entry,
> +                                           container_fifo_reset_entry,
> +                                           container_fifo_free_entry,
> +                                           NULL);
> +    if (!cf->pool)
> +        goto fail;
> +
> +    return cf;
> +fail:
> +    ff_container_fifo_free(&cf);
> +    return NULL;
> +}
> +
> +void ff_container_fifo_free(ContainerFifo **pcf)
> +{
> +    ContainerFifo *cf;
> +
> +    if (!*pcf)
> +        return;
> +
> +    cf = *pcf;
> +
> +    if (cf->fifo) {
> +        void *obj;
> +        while (av_fifo_read(cf->fifo, &obj, 1) >= 0)
> +            ff_refstruct_unref(&obj);
> +        av_fifo_freep2(&cf->fifo);
> +    }
> +
> +    ff_refstruct_pool_uninit(&cf->pool);
> +
> +    av_freep(pcf);
> +}
> +
> +int ff_container_fifo_read(ContainerFifo *cf, void *obj)
> +{
> +    void **psrc;
> +    int ret;
> +
> +    ret = av_fifo_read(cf->fifo, &psrc, 1);
> +    if (ret < 0)
> +        return ret;
> +
> +    ret = cf->fifo_read(obj, *psrc);
> +    ff_refstruct_unref(&psrc);
> +
> +    return ret;
> +}
> +
> +int ff_container_fifo_write(ContainerFifo *cf, void *obj)
> +{
> +    void **pdst;
> +    int ret;
> +
> +    pdst = ff_refstruct_pool_get(cf->pool);
> +    if (!pdst)
> +        return AVERROR(ENOMEM);
> +
> +    ret = cf->fifo_write(*pdst, obj);

This API design presumes that one never one to "move" an object into the
fifo. This need not be true at all (indeed, the reference from the
frame_grain frame could be directly moved; it is not used for the
decoding process).

> +    if (ret < 0)
> +        goto fail;
> +
> +    ret = av_fifo_write(cf->fifo, &pdst, 1);
> +    if (ret < 0)
> +        goto fail;
> +
> +    return 0;
> +fail:
> +    ff_refstruct_unref(&pdst);
> +    return ret;
> +}
> +
> +size_t ff_container_fifo_can_read(ContainerFifo *cf)
> +{
> +    return av_fifo_can_read(cf->fifo);
> +}
> +
> +static void* frame_alloc(void)

Inconsistent placement of *

> +{
> +    return av_frame_alloc();
> +}
> +
> +static void frame_reset(void *obj)
> +{
> +    av_frame_unref(obj);
> +}
> +
> +static void frame_free(void *obj)
> +{
> +    AVFrame *frame = obj;
> +    av_frame_free(&frame);
> +}
> +
> +static int frame_ref(void *dst, void *src)
> +{
> +    return av_frame_ref(dst, src);
> +}
> +
> +static int frame_move_ref(void *dst, void *src)
> +{
> +    av_frame_move_ref(dst, src);
> +    return 0;
> +}
> +
> +ContainerFifo *ff_container_fifo_alloc_avframe(unsigned flags)
> +{
> +    return ff_container_fifo_alloc(frame_alloc, frame_reset, frame_free,
> +                                   frame_ref, frame_move_ref);
> +}
> diff --git a/libavcodec/container_fifo.h b/libavcodec/container_fifo.h
> new file mode 100644
> index 0000000000..75a8f9e86b
> --- /dev/null
> +++ b/libavcodec/container_fifo.h
> @@ -0,0 +1,87 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVCODEC_CONTAINER_FIFO_H
> +#define AVCODEC_CONTAINER_FIFO_H
> +
> +/**
> + * ContainerFifo is a FIFO for "containers" - dynamically allocated reusable
> + * structs (e.g. AVFrame or AVPacket). ContainerFifo uses an internal pool of
> + * such containers to avoid allocating and freeing them repeatedly.
> + */
> +typedef struct ContainerFifo ContainerFifo;
> +
> +/**
> + * Allocate a new ContainerFifo for the container type defined by provided
> + * callbacks.
> + *
> + * @param container_alloc allocate a new container instance and return a pointer
> + *                        to it, or NULL on failure
> + * @param container_reset reset the provided container instance to a clean state
> + * @param container_free free the provided container instance
> + * @param fifo_write transfer the contents of src to dst, where src is a
> + *                   container instance provided to ff_container_fifo_write()
> + * @param fifo_read transfer the contents of src to dst in other cases
> + *
> + * @note fifo_read() and fifo_write() are different parameters in order to allow
> + *       fifo_write() implementations that make a new reference in dst, leaving
> + *       src untouched (see e.g. ff_container_fifo_alloc_avframe())
> + */
> +ContainerFifo*
> +ff_container_fifo_alloc(void* (*container_alloc)(void),
> +                        void  (*container_reset)(void *obj),
> +                        void  (*container_free) (void *obj),
> +                        int   (*fifo_write)     (void *dst, void *src),
> +                        int   (*fifo_read)      (void *dst, void *src));
> +
> +/**
> + * Allocate a ContainerFifo instance for AVFrames.
> + * Note that ff_container_fifo_write() will call av_frame_ref() on src, making a
> + * new reference in dst and leaving src untouched.
> + *
> + * @param flags unused currently
> + */
> +ContainerFifo *ff_container_fifo_alloc_avframe(unsigned flags);
> +
> +/**
> + * Free a ContainerFifo and everything in it.
> + */
> +void ff_container_fifo_free(ContainerFifo **pf);
> +
> +/**
> + * Write the contents of obj to the FIFO.
> + *
> + * The fifo_write() callback previously provided to ff_container_fifo_alloc()
> + * will be called with obj as src in order to perform the actual transfer.
> + */
> +int ff_container_fifo_write(ContainerFifo *pf, void *obj);
> +
> +/**
> + * Read the next available object from the FIFO into obj.
> + *
> + * The fifo_read() callback previously provided to ff_container_fifo_alloc()
> + * will be called with obj as dst in order to perform the actual transfer.
> + */
> +int ff_container_fifo_read(ContainerFifo *pf, void *obj);
> +
> +/**
> + * @return number of objects available for reading
> + */
> +size_t ff_container_fifo_can_read(ContainerFifo *pf);
> +
> +#endif // AVCODEC_CONTAINER_FIFO_H

Quoting Andreas Rheinhardt (2024-08-10 02:09:19)
> Anton Khirnov:
> > +static int container_fifo_init_entry(FFRefStructOpaque opaque, void *obj)
> > +{
> > +    ContainerFifo *cf = opaque.nc;
> > +    void **pobj = obj;
> > +
> > +    *pobj = cf->container_alloc();
> > +    if (!*pobj)
> > +        return AVERROR(ENOMEM);
> > +
> > +    return 0;
> > +}
> > +
> > +static void container_fifo_reset_entry(FFRefStructOpaque opaque, void *obj)
> > +{
> > +    ContainerFifo *cf = opaque.nc;
> > +    cf->container_reset(*(void**)obj);
> > +}
> > +
> > +static void container_fifo_free_entry(FFRefStructOpaque opaque, void *obj)
> > +{
> > +    ContainerFifo *cf = opaque.nc;
> > +    cf->container_free(*(void**)obj);
> > +}
> 
> container_fifo_(init|reset|free)_entry seem unnecessary if you simply
> expected the user to already provide a RefStruct-compatible callback.

I considered that, but decided against it as it would leak
internal implementation details to callers.

> > +int ff_container_fifo_write(ContainerFifo *cf, void *obj)
> > +{
> > +    void **pdst;
> > +    int ret;
> > +
> > +    pdst = ff_refstruct_pool_get(cf->pool);
> > +    if (!pdst)
> > +        return AVERROR(ENOMEM);
> > +
> > +    ret = cf->fifo_write(*pdst, obj);
> 
> This API design presumes that one never one to "move" an object into the
> fifo. This need not be true at all (indeed, the reference from the
> frame_grain frame could be directly moved; it is not used for the
> decoding process).

I don't follow, how does the API preclude moving container contents in
the fifo_write() callback? Unless you mean moving the actual container?

> > +    if (ret < 0)
> > +        goto fail;
> > +
> > +    ret = av_fifo_write(cf->fifo, &pdst, 1);
> > +    if (ret < 0)
> > +        goto fail;
> > +
> > +    return 0;
> > +fail:
> > +    ff_refstruct_unref(&pdst);
> > +    return ret;
> > +}
> > +
> > +size_t ff_container_fifo_can_read(ContainerFifo *cf)
> > +{
> > +    return av_fifo_can_read(cf->fifo);
> > +}
> > +
> > +static void* frame_alloc(void)
> 
> Inconsistent placement of *

Fixed locally.

ping

Quoting Anton Khirnov (2024-08-16 10:44:42)
> ping

Will push the rest of this set in a couple days unless there are further
comments.