[vlc-devel] [vlc-commits] va: use operations structure
Rémi Denis-Courmont
remi at remlab.net
Tue Jul 23 08:06:09 CEST 2019
I find that much harder to read, and most static analyzers and syntax highlighters too.
Le 23 juillet 2019 08:20:40 GMT+03:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>On 2019-07-22 15:16, Rémi Denis-Courmont wrote:
>> Hi,
>>
>> I don't see how a setter can work with static initialisation.
>
>This works, it just preprocessor processing:
>
>#define DEFINE_VA_OPERATIONS(get, close) \
> static const struct vlc_va_operations ops = { (get), (close) };
>
>DEFINE_VA_OPERATIONS( Get, Close )
>
>
>> And adding custom macros for static initialisation only makes the
>code harder for humans and static analysis to read.
>
>I doubt a static analyzer would choke on preprocessor generated code,
>or
>it's not very good.
>
>As for the readability it's debatable. It's don't think it's better or
>worse.
>
>> It is not that black and white. Named initialisation has two
>problems,
> > that classic sequential initialisation does not. 1) it does not work
> > in C++, and as much as I'd like to get rid of C++ code... Not
> > happening.
>
>Since none of these files are C++ it's not an issue. It would only
>become one if we use a macro. But then if we do we already handle the
>order/values we set so we wouldn't really need it.
>
>Anyway, I just tried a simple exemple in C++ and named and unnamed
>work:
>
>static void logger(void *data, int type, const vlc_log_t *item,
> const char *fmt, va_list args)
>{}
>
>static const struct vlc_logger_operations libvlc_log_ops = {
> .destroy = NULL, .log = logger
>};
>
>
> > 2) it mutes warnings about missing initialisers when a member is
> > added.
>
>This is also the case with unnamed initialization, if the field is
>added
>at the end (otherwise the pointers are incompatible). At least with the
>
>macro above, this cannot happen. Which is the whole point of the macro
>(it seems static assertion is not possible to test for NULL values, too
>
>bad).
>
>> Le 22 juillet 2019 07:42:25 GMT+03:00, Steve Lhomme
><robux4 at ycbcr.xyz> a écrit :
>>> These "operations" structures should have setters. So when they
>evolve
>>> (adding fields) the code has to evolve accordingly.
>>>
>>> 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, };
>>>> +
>>>> 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
>>>>
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>>
>> --
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>excuser ma brièveté.
>>
>>
>> _______________________________________________
>> 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
--
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190723/44a971b9/attachment.html>
More information about the vlc-devel
mailing list