<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
  <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
  <meta http-equiv="Content-Style-Type" content="text/css" />
  <meta name="generator" content="pandoc" />
  <title></title>
  <style type="text/css">
      code{white-space: pre-wrap;}
      span.smallcaps{font-variant: small-caps;}
      span.underline{text-decoration: underline;}
      div.column{display: inline-block; vertical-align: top; width: 50%;}
  </style>
</head>
<body>
<p>Hi Steve,</p>
<p>On 2018-08-14 15:26, Steve Lhomme wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> vlc | branch: master | Steve Lhomme <robux4@ycbcr.xyz> | Tue Aug 14 12:49:51 2018 +0200| [25756f4b78a010599b541b6df80e534019f0d5ab] | committer: Steve Lhomme

 es_out: add multiview mode info in the ES description</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<h3 id="httpgit.videolan.orggitweb.cgivlc.gitacommith25756f4b78a010599b541b6df80e534019f0d5ab">http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=25756f4b78a010599b541b6df80e534019f0d5ab</h3>
</blockquote>
<pre><code>  src/input/es_out.c | 19 +++++++++++++++++++
  1 file changed, 19 insertions(+)

 diff --git a/src/input/es_out.c b/src/input/es_out.c
 index 37d4c10ab6..f1f13cad66 100644
 --- a/src/input/es_out.c
 +++ b/src/input/es_out.c
 @@ -3370,6 +3370,25 @@ static void EsOutUpdateInfo( es_out_t *out, es_out_id_t *es, const es_format_t *
             info_category_AddInfo( p_cat, _("Chroma location"), "%s",
                 vlc_gettext(c_loc_names[fmt->video.chroma_location]) );
         }
 +       if( fmt->video.multiview_mode != MULTIVIEW_2D )
 +       {
 +           static const char c_multiview_names[][16] = {</code></pre>
</blockquote>
<p><code>sizeof( "Column Sequential" )</code> is <code>18</code>. I’d be extremely surprised if your compiler did not warn you about this, so please pay closer attention to diagnostics issued for paths you’ve touched.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> +               [MULTIVIEW_2D] = N_("2D"),
 +               [MULTIVIEW_STEREO_SBS] = N_("Side-By-Side"),
 +               [MULTIVIEW_STEREO_TB] = N_("Top-Bottom"),
 +               [MULTIVIEW_STEREO_ROW] = N_("Row Sequential"),
 +               [MULTIVIEW_STEREO_COL] = N_("Column Sequential"),
 +               [MULTIVIEW_STEREO_FRAME] =N_("Frame Sequential"),
 +               [MULTIVIEW_STEREO_CHECKERBOARD] = N_("Checkboard"),
 +           };
 +           static_assert(ARRAY_SIZE(c_multiview_names) == MULTIVIEW_STEREO_CHECKERBOARD+1,
 +                         "Multiview format table mismatch");</code></pre>
</blockquote>
<p>I would strongly advice against such a <code>static_assert</code> as it does not really protect the read that follows.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> +           info_category_AddInfo( p_cat, _("Stereo Mode"), "%s",
 +               vlc_gettext(c_multiview_names[fmt->video.multiview_mode]) );</code></pre>
</blockquote>
<p>Imagine if someone, for whatever reason, adds an entry to <code>enum video_multiview_mode_t</code> after <code>MULTIVIEW_STEREO_CHECKERBOARD</code>, the <code>static_assert</code> will not fire, but your implementation would still result in an <em>out-of-bound</em> read.</p>
<p>There are numerous ways to solve this issue, one is using the same mechanism which is used in other places in the codebase; introduce something like <code>MULTIVIEW_STEREO_MAX</code> (or suitable name). See <code>CHROMA_LOCATION_MAX</code>, as an example.</p>
<p>However unlikely that such mismatching would take place in practice, I am not a fan of the current implementation.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> +           info_category_AddInfo( p_cat, _("First Stereo Eye"),
 +                                  vlc_gettext(fmt->video.b_multiview_right_eye_first ?
 +                                      N_("Right") : N_("Left")) );
 +       }
         if( fmt->video.projection_mode != PROJECTION_MODE_RECTANGULAR )
         {
             const char *psz_loc_name = NULL;</code></pre>
</blockquote>
</body>
</html>