<html><head></head><body>I find that much harder to read, and most static analyzers and syntax highlighters too.<br><br><div class="gmail_quote">Le 23 juillet 2019 08:20:40 GMT+03:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">On 2019-07-22 15:16, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">Hi,<br><br>I don't see how a setter can work with static initialisation.<br></blockquote><br>This works, it just preprocessor processing:<br><br>#define DEFINE_VA_OPERATIONS(get, close) \<br>     static const struct vlc_va_operations ops = { (get), (close) };<br><br>DEFINE_VA_OPERATIONS( Get, Close )<br><br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">And adding custom macros for static initialisation only makes the code harder for humans and static analysis to read. <br></blockquote><br>I doubt a static analyzer would choke on preprocessor generated code, or <br>it's not very good.<br><br>As for the readability it's debatable. It's don't think it's better or <br>worse.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">It is not that black and white. Named initialisation has two problems,<br>that classic sequential initialisation does not. 1) it does not work<br>in C++, and as much as I'd like to get rid of C++ code... Not<br>happening.<br></blockquote><br>Since none of these files are C++ it's not an issue. It would only <br>become one if we use a macro. But then if we do we already handle the <br>order/values we set so we wouldn't really need it.<br><br>Anyway, I just tried a simple exemple in C++ and named and unnamed work:<br><br>static void logger(void *data, int type, const vlc_log_t *item,<br>                             const char *fmt, va_list args)<br>{}<br><br>static const struct vlc_logger_operations libvlc_log_ops = {<br>      .destroy = NULL, .log = logger<br>};<br><br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">2) it mutes warnings about missing initialisers when a member is<br>added.<br></blockquote><br>This is also the case with unnamed initialization, if the field is added <br>at the end (otherwise the pointers are incompatible). At least with the <br>macro above, this cannot happen. Which is the whole point of the macro <br>(it seems static assertion is not possible to test for NULL values, too <br>bad).<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">Le 22 juillet 2019 07:42:25 GMT+03:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"> These "operations" structures should have setters. So when they evolve<br> (adding fields) the code has to evolve accordingly.<br><br> On 2019-07-19 18:33, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">vlc | branch: master | Rémi Denis-Courmont <remi@remlab.net> | Fri<br></blockquote>Jul 19 19:32:33 2019 +0300| [bbbdc5ca6cf8f95d6b8f80ebc6d41907fdee45cb]<br>| committer: Rémi Denis-Courmont<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;"> va: use operations structure<br><br></blockquote><a href="http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=bbbdc5ca6cf8f95d6b8f80ebc6d41907fdee45cb">http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=bbbdc5ca6cf8f95d6b8f80ebc6d41907fdee45cb</a><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;"><hr>    modules/codec/avcodec/d3d11va.c |  6 +++---<br>    modules/codec/avcodec/dxva2.c   |  5 +++--<br>    modules/codec/avcodec/va.c      |  4 ++--<br>    modules/codec/avcodec/va.h      | 10 +++++++---<br>    modules/codec/avcodec/vaapi.c   |  5 +++--<br>    modules/hw/vdpau/avcodec.c      |  5 +++--<br>    6 files changed, 21 insertions(+), 14 deletions(-)<br><br> diff --git a/modules/codec/avcodec/d3d11va.c<br></blockquote>b/modules/codec/avcodec/d3d11va.c<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">index 6823b8d2fb..90f5a00d19 100644<br>--- a/modules/codec/avcodec/d3d11va.c<br>+++ b/modules/codec/avcodec/d3d11va.c<br>@@ -308,6 +308,8 @@ static void Close(vlc_va_t *va, void **ctx)<br>       free(sys);<br>   }<br>   <br>+static const struct vlc_va_operations ops = { Get, Close, };<br>+<br>   static int Open(vlc_va_t *va, AVCodecContext *ctx, enum PixelFormat<br></blockquote>pix_fmt,<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">                   const es_format_t *fmt, picture_sys_d3d11_t *p_sys)<br>   {<br>@@ -409,9 +411,7 @@ static int Open(vlc_va_t *va, AVCodecContext<br></blockquote>*ctx, enum PixelFormat pix_fmt,<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">       ctx->hwaccel_context = &sys->hw;<br>   <br>-    va->get = Get;<br>-    va->close = Close;<br>-<br>+    va->ops = &ops;<br>       return VLC_SUCCESS;<br>   <br>   error:<br>diff --git a/modules/codec/avcodec/dxva2.c<br></blockquote>b/modules/codec/avcodec/dxva2.c<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">index ca45421d3c..596538d0f3 100644<br>--- a/modules/codec/avcodec/dxva2.c<br>+++ b/modules/codec/avcodec/dxva2.c<br>@@ -253,6 +253,8 @@ static void Close(vlc_va_t *va, void **ctx)<br>       free(sys);<br>   }<br>   <br>+static const struct vlc_va_operations ops = { Get, Close, };<br>+<br>   static int Open(vlc_va_t *va, AVCodecContext *ctx, enum PixelFormat<br></blockquote>pix_fmt,<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">                   const es_format_t *fmt, picture_sys_d3d9_t *p_sys)<br>   {<br>@@ -337,8 +339,7 @@ static int Open(vlc_va_t *va, AVCodecContext<br></blockquote>*ctx, enum PixelFormat pix_fmt,<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">       ctx->hwaccel_context = &sys->hw;<br>   <br>-    va->get = Get;<br>-    va->close = Close;<br>+    va->ops = &ops;<br>       return VLC_SUCCESS;<br>   <br>   error:<br>diff --git a/modules/codec/avcodec/va.c b/modules/codec/avcodec/va.c<br>index 545699e870..d491fed0e9 100644<br>--- a/modules/codec/avcodec/va.c<br>+++ b/modules/codec/avcodec/va.c<br>@@ -125,7 +125,7 @@ vlc_va_t *vlc_va_New(vlc_object_t *obj,<br></blockquote>AVCodecContext *avctx,<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">   void vlc_va_Delete(vlc_va_t *va, void **hwctx)<br>   {<br>-    if (va->close != NULL)<br>-        va->close(va, hwctx);<br>+    if (va->ops->close != NULL)<br>+        va->ops->close(va, hwctx);<br>       vlc_object_delete(va);<br>   }<br>diff --git a/modules/codec/avcodec/va.h b/modules/codec/avcodec/va.h<br>index 5ef2df1307..aeab69b34b 100644<br>--- a/modules/codec/avcodec/va.h<br>+++ b/modules/codec/avcodec/va.h<br>@@ -28,12 +28,16 @@<br>   typedef struct vlc_va_t vlc_va_t;<br>   typedef struct vlc_va_sys_t vlc_va_sys_t;<br>   <br>+struct vlc_va_operations {<br>+    int (*get)(vlc_va_t *, picture_t *pic, uint8_t **surface);<br>+    void (*close)(vlc_va_t *, void **hwctx);<br>+};<br>+<br>   struct vlc_va_t {<br>       struct vlc_object_t obj;<br>   <br>       vlc_va_sys_t *sys;<br>-    int (*get)(vlc_va_t *, picture_t *pic, uint8_t **surface);<br>-    void (*close)(vlc_va_t *, void **hwctx);<br>+    const struct vlc_va_operations *ops;<br>   };<br>   <br>   /**<br>@@ -75,7 +79,7 @@ vlc_va_t *vlc_va_New(vlc_object_t *obj,<br></blockquote>AVCodecContext *,<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;"> */<br>static inline int vlc_va_Get(vlc_va_t *va, picture_t *pic, uint8_t<br></blockquote>**surface)<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">   {<br>-    return va->get(va, pic, surface);<br>+    return va->ops->get(va, pic, surface);<br>   }<br>   <br>   /**<br>diff --git a/modules/codec/avcodec/vaapi.c<br></blockquote>b/modules/codec/avcodec/vaapi.c<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">index 948916d046..4ed67677d5 100644<br>--- a/modules/codec/avcodec/vaapi.c<br>+++ b/modules/codec/avcodec/vaapi.c<br>@@ -145,6 +145,8 @@ static void Delete(vlc_va_t *va, void **hwctx)<br>       free(sys);<br>   }<br>   <br>+static const struct vlc_va_operations ops = { Get, Delete, };<br>+<br>   static int Create(vlc_va_t *va, AVCodecContext *ctx, enum<br></blockquote>PixelFormat pix_fmt,<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">                     const es_format_t *fmt, void *p_sys)<br>   {<br>@@ -201,8 +203,7 @@ static int Create(vlc_va_t *va, AVCodecContext<br></blockquote>*ctx, enum PixelFormat pix_fmt,<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">       ctx->hwaccel_context = &sys->hw_ctx;<br>       va->sys = sys;<br>-    va->get = Get;<br>-    va->close = Delete;<br>+    va->ops = &ops;<br>       return VLC_SUCCESS;<br>   <br>   error:<br>diff --git a/modules/hw/vdpau/avcodec.c b/modules/hw/vdpau/avcodec.c<br>index 92fe0a65f6..c1d5d79cea 100644<br>--- a/modules/hw/vdpau/avcodec.c<br>+++ b/modules/hw/vdpau/avcodec.c<br>@@ -128,6 +128,8 @@ static void Close(vlc_va_t *va, void **hwctx)<br>       free(sys);<br>   }<br>   <br>+static const struct vlc_va_operations ops = { Lock, Close, };<br>+<br>   static int Open(vlc_va_t *va, AVCodecContext *avctx, enum<br></blockquote>PixelFormat pix_fmt,<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">                   const es_format_t *fmt, void *p_sys)<br>   {<br>@@ -215,8 +217,7 @@ static int Open(vlc_va_t *va, AVCodecContext<br></blockquote>*avctx, enum PixelFormat pix_fmt,<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">if (vdp_get_information_string(sys->vdp, &infos) ==<br></blockquote>VDP_STATUS_OK)<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">            msg_Info(va, "Using %s", infos);<br>    <br> -    va->get = Lock;<br> -    va->close = Close;<br> +    va->ops = &ops;<br>        return VLC_SUCCESS;<br>    <br>    error:<hr> vlc-commits mailing list<br> vlc-commits@videolan.org<br> <a href="https://mailman.videolan.org/listinfo/vlc-commits">https://mailman.videolan.org/listinfo/vlc-commits</a><br><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></blockquote>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.<hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></pre></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>