[vlc-devel] [vlc-commits] va: use operations structure

[Steve Lhomme]

On 2019-07-19 18:33, Rémi Denis-Courmont wrote:
> vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Fri Jul 19 19:32:33 2019 +0300| [bbbdc5ca6cf8f95d6b8f80ebc6d41907fdee45cb] | committer: Rémi Denis-Courmont
> 
> va: use operations structure
> 
>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=bbbdc5ca6cf8f95d6b8f80ebc6d41907fdee45cb
> ---
> 
>   modules/codec/avcodec/d3d11va.c |  6 +++---
>   modules/codec/avcodec/dxva2.c   |  5 +++--
>   modules/codec/avcodec/va.c      |  4 ++--
>   modules/codec/avcodec/va.h      | 10 +++++++---
>   modules/codec/avcodec/vaapi.c   |  5 +++--
>   modules/hw/vdpau/avcodec.c      |  5 +++--
>   6 files changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/modules/codec/avcodec/d3d11va.c b/modules/codec/avcodec/d3d11va.c
> index 6823b8d2fb..90f5a00d19 100644
> --- a/modules/codec/avcodec/d3d11va.c
> +++ b/modules/codec/avcodec/d3d11va.c
> @@ -308,6 +308,8 @@ static void Close(vlc_va_t *va, void **ctx)
>       free(sys);
>   }
>   
> +static const struct vlc_va_operations ops = { Get, Close, };

