<!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,</p>
<p>On 2018-09-28 12:26, Steve Lhomme wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> regression introduced by 2054d46ba29d7986d8403656170ff9445ab18efc
---
src/misc/picture.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/misc/picture.c b/src/misc/picture.c
index f8059a603e3..f61e0a3d0ef 100644
--- a/src/misc/picture.c
+++ b/src/misc/picture.c
@@ -189,6 +189,7 @@ static picture_priv_t *picture_NewPrivate(const video_format_t *restrict p_fmt)
memset( p_picture, 0, sizeof( *p_picture ) );
+ p_picture->format = *p_fmt;</code></pre>
</blockquote>
<p>LGTM, as long as we are sure that we will not (once again) step into life-time issues related to the ownership of <code>video_format_t.p_palette</code> (but as it worked before, well.. it at least wouldn’t get worse than what we had previously).</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> /* Make sure the real dimensions are a multiple of 16 */
if( picture_Setup( p_picture, p_fmt ) )
{</code></pre>
</blockquote>
<p>This regression was noticed as part of a conversation between me and <code>haasn</code> on <code>#videolan</code> (as <code>haasn</code> was confused about what caused his previously working code to all of a sudden behave weird).</p>
<p>I started digging in the commit-log, and found that <code>picture_t.format</code> a long time in the past had a different type than <code>video_format_t</code>, but later a <code>typedef</code> was introduced to make <code>video_frame_format_t</code> an alias for <code>video_format_t</code>.</p>
<ul>
<li><a href="http://git.videolan.org/?p=vlc.git;a=commit;h=1a67448183a9c5ab7b5b427fb80d2d1a2e34ff8d">1a67448183a: … include/vlc_video.h: extended video_frame_format_t</a></li>
</ul>
<p><code>haasn</code> is working on <code>video_output/vulcan</code>. As the information he is after is available in <code>vout_display_t.fmt</code> he simply modified his patch to use that instead of the format associated with a single <code>picture_t</code>.</p>
<p>So, my question is (ignoring the obvious breakage of current modules relying on <code>picture_t.format</code> having all its fields populated and not only those from <code>src/misc/picture.c:picture_Setup</code>): should we go back to the previous behavior where a smaller distinct type, <code>video_frame_format_t</code>, is used for each <code>picture_t</code>?</p>
<p>We of course have the issue where a <em>format-change</em> mid-stream will result in weird behavior due to the async nature of changing the associated format, and the <code>picture_t</code>-processing not matching the update (applying the format to too many or too few frames). After a very quick conversation with <code>tguillem</code> this is something we can and will address at the <em>vout-workshop</em>, though I would like to plant this seed (regarding change of type and things that might need changing) here on <code>vlc-devel</code> to get as many opinions as possible prior to said workshop.</p>
<p>Any opinions about this that might be worth sharing prior to any extensive <em>workshop</em> on the matter?</p>
<p>Best Regards,<br />
Filip</p>
</body>
</html>