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

[Anton Khirnov]

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.

-- 
Anton Khirnov