<html><head></head><body>Adding a "dummy" maximum inside an enum causes warnings in switch statements. I don't recommend it...<br><br><div class="gmail_quote">Le 15 août 2018 10:48:00 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">Hi Filip,<br><br><br>On 15/08/2018 01:58, Filip Roséen wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;"><br> Hi Steve,<br><br> On 2018-08-14 15:26, Steve Lhomme wrote:<br><br>     |vlc | branch: master | Steve Lhomme <robux4@ycbcr.xyz> | Tue Aug<br>     14 12:49:51 2018 +0200| [25756f4b78a010599b541b6df80e534019f0d5ab]<br>     | committer: Steve Lhomme es_out: add multiview mode info in the<br>     ES description|<br><br><br>               <a href="http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=25756f4b78a010599b541b6df80e534019f0d5ab">http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=25756f4b78a010599b541b6df80e534019f0d5ab</a><br><br>     |src/input/es_out.c | 19 +++++++++++++++++++ 1 file changed, 19<br>     insertions(+) diff --git a/src/input/es_out.c b/src/input/es_out.c<br>     index 37d4c10ab6..f1f13cad66 100644 --- a/src/input/es_out.c +++<br>     b/src/input/es_out.c @@ -3370,6 +3370,25 @@ static void<br>     EsOutUpdateInfo( es_out_t *out, es_out_id_t *es, const es_format_t<br>     * info_category_AddInfo( p_cat, _("Chroma location"), "%s",<br>     vlc_gettext(c_loc_names[fmt->video.chroma_location]) ); } + if(<br>     fmt->video.multiview_mode != MULTIVIEW_2D ) + { + static const<br>     char c_multiview_names[][16] = {|<br><br> |sizeof( "Column Sequential" )| is |18|. I’d be extremely surprised if <br> your compiler did not warn you about this, so please pay closer <br> attention to diagnostics issued for paths you’ve touched.</blockquote><br><br>Yes, I just noticed now. I recently switched to a newer Qt Creator which <br>uses Clang to produce a lot more useful warnings but I'm a bit <br>overwhelmed with all the stuff there is to clean. I made some changes to <br>see the most obvious one.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">     |+ [MULTIVIEW_2D] = N_("2D"), + [MULTIVIEW_STEREO_SBS] =<br>     N_("Side-By-Side"), + [MULTIVIEW_STEREO_TB] = N_("Top-Bottom"), +<br>     [MULTIVIEW_STEREO_ROW] = N_("Row Sequential"), +<br>     [MULTIVIEW_STEREO_COL] = N_("Column Sequential"), +<br>     [MULTIVIEW_STEREO_FRAME] =N_("Frame Sequential"), +<br>     [MULTIVIEW_STEREO_CHECKERBOARD] = N_("Checkboard"), + }; +<br>     static_assert(ARRAY_SIZE(c_multiview_names) ==<br>     MULTIVIEW_STEREO_CHECKERBOARD+1, + "Multiview format table<br>     mismatch");|<br><br> I would strongly advice against such a |static_assert| as it does not <br> really protect the read that follows.</blockquote><br><br>Indeed.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">     |+ info_category_AddInfo( p_cat, _("Stereo Mode"), "%s", +<br>     vlc_gettext(c_multiview_names[fmt->video.multiview_mode]) );|<br><br> Imagine if someone, for whatever reason, adds an entry to |enum <br> video_multiview_mode_t| after |MULTIVIEW_STEREO_CHECKERBOARD|, the <br> |static_assert| will not fire, but your implementation would still <br> result in an /out-of-bound/ read.<br><br> There are numerous ways to solve this issue, one is using the same <br> mechanism which is used in other places in the codebase; introduce <br> something like |MULTIVIEW_STEREO_MAX| (or suitable name). See <br> |CHROMA_LOCATION_MAX|, as an example.</blockquote><br><br>Yes, the max approach is safer.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;"> However unlikely that such mismatching would take place in practice, I <br> am not a fan of the current implementation.</blockquote><br><br>If you mean that way of static as done 4 times above this code ? If you <br>have something cleaner feel free to submit a patch.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">     |+ info_category_AddInfo( p_cat, _("First Stereo Eye"), +<br>     vlc_gettext(fmt->video.b_multiview_right_eye_first ? + N_("Right")<br>     : N_("Left")) ); + } if( fmt->video.projection_mode !=<br>     PROJECTION_MODE_RECTANGULAR ) { const char *psz_loc_name = NULL;|<br><br><br><br><hr><br> 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><hr><br>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>