Anton Khirnov:
> Quoting Andreas Rheinhardt (2023-10-12 15:51:18)
>> Anton Khirnov:
>>> Quoting Andreas Rheinhardt (2023-09-19 21:57:16)
>>>> To do this, make FFRefStructPool itself refcounted according
>>>> to the RefStruct API.
>>>>
>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
>>>> ---
>>>> libavcodec/refstruct.c | 29 ++++++++++++++++-------------
>>>> libavcodec/refstruct.h | 5 ++++-
>>>> 2 files changed, 20 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/libavcodec/refstruct.c b/libavcodec/refstruct.c
>>>> index f8d040874d..2108ff8163 100644
>>>> --- a/libavcodec/refstruct.c
>>>> +++ b/libavcodec/refstruct.c
>>>> @@ -187,7 +187,7 @@ static void pool_free(FFRefStructPool *pool)
>>>> pthread_mutex_destroy(&pool->mutex);
>>>> if (pool->free_cb)
>>>> pool->free_cb(pool->opaque);
>>>> - av_free(pool);
>>>> + av_free(get_refcount(pool));
>>>> }
>>>>
>>>> static void pool_free_entry(FFRefStructPool *pool, RefCount *ref)
>>>> @@ -278,13 +278,17 @@ void *ff_refstruct_pool_get(FFRefStructPool *pool)
>>>> return ret;
>>>> }
>>>>
>>>> -void ff_refstruct_pool_uninit(FFRefStructPool **poolp)
>>>> +static void pool_unref(void *ref)
>>>> {
>>>> - FFRefStructPool *pool = *poolp;
>>>> - RefCount *entry;
>>>> + FFRefStructPool *pool = get_userdata(ref);
>>>> + if (atomic_fetch_sub_explicit(&pool->refcount, 1,
>>>> memory_order_acq_rel) == 1)
>>>
>>> Is there a reason you cannot fold pool->refcount into the pool's
>>> containing RefStruct?
>>>
>>
>> If I simply incremented the pool's refcount for every entry currently in
>> use by users, then the entries would only be freed when the last entry
>> has been returned and all the references to the pool unreferenced.
>
> Ok, can you please mention this in a comment somewhere? It's quite
> non-obvious why do both pool_unref() and refstruct_pool_uninit() exist.
>
>> In fact, when I did this, I pondered two things: Shall I make
>> ff_refstruct_pool_uninit() free all the currently available buffers and
>> then unreference the caller's reference or shall I just make it a
>> wrapper to ff_refstruct_unref() to decrement the pool's refcount? The
>> latter is very simple and I did it; the former could be advantageous in
>> particular in case of frame-threading in case the dimensions change. (In
>> this scenario, no user will ever create new entries after the first user
>> unreferences a pool.)
>
> But in other scenarios you might want to get rid of some pool references
> while still using the others, so maybe that should be a flag if
> anything.
>
If I added a non-inlined ff_refstruct_pool_uninit(), I could simply
still allow to call ff_refstruct_unref() on pool-references, so that one
can get both behaviours. (Notice that this allows a malicious owner of a
pool-reference to drain the pool ad libitum, but you can do way worse
stuff with a reference than this.)
- 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-requ...@ffmpeg.org with subject "unsubscribe".