You should set the name for each callback. If the order of the struct is 
changed, the code is broken (and I'm planning to add a callback in there).

That's why I'd prefer a generic setter that cannot be mistaken. Adding, 
removing, changing callbacks will be safe from oversight (won't 
compile). That will avoid breaking platforms by mistake when gitlab 
checks all the platforms before pushing is allowed.

> +
>   static int Open(vlc_va_t *va, AVCodecContext *ctx, enum PixelFormat pix_fmt,
>                   const es_format_t *fmt, picture_sys_d3d11_t *p_sys)
>   {
> @@ -409,9 +411,7 @@ static int Open(vlc_va_t *va, AVCodecContext *ctx, enum PixelFormat pix_fmt,
>   
>       ctx->hwaccel_context = &sys->hw;
>   
> -    va->get = Get;
> -    va->close = Close;
> -
> +    va->ops = &ops;
>       return VLC_SUCCESS;
>   
>   error:
> diff --git a/modules/codec/avcodec/dxva2.c b/modules/codec/avcodec/dxva2.c
> index ca45421d3c..596538d0f3 100644
> --- a/modules/codec/avcodec/dxva2.c
> +++ b/modules/codec/avcodec/dxva2.c
> @@ -253,6 +253,8 @@ static void Close(vlc_va_t *va, void **ctx)
>       free(sys);
>   }
>   
> +static const struct vlc_va_operations ops = { Get, Close, };
> +
>   static int Open(vlc_va_t *va, AVCodecContext *ctx, enum PixelFormat pix_fmt,
>                   const es_format_t *fmt, picture_sys_d3d9_t *p_sys)
>   {
> @@ -337,8 +339,7 @@ static int Open(vlc_va_t *va, AVCodecContext *ctx, enum PixelFormat pix_fmt,
>   
>       ctx->hwaccel_context = &sys->hw;
>   
> -    va->get = Get;
> -    va->close = Close;
> +    va->ops = &ops;
>       return VLC_SUCCESS;
>   
>   error:
> diff --git a/modules/codec/avcodec/va.c b/modules/codec/avcodec/va.c
> index 545699e870..d491fed0e9 100644
> --- a/modules/codec/avcodec/va.c
> +++ b/modules/codec/avcodec/va.c
> @@ -125,7 +125,7 @@ vlc_va_t *vlc_va_New(vlc_object_t *obj, AVCodecContext *avctx,
>   
>   void vlc_va_Delete(vlc_va_t *va, void **hwctx)
>   {
> -    if (va->close != NULL)
> -        va->close(va, hwctx);
> +    if (va->ops->close != NULL)
> +        va->ops->close(va, hwctx);
>       vlc_object_delete(va);
>   }
> diff --git a/modules/codec/avcodec/va.h b/modules/codec/avcodec/va.h
> index 5ef2df1307..aeab69b34b 100644
> --- a/modules/codec/avcodec/va.h
> +++ b/modules/codec/avcodec/va.h
> @@ -28,12 +28,16 @@
>   typedef struct vlc_va_t vlc_va_t;
>   typedef struct vlc_va_sys_t vlc_va_sys_t;
>   
> +struct vlc_va_operations {
> +    int (*get)(vlc_va_t *, picture_t *pic, uint8_t **surface);
> +    void (*close)(vlc_va_t *, void **hwctx);
> +};
> +
>   struct vlc_va_t {
>       struct vlc_object_t obj;
>   
>       vlc_va_sys_t *sys;
> -    int (*get)(vlc_va_t *, picture_t *pic, uint8_t **surface);
> -    void (*close)(vlc_va_t *, void **hwctx);
> +    const struct vlc_va_operations *ops;
>   };
>   
>   /**
> @@ -75,7 +79,7 @@ vlc_va_t *vlc_va_New(vlc_object_t *obj, AVCodecContext *,
>    */
>   static inline int vlc_va_Get(vlc_va_t *va, picture_t *pic, uint8_t **surface)
>   {
> -    return va->get(va, pic, surface);
> +    return va->ops->get(va, pic, surface);
>   }
>   
>   /**
> diff --git a/modules/codec/avcodec/vaapi.c b/modules/codec/avcodec/vaapi.c
> index 948916d046..4ed67677d5 100644
> --- a/modules/codec/avcodec/vaapi.c
> +++ b/modules/codec/avcodec/vaapi.c
> @@ -145,6 +145,8 @@ static void Delete(vlc_va_t *va, void **hwctx)
>       free(sys);
>   }
>   
> +static const struct vlc_va_operations ops = { Get, Delete, };
> +
>   static int Create(vlc_va_t *va, AVCodecContext *ctx, enum PixelFormat pix_fmt,
>                     const es_format_t *fmt, void *p_sys)
>   {
> @@ -201,8 +203,7 @@ static int Create(vlc_va_t *va, AVCodecContext *ctx, enum PixelFormat pix_fmt,
>   
>       ctx->hwaccel_context = &sys->hw_ctx;
>       va->sys = sys;
> -    va->get = Get;
> -    va->close = Delete;
> +    va->ops = &ops;
>       return VLC_SUCCESS;
>   
>   error:
> diff --git a/modules/hw/vdpau/avcodec.c b/modules/hw/vdpau/avcodec.c
> index 92fe0a65f6..c1d5d79cea 100644
> --- a/modules/hw/vdpau/avcodec.c
> +++ b/modules/hw/vdpau/avcodec.c
> @@ -128,6 +128,8 @@ static void Close(vlc_va_t *va, void **hwctx)
>       free(sys);
>   }
>   
> +static const struct vlc_va_operations ops = { Lock, Close, };
> +
>   static int Open(vlc_va_t *va, AVCodecContext *avctx, enum PixelFormat pix_fmt,
>                   const es_format_t *fmt, void *p_sys)
>   {
> @@ -215,8 +217,7 @@ static int Open(vlc_va_t *va, AVCodecContext *avctx, enum PixelFormat pix_fmt,
>       if (vdp_get_information_string(sys->vdp, &infos) == VDP_STATUS_OK)
>           msg_Info(va, "Using %s", infos);
>   
> -    va->get = Lock;
> -    va->close = Close;
> +    va->ops = &ops;
>       return VLC_SUCCESS;
>   
>   error:
> 
> _______________________________________________
> vlc-commits mailing list
> vlc-commits at videolan.org
> https://mailman.videolan.org/listinfo/vlc-commits
>