[vlc-devel] [PATCH 1/2] picture: always copy context when cloning

[Steve Lhomme]

One main difference between p_sys and the context is that the p_sys was 
"seen" outside of the picture pool when the context was not (blank).

It should work because formats using a picture context were usually only 
setting it on pictures coming out of the picture pool. But it's a 
limitation that was not fun to deal with.

Now that many (all?) output that set a context on a picture are aware 
the picture might be destroyed when their module is already destroyed, 
that should work well with the picture pool still having references to a 
context while the module that produced it is almost dead.

On 2021-04-20 16:17, Romain Vimont wrote:
> From: Alexandre Janniaux <ajanni at videolabs.io>
> 
> picture_Clone was copying the original picture context into the new
> clone while picture_pool_Clone wasn't. As it is possible to create a
> pool from existing pictures, it's quite handy to be able to associate
> the graphical resources needed for each picture to their context so
> that it can be retrieved after a call to picture_pool_Wait/Get.
> 
> The rationale is that because resources like pixels are shared between
> the original picture and the clone, we should probably do the same with
> picture_context and have a unified way for picture producer to
> reference such resources.
> 
> This patch removes the special handling in picture_Clone and move it to
> picture_InternalClone for that reason.
> ---
>   src/misc/picture.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/misc/picture.c b/src/misc/picture.c
> index 5b867a7591..e331d6d92e 100644
> --- a/src/misc/picture.c
> +++ b/src/misc/picture.c
> @@ -438,18 +438,21 @@ picture_t *picture_InternalClone(picture_t *picture,
>       picture_t *clone = picture_NewFromResource(&picture->format, &res);
>       if (likely(clone != NULL)) {
>           ((picture_priv_t *)clone)->gc.opaque = opaque;
> +
> +        /* The picture context is responsible for potentially holding the
> +         * video context attached to the picture if needed. */
> +        if (picture->context != NULL)
> +            clone->context = picture->context->copy(picture->context);
> +
>           picture_Hold(picture);
>       }
> +
>       return clone;
>   }
>   
>   picture_t *picture_Clone(picture_t *picture)
>   {
>       picture_t *clone = picture_InternalClone(picture, picture_DestroyClone, picture);
> -    if (likely(clone != NULL)) {
> -        if (picture->context != NULL)
> -            clone->context = picture->context->copy(picture->context);
> -    }
>       return clone;
>   }
>   
> -- 
> 2.31.0
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>