[vlc-devel] [PATCH 2/7] picture_pool: add the video format requested when waiting for a picture
Steve Lhomme
robux4 at ycbcr.xyz
Mon Nov 9 09:07:02 CET 2020
On 2020-11-06 17:02, Alexandre Janniaux wrote:
> Hi,
>
> I'm not sure this is the correct level of abstraction
> regarding how we discussed this originally. From what
> I understood, we want to change the pool when the format
> change, not adapt the previous pool. The picture_pool
> even facilitates this by being reference-counted.
>
> I guess your main issue with this is that previous pictures
> would not be released until all pictures are released.
> Maybe picture_pool_Release could implement some sort of
> drain to release any current and future requeued picture
> to handle this case smoothly instead? It would highly
> simplify the API for handling this.
This is already handled in the other pool helpers. They are refcounted
and released when a new one is needed (which is rare, as it's only when
the surface/buffer characteristics change). When the last picture of the
defunct pool is released, the pool resources are released as well.
> Regards,
> --
> Alexandre Janniaux
> Videolabs
>
> On Fri, Nov 06, 2020 at 03:42:09PM +0100, Steve Lhomme wrote:
>> In push mode it's convenient to keep the same pool of buffers when only small
>> changes to the video format occur.
>>
>> Passing NULL for the video format means no change to the original format is
>> required.
>> ---
>> include/vlc_picture_pool.h | 2 +-
>> modules/hw/nvdec/hw_pool.c | 4 ++--
>> modules/hw/nvdec/hw_pool.h | 2 +-
>> modules/hw/nvdec/nvdec.c | 2 +-
>> modules/hw/vaapi/chroma.c | 2 +-
>> modules/hw/vaapi/filters.c | 2 +-
>> src/input/decoder.c | 2 +-
>> src/misc/picture.c | 7 ++++---
>> src/misc/picture.h | 2 +-
>> src/misc/picture_pool.c | 13 +++++++------
>> src/test/picture_pool.c | 2 +-
>> src/video_output/video_output.c | 2 +-
>> 12 files changed, 22 insertions(+), 20 deletions(-)
>>
>> diff --git a/include/vlc_picture_pool.h b/include/vlc_picture_pool.h
>> index aa5883ee521..8e325392d2c 100644
>> --- a/include/vlc_picture_pool.h
>> +++ b/include/vlc_picture_pool.h
>> @@ -102,7 +102,7 @@ VLC_API picture_t * picture_pool_Get( picture_pool_t * ) VLC_USED;
>> *
>> * @note This function is thread-safe.
>> */
>> -VLC_API picture_t *picture_pool_Wait(picture_pool_t *) VLC_USED;
>> +VLC_API picture_t *picture_pool_Wait(picture_pool_t *, const video_format_t *) VLC_USED;
>>
>> /**
>> * Cancel the picture pool.
>> diff --git a/modules/hw/nvdec/hw_pool.c b/modules/hw/nvdec/hw_pool.c
>> index 63c6f1911f3..3d117d66ca2 100644
>> --- a/modules/hw/nvdec/hw_pool.c
>> +++ b/modules/hw/nvdec/hw_pool.c
>> @@ -103,9 +103,9 @@ error:
>> return NULL;
>> }
>>
>> -picture_t* nvdec_pool_Wait(nvdec_pool_t *pool)
>> +picture_t* nvdec_pool_Wait(nvdec_pool_t *pool, const video_format_t *fmt)
>> {
>> - picture_t *pic = picture_pool_Wait(pool->picture_pool);
>> + picture_t *pic = picture_pool_Wait(pool->picture_pool, fmt);
>> if (!pic)
>> return NULL;
>>
>> diff --git a/modules/hw/nvdec/hw_pool.h b/modules/hw/nvdec/hw_pool.h
>> index 59362116b3d..15bedfde868 100644
>> --- a/modules/hw/nvdec/hw_pool.h
>> +++ b/modules/hw/nvdec/hw_pool.h
>> @@ -45,4 +45,4 @@ void nvdec_pool_Release(nvdec_pool_t *);
>> *
>> * The picture.p_sys is always NULL.
>> */
>> -picture_t* nvdec_pool_Wait(nvdec_pool_t *);
>> +picture_t* nvdec_pool_Wait(nvdec_pool_t *, const video_format_t *);
>> diff --git a/modules/hw/nvdec/nvdec.c b/modules/hw/nvdec/nvdec.c
>> index ebed90be51b..f16702f89ae 100644
>> --- a/modules/hw/nvdec/nvdec.c
>> +++ b/modules/hw/nvdec/nvdec.c
>> @@ -419,7 +419,7 @@ static int CUDAAPI HandlePictureDisplay(void *p_opaque, CUVIDPARSERDISPINFO *p_d
>>
>> if ( is_nvdec_opaque(p_dec->fmt_out.video.i_chroma) )
>> {
>> - p_pic = nvdec_pool_Wait(p_sys->out_pool);
>> + p_pic = nvdec_pool_Wait(p_sys->out_pool, &p_dec->fmt_out.video);
>> if (unlikely(p_pic == NULL))
>> return 0;
>> pic_context_nvdec_t *picctx = NVDEC_PICCONTEXT_FROM_PICCTX(p_pic->context);
>> diff --git a/modules/hw/vaapi/chroma.c b/modules/hw/vaapi/chroma.c
>> index f43e0dc1cd0..ff33c4a2f26 100644
>> --- a/modules/hw/vaapi/chroma.c
>> +++ b/modules/hw/vaapi/chroma.c
>> @@ -242,7 +242,7 @@ UploadSurface(filter_t *filter, picture_t *src)
>> VADisplay const va_dpy = p_sys->dpy;
>> VAImage dest_img;
>> void * dest_buf;
>> - picture_t * dest_pic = picture_pool_Wait(p_sys->dest_pics);
>> + picture_t * dest_pic = picture_pool_Wait(p_sys->dest_pics, NULL);
>>
>> if (!dest_pic)
>> {
>> diff --git a/modules/hw/vaapi/filters.c b/modules/hw/vaapi/filters.c
>> index bd933ac8c63..9174e21cd4d 100644
>> --- a/modules/hw/vaapi/filters.c
>> +++ b/modules/hw/vaapi/filters.c
>> @@ -211,7 +211,7 @@ Filter(filter_t * filter, picture_t * src,
>> {
>> filter_sys_t *const filter_sys = filter->p_sys;
>> VABufferID pipeline_buf = VA_INVALID_ID;
>> - picture_t *const dest = picture_pool_Wait(filter_sys->dest_pics);
>> + picture_t *const dest = picture_pool_Wait(filter_sys->dest_pics, NULL);
>> if (!dest)
>> return NULL;
>>
>> diff --git a/src/input/decoder.c b/src/input/decoder.c
>> index 77ccd67e530..1610d300ae2 100644
>> --- a/src/input/decoder.c
>> +++ b/src/input/decoder.c
>> @@ -620,7 +620,7 @@ static picture_t *ModuleThread_NewVideoBuffer( decoder_t *p_dec )
>> assert( p_owner->p_vout );
>> assert( p_owner->out_pool );
>>
>> - picture_t *pic = picture_pool_Wait( p_owner->out_pool );
>> + picture_t *pic = picture_pool_Wait( p_owner->out_pool, &p_dec->fmt_out.video );
>> if (pic)
>> picture_Reset( pic );
>> return pic;
>> diff --git a/src/misc/picture.c b/src/misc/picture.c
>> index e00b1303253..6902b9a8fcd 100644
>> --- a/src/misc/picture.c
>> +++ b/src/misc/picture.c
>> @@ -423,7 +423,7 @@ static void picture_DestroyClone(picture_t *clone)
>> picture_Release(picture);
>> }
>>
>> -picture_t *picture_InternalClone(picture_t *picture,
>> +picture_t *picture_InternalClone(picture_t *picture, const video_format_t *fmt,
>> void (*pf_destroy)(picture_t *), void *opaque)
>> {
>> picture_resource_t res = {
>> @@ -437,7 +437,7 @@ picture_t *picture_InternalClone(picture_t *picture,
>> res.p[i].i_pitch = picture->p[i].i_pitch;
>> }
>>
>> - picture_t *clone = picture_NewFromResource(&picture->format, &res);
>> + picture_t *clone = picture_NewFromResource(fmt ? fmt : &picture->format, &res);
>> if (likely(clone != NULL)) {
>> ((picture_priv_t *)clone)->gc.opaque = opaque;
>> picture_Hold(picture);
>> @@ -447,7 +447,8 @@ picture_t *picture_InternalClone(picture_t *picture,
>>
>> picture_t *picture_Clone(picture_t *picture)
>> {
>> - picture_t *clone = picture_InternalClone(picture, picture_DestroyClone, picture);
>> + picture_t *clone = picture_InternalClone(picture, NULL,
>> + picture_DestroyClone, picture);
>> if (likely(clone != NULL)) {
>> if (picture->context != NULL)
>> clone->context = picture->context->copy(picture->context);
>> diff --git a/src/misc/picture.h b/src/misc/picture.h
>> index 684eb2ada69..a815bb8a316 100644
>> --- a/src/misc/picture.h
>> +++ b/src/misc/picture.h
>> @@ -36,4 +36,4 @@ typedef struct
>> void *picture_Allocate(int *, size_t);
>> void picture_Deallocate(int, void *, size_t);
>>
>> -picture_t * picture_InternalClone(picture_t *, void (*pf_destroy)(picture_t *), void *);
>> +picture_t * picture_InternalClone(picture_t *, const video_format_t *, void (*pf_destroy)(picture_t *), void *);
>> diff --git a/src/misc/picture_pool.c b/src/misc/picture_pool.c
>> index d2df4bf2eb0..7438e5f1d6a 100644
>> --- a/src/misc/picture_pool.c
>> +++ b/src/misc/picture_pool.c
>> @@ -85,13 +85,14 @@ static void picture_pool_ReleaseClone(picture_t *clone)
>> }
>>
>> static picture_t *picture_pool_ClonePicture(picture_pool_t *pool,
>> - unsigned offset)
>> + unsigned offset,
>> + const video_format_t *fmt)
>> {
>> picture_t *picture = pool->picture[offset];
>> uintptr_t sys = ((uintptr_t)pool) + offset;
>>
>> - picture_t *clone = picture_InternalClone(picture, picture_pool_ReleaseClone,
>> - (void*)sys);
>> + picture_t *clone = picture_InternalClone(picture, fmt,
>> + picture_pool_ReleaseClone, (void*)sys);
>> if (clone != NULL) {
>> assert(!picture_HasChainedPics(clone));
>> atomic_fetch_add_explicit(&pool->refs, 1, memory_order_relaxed);
>> @@ -191,14 +192,14 @@ picture_t *picture_pool_Get(picture_pool_t *pool)
>> vlc_mutex_unlock(&pool->lock);
>> available &= ~(1ULL << i);
>>
>> - return picture_pool_ClonePicture(pool, i);
>> + return picture_pool_ClonePicture(pool, i, NULL);
>> }
>>
>> vlc_mutex_unlock(&pool->lock);
>> return NULL;
>> }
>>
>> -picture_t *picture_pool_Wait(picture_pool_t *pool)
>> +picture_t *picture_pool_Wait(picture_pool_t *pool, const video_format_t *fmt)
>> {
>> vlc_mutex_lock(&pool->lock);
>> assert(pool->refs > 0);
>> @@ -217,7 +218,7 @@ picture_t *picture_pool_Wait(picture_pool_t *pool)
>> pool->available &= ~(1ULL << i);
>> vlc_mutex_unlock(&pool->lock);
>>
>> - return picture_pool_ClonePicture(pool, i);
>> + return picture_pool_ClonePicture(pool, i, fmt);
>> }
>>
>> void picture_pool_Cancel(picture_pool_t *pool, bool canceled)
>> diff --git a/src/test/picture_pool.c b/src/test/picture_pool.c
>> index 9b43c25fdd4..e4b5491c241 100644
>> --- a/src/test/picture_pool.c
>> +++ b/src/test/picture_pool.c
>> @@ -75,7 +75,7 @@ static void test(bool zombie)
>> picture_Release(pics[i]);
>>
>> for (unsigned i = 0; i < PICTURES; i++) {
>> - pics[i] = picture_pool_Wait(pool);
>> + pics[i] = picture_pool_Wait(pool, NULL);
>> assert(pics[i] != NULL);
>> }
>>
>> diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
>> index 20e569e173c..de515b4546e 100644
>> --- a/src/video_output/video_output.c
>> +++ b/src/video_output/video_output.c
>> @@ -468,7 +468,7 @@ picture_t *vout_GetPicture(vout_thread_t *vout)
>> {
>> vout_thread_sys_t *sys = VOUT_THREAD_TO_SYS(vout);
>> assert(!sys->dummy);
>> - picture_t *picture = picture_pool_Wait(sys->sys.private.display_pool);
>> + picture_t *picture = picture_pool_Wait(sys->sys.private.display_pool, NULL);
>> if (likely(picture != NULL)) {
>> picture_Reset(picture);
>> video_format_CopyCropAr(&picture->format, &sys->original);
>> --
>> 2.26.2
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>
More information about the vlc-devel
mailing list