[vlc-devel] [PATCH] picture: refactor to use common code in cloning

[Steve Lhomme]

Le 18/03/2018 à 13:41, Matthew Whitworth a écrit :
> refactor to remove dup clone code per TODO comment
> ---
>   src/misc/picture.c      | 35 ++++++++++++++++++++++++-----------
>   src/misc/picture.h      |  2 ++
>   src/misc/picture_pool.c | 19 ++-----------------
>   3 files changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/src/misc/picture.c b/src/misc/picture.c
> index 7506e47725..5c8587af79 100644
> --- a/src/misc/picture.c
> +++ b/src/misc/picture.c
> @@ -403,28 +403,41 @@ static void picture_DestroyClone(picture_t *clone)
>       picture_Release(picture);
>   }
>   
> -picture_t *picture_Clone(picture_t *picture)
> +picture_t *picture_DuplicateWithOverrides(picture_t *picture, picture_sys_t *sys, void (*pf_destroy)(picture_t *), void* opaque)

Try to avoid very long lines, especially in the core.

>   {
> -    /* TODO: merge common code with picture_pool_ClonePicture(). */
>       picture_resource_t res = {
> -        .p_sys = picture->p_sys,
> -        .pf_destroy = picture_DestroyClone,
> +        .p_sys = sys,
> +        .pf_destroy = pf_destroy
>       };
>   
> +    picture_t* clone;
> +
>       for (int i = 0; i < picture->i_planes; i++) {
>           res.p[i].p_pixels = picture->p[i].p_pixels;
>           res.p[i].i_lines = picture->p[i].i_lines;
>           res.p[i].i_pitch = picture->p[i].i_pitch;
>       }
>   
> -    picture_t *clone = picture_NewFromResource(&picture->format, &res);
> -    if (likely(clone != NULL)) {
> -        ((picture_priv_t *)clone)->gc.opaque = picture;
> -        picture_Hold(picture);
> +    clone = picture_NewFromResource(&picture->format, &res);
> +
> +    if (unlikely(clone == NULL))
> +        return NULL;
> +
> +    ((picture_priv_t *)clone)->gc.opaque = opaque;
> +    picture_Hold(picture);
> +    return clone;
> +}
> +
> +picture_t *picture_Clone(picture_t *picture)
> +{
> +    picture_t *clone = picture_DuplicateWithOverrides(picture, picture->p_sys, picture_DestroyClone, picture);

Same comment

> +
> +    if (unlikely(clone == NULL))
> +        return NULL;
> +
> +    if (picture->context != NULL)
> +        clone->context = picture->context->copy(picture->context);
>   
> -        if (picture->context != NULL)
> -            clone->context = picture->context->copy(picture->context);
> -    }
>       return clone;
>   }
>   
> diff --git a/src/misc/picture.h b/src/misc/picture.h
> index 70ee64878d..e6127af3c7 100644
> --- a/src/misc/picture.h
> +++ b/src/misc/picture.h
> @@ -32,3 +32,5 @@ typedef struct
>           void *opaque;
>       } gc;
>   } picture_priv_t;
> +
> +picture_t *picture_DuplicateWithOverrides(picture_t *picture, picture_sys_t *sys, void (*pf_destroy)(picture_t *), void* opaque);
> diff --git a/src/misc/picture_pool.c b/src/misc/picture_pool.c
> index 74712c81f5..b4c179f943 100644
> --- a/src/misc/picture_pool.c
> +++ b/src/misc/picture_pool.c
> @@ -95,24 +95,9 @@ static picture_t *picture_pool_ClonePicture(picture_pool_t *pool,
>                                               unsigned offset)
>   {
>       picture_t *picture = pool->picture[offset];
> -    uintptr_t sys = ((uintptr_t)pool) + offset;
> -    picture_resource_t res = {
> -        .p_sys = picture->p_sys,
> -        .pf_destroy = picture_pool_ReleasePicture,
> -    };
> -
> -    for (int i = 0; i < picture->i_planes; i++) {
> -        res.p[i].p_pixels = picture->p[i].p_pixels;
> -        res.p[i].i_lines = picture->p[i].i_lines;
> -        res.p[i].i_pitch = picture->p[i].i_pitch;
> -    }
> +    void* opaque = (void *)(((uintptr_t)pool) + offset);
>   
> -    picture_t *clone = picture_NewFromResource(&picture->format, &res);
> -    if (likely(clone != NULL)) {
> -        ((picture_priv_t *)clone)->gc.opaque = (void *)sys;
> -        picture_Hold(picture);
> -    }
> -    return clone;
> +    return picture_DuplicateWithOverrides(picture, picture->p_sys, picture_pool_ReleasePicture, opaque);

Same comment.

>   }
>   
>   picture_pool_t *picture_pool_NewExtended(const picture_pool_configuration_t *cfg)