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".