[vlc-devel] [vlc-commits] va: use operations structure
Steve Lhomme
robux4 at ycbcr.xyz
Tue Jul 23 07:20:40 CEST 2019
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
>
More information about the vlc-devel
mailing list