Message ID | 20240716171155.31838-36-anton@khirnov.net |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,01/39] tests/fate/vcodec: add vsynth tests for FFV1 version 2 | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
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.
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); +} + +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); + 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) +{ + 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