Re: [FFmpeg-devel] [PATCH 02/42] avcodec/refstruct: Add simple API for refcounted objects

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Andreas Rheinhardt:
> For now, this API is supposed to replace all the internal uses
> of reference counted objects in libavcodec; "internal" here
> means that the object is created in libavcodec and is never
> put directly in the hands of anyone outside of it.
> 
> It is intended to be made public eventually, but for now
> I enjoy the ability to modify it freely.
> 
> Several shortcomings of the AVBuffer API motivated this API:
> a) The unnecessary allocations (and ensuing error checks)
> when using the API. Besides the need for runtime checks it
> imposes upon the developer the burden of thinking through
> what happens in case an error happens. Furthermore, these
> error paths are typically not covered by FATE.
> b) The AVBuffer API is designed with buffers and not with
> objects in mind: The type for the actual buffers used
> is uint8_t*; it pretends to be able to make buffers
> writable, but this is wrong in case the buffer is not a POD.
> Another instance of this thinking is the lack of a reset
> callback in the AVBufferPool API.
> c) The AVBuffer API incurs unnecessary indirections by
> going through the AVBufferRef.data pointer. In case the user
> tries to avoid this indirection and stores a pointer to
> AVBuffer.data separately (which also allows to use the correct
> type), the user has to keep these two pointers in sync
> in case they can change (and in any case has two pointers
> occupying space in the containing context). See the following
> commit using this API for H.264 parameter sets for an example
> of the removal of such syncing code as well as the casts
> involved in the parts where only the AVBufferRef* pointer
> was stored.
> d) Given that the AVBuffer API allows custom allocators,
> creating refcounted objects with dedicated free functions
> often involves a lot of boilerplate like this:
> obj = av_mallocz(sizeof(*obj));
> ref = av_buffer_create((uint8_t*)obj, sizeof(*obj), free_func, opaque, 0);
> if (!ref) {
>     av_free(obj);
>     return AVERROR(ENOMEM);
> }
> (There is also a corresponding av_free() at the end of free_func().)
> This is now just
> obj = ff_refstruct_alloc_ext(sizeof(*obj), 0, opaque, free_func);
> if (!obj)
>     return AVERROR(ENOMEM);
> See the subsequent patch for the framepool (i.e. get_buffer.c)
> for an example.
> 
> This API does things differently; it is designed to be lightweight*
> as well as geared to the common case where the allocator of the
> underlying object does not matter as long as it is big enough and
> suitably aligned. This allows to allocate the user data together
> with the API's bookkeeping data which avoids an allocation as well
> as the need for separate pointers to the user data and the API's
> bookkeeping data. This entails that the actual allocation of the
> object is performed by refstruct, not the user. This is responsible
> for avoiding the boilerplate code mentioned in d).
> 
> As a downside, custom allocators are not supported, but it will
> become apparent in subsequent commits that there are enough
> usecases to make it worthwhile.
> 
> Another advantage of this API is that one only needs to include
> the relevant header if one uses the API and not when one includes
> the header or some other component that uses it. This is because there
> is no refstruct type analog of AVBufferRef. This brings with it
> one further downside: It is not apparent from the pointer itself
> whether the underlying object is managed by the refstruct API
> or whether this pointer is a reference to it (or merely a pointer
> to it).
> 
> Finally, this API supports const-qualified opaque pointees;
> this will allow to avoid casting const away by the CBS code.
> 
> *: Basically the only exception to the you-only-pay-for-what-you-use
> rule is that it always uses atomics for the refcount.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> I am the most unsure about whether to use FFRefStructOpaque
> at all or not just a void*; the only beneficiary of this
> is CBS where it saves casting one const away.
> 
> I am also open to other naming suggestions like RefObject
> (RefObj?) for this API.
> 
>  libavcodec/Makefile    |   1 +
>  libavcodec/refstruct.c | 139 +++++++++++++++++++++++++++++++++++++++
>  libavcodec/refstruct.h | 145 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 285 insertions(+)
>  create mode 100644 libavcodec/refstruct.c
>  create mode 100644 libavcodec/refstruct.h
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index bf3b0a93f9..7541f38535 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -55,6 +55,7 @@ OBJS = ac3_parser.o                                                     \
>         profiles.o                                                       \
>         qsv_api.o                                                        \
>         raw.o                                                            \
> +       refstruct.o                                                      \
>         utils.o                                                          \
>         version.o                                                        \
>         vlc.o                                                            \
> diff --git a/libavcodec/refstruct.c b/libavcodec/refstruct.c
> new file mode 100644
> index 0000000000..917cf6b7ac
> --- /dev/null
> +++ b/libavcodec/refstruct.c
> @@ -0,0 +1,139 @@
> +/*
> + * 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 <stdatomic.h>
> +#include <stdint.h>
> +#include <string.h>
> +
> +#include "internal.h"
> +#include "refstruct.h"
> +
> +#include "libavutil/macros.h"
> +#include "libavutil/mem.h"
> +
> +typedef struct RefCount {
> +    /**
> +     * An uintptr_t is big enough to hold the address of every reference,
> +     * so no overflow can happen when incrementing the refcount as long as
> +     * the user does not throw away references.
> +     */
> +    atomic_uintptr_t  refcount;
> +    FFRefStructOpaque opaque;
> +    void (*free_cb)(FFRefStructOpaque opaque, void *obj);
> +} RefCount;
> +
> +#if __STDC_VERSION__ >= 201112L
> +#define REFCOUNT_OFFSET FFALIGN(sizeof(RefCount), FFMAX3(STRIDE_ALIGN, 16, _Alignof(max_align_t)))
> +#else
> +#define REFCOUNT_OFFSET FFALIGN(sizeof(RefCount), FFMAX(STRIDE_ALIGN, 16))
> +#endif
> +
> +static RefCount *get_refcount(void *obj)
> +{
> +    return (RefCount*)((char*)obj - REFCOUNT_OFFSET);
> +}
> +
> +static void *get_userdata(void *buf)
> +{
> +    return (char*)buf + REFCOUNT_OFFSET;
> +}
> +
> +static void refcount_init(RefCount *ref, FFRefStructOpaque opaque,
> +                          void (*free_cb)(FFRefStructOpaque opaque, void *obj))
> +{
> +    atomic_init(&ref->refcount, 1);
> +    ref->opaque  = opaque;
> +    ref->free_cb = free_cb;
> +}
> +
> +void *ff_refstruct_alloc_ext_c(size_t size, unsigned flags, FFRefStructOpaque opaque,
> +                               void (*free_cb)(FFRefStructOpaque opaque, void *obj))
> +{
> +    void *buf, *obj;
> +
> +    if (size > SIZE_MAX - REFCOUNT_OFFSET)
> +        return NULL;
> +    buf = av_malloc(size + REFCOUNT_OFFSET);
> +    if (!buf)
> +        return NULL;
> +    refcount_init(buf, opaque, free_cb);
> +    obj = get_userdata(buf);
> +    if (!(flags & FF_REFSTRUCT_FLAG_NO_ZEROING))
> +        memset(obj, 0, size);
> +
> +    return obj;
> +}
> +
> +void *ff_refstruct_allocz(size_t size)
> +{
> +    return ff_refstruct_alloc_ext(size, 0, NULL, NULL);
> +}
> +
> +void ff_refstruct_unref(void *objp)
> +{
> +    void *obj;
> +    RefCount *ref;
> +
> +    memcpy(&obj, objp, sizeof(obj));
> +    if (!obj)
> +        return;
> +    memcpy(objp, &(void *){ NULL }, sizeof(obj));
> +
> +    ref = get_refcount(obj);
> +    if (atomic_fetch_sub_explicit(&ref->refcount, 1, memory_order_acq_rel) == 1) {
> +        if (ref->free_cb)
> +            ref->free_cb(ref->opaque, obj);
> +        av_free(ref);
> +    }
> +
> +    return;
> +}
> +
> +void *ff_refstruct_ref(void *obj)
> +{
> +    RefCount *ref = get_refcount(obj);
> +
> +    atomic_fetch_add_explicit(&ref->refcount, 1, memory_order_relaxed);
> +
> +    return obj;
> +}
> +
> +const void *ff_refstruct_ref_c(const void *obj)
> +{
> +    /* Casting const away here is fine, as it is only supposed
> +     * to apply to the user's data and not our bookkeeping data. */
> +    RefCount *ref = get_refcount((void*)obj);
> +
> +    atomic_fetch_add_explicit(&ref->refcount, 1, memory_order_relaxed);
> +
> +    return obj;
> +}
> +
> +void ff_refstruct_replace(void *dstp, const void *src)
> +{
> +    const void *dst;
> +    memcpy(&dst, dstp, sizeof(dst));
> +
> +    if (src == dst)
> +        return;
> +    ff_refstruct_unref(dstp);
> +    if (src) {
> +        dst = ff_refstruct_ref_c(src);
> +        memcpy(dstp, &dst, sizeof(dst));
> +    }
> +}
> diff --git a/libavcodec/refstruct.h b/libavcodec/refstruct.h
> new file mode 100644
> index 0000000000..0086717c17
> --- /dev/null
> +++ b/libavcodec/refstruct.h
> @@ -0,0 +1,145 @@
> +/*
> + * 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_REFSTRUCT_H
> +#define AVCODEC_REFSTRUCT_H
> +
> +#include <stddef.h>
> +
> +/**
> + * RefStruct is an API for creating reference-counted objects
> + * with minimal overhead. The API is designed for objects,
> + * not buffers like the AVBuffer API. The main differences
> + * to the AVBuffer API are as follows:
> + *
> + * - It uses void* instead of uint8_t* as its base type due to
> + *   its focus on objects.
> + * - There are no equivalents of AVBuffer and AVBufferRef.
> + *   E.g. there is no way to get the usable size of the object:
> + *   The user is supposed to know what is at the other end of
> + *   the pointer. It also avoids one level of indirection.
> + * - Custom allocators are not supported. This allows to simplify
> + *   the implementation and reduce the amount of allocations.
> + * - It also has the advantage that the user's free callback need
> + *   only free the resources owned by the object, but not the
> + *   object itself.
> + * - Because referencing (and replacing) an object managed by the
> + *   RefStruct API does not involve allocations, they can not fail
> + *   and therefore need not be checked.
> + *
> + * @note Referencing and unreferencing the buffers is thread-safe and thus
> + * may be done from multiple threads simultaneously without any need for
> + * additional locking.
> + */
> +
> +/**
> + * This union is used for all opaque parameters in this API to spare the user
> + * to cast const away in case the opaque to use is const-qualified.
> + *
> + * The functions provided by this API with an FFRefStructOpaque come in pairs
> + * named foo_c and foo. The foo function accepts void* as opaque and is just
> + * a wrapper around the foo_c function; "_c" means "(potentially) const".
> + */
> +typedef union {
> +    void *nc;
> +    const void *c;
> +} FFRefStructOpaque;
> +
> +/**
> + * If this flag is set in ff_refstruct_alloc_ext_c(), the object will not
> + * be initially zeroed.
> + */
> +#define FF_REFSTRUCT_FLAG_NO_ZEROING (1 << 0)
> +
> +/**
> + * Allocate a refcounted object of usable size `size` managed via
> + * the RefStruct API.
> + *
> + * By default (in the absence of flags to the contrary),
> + * the returned object is initially zeroed.
> + *
> + * @param size    Desired usable size of the returned object.
> + * @param flags   A bitwise combination of FF_REFSTRUCT_FLAG_* flags.
> + * @param opaque  A pointer that will be passed to the free_cb callback.
> + * @param free_cb A callback for freeing this object's content
> + *                when its reference count reaches zero;
> + *                it must not free the object itself.
> + * @return A pointer to an object of the desired size or NULL on failure.
> + */
> +void *ff_refstruct_alloc_ext_c(size_t size, unsigned flags, FFRefStructOpaque opaque,
> +                               void (*free_cb)(FFRefStructOpaque opaque, void *obj));
> +
> +/**
> + * A wrapper around ff_refstruct_alloc_ext_c() for the common case
> + * of a non-const qualified opaque.
> + *
> + * @see ff_refstruct_alloc_ext_c()
> + */
> +static inline
> +void *ff_refstruct_alloc_ext(size_t size, unsigned flags, void *opaque,
> +                             void (*free_cb)(FFRefStructOpaque opaque, void *obj))
> +{
> +    return ff_refstruct_alloc_ext_c(size, flags, (FFRefStructOpaque){.nc = opaque},
> +                                    free_cb);
> +}
> +
> +/**
> + * Equivalent to ff_refstruct_alloc_ext(size, 0, NULL, NULL)
> + */
> +void *ff_refstruct_allocz(size_t size);
> +
> +/**
> + * Decrement the reference count of the underlying object and automatically
> + * free the object if there are no more references to it.
> + *
> + * `*objp == NULL` is legal and a no-op.
> + *
> + * @param objp Pointer to a pointer that is either NULL or points to an object
> + *             managed via this API. `*objp` is set to NULL on return.
> + */
> +void ff_refstruct_unref(void *objp);
> +
> +/**
> + * Create a new reference to an object managed via this API,
> + * i.e. increment the reference count of the underlying object
> + * and return obj.
> + * @return a pointer equal to obj.
> + */
> +void *ff_refstruct_ref(void *obj);
> +
> +/**
> + * Analog of ff_refstruct_ref(), but for constant objects.
> + * @see ff_refstruct_ref()
> + */
> +const void *ff_refstruct_ref_c(const void *obj);
> +
> +/**
> + * Ensure `*dstp` refers to the same object as src.
> + *
> + * If `*dstp` is already equal to src, do nothing. Otherwise unreference `*dstp`
> + * and replace it with a new reference to src in case `src != NULL` (this
> + * involves incrementing the reference count of src's underlying object) or
> + * with NULL otherwise.
> + *
> + * @param dstp Pointer to a pointer that is either NULL or points to an object
> + *             managed via this API.
> + * @param src  A pointer to an object managed via this API or NULL.
> + */
> +void ff_refstruct_replace(void *dstp, const void *src);
> +
> +#endif /* AVCODEC_REFSTRUCT_H */

Will apply patches 2-16 (i.e. up to the pool API) tomorrow unless there
are objections.

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".