<!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;}</style>
</head>
<body>
<p>Hi Steve,</p>
<p>On 2017-03-07 08:31, Steve Lhomme wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> On Mon, Mar 6, 2017 at 4:24 PM, Filip Roséen <filip@atch.se> wrote:</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> On 2017-03-06 16:09, Steve Lhomme wrote:
+ /* Linear to ST2084 */
+ psz_src_transform =
+ "const float m1 = 2610.0 / (4096.0 * 4);\
+ const float m2 = (2523.0 / 4096.0) * 128.0;\
+ const float c1 = 3424.0 / 4096.0;\
+ const float c2 = (2413.0 / 4096.0) * 32.0;\
+ const float c3 = (2392.0 / 4096.0) * 32.0;\
+ \
Given that the above constants are also declared in patch 1/4, would it not
make sense to introduce a common helper for both cases?</code></pre>
</blockquote>
<pre><code> Yeah, it will make the things a little less readable though.</code></pre>
</blockquote>
<p>Naming the constants <code>ST2084_$x</code> where <code>$x</code> is each entity in <code>{ m1, m2, c1, c2, c3 }</code> would, at least to me, make the code more readable as it would make the magic constants disappear from the calculations, while also making it easier to maintain in the future.</p>
<p>Or even better, introduce two arrays <code>float const ST2084_m[2]</code> and <code>float const ST2084_c[3]</code>, either on their own or wrapped in a suitably named struct (with anonymous type).</p>
<p>But currently I guess it does not matter much, I just thought I’d share my immediate reaction when reading the patch.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> + rgb = pow(rgb, m1);\
+ rgb = (c1 + c2 * rgb) / (1 + c3 * rgb);\
+ rgb = pow(rgb, m3);\
NITPICK: Is it by intention that pow, together with double-literals are
used, while the variables in question are float? If floats are really
desired, powf and .0f seems more suitable to declare such intent.</code></pre>
</blockquote>
<pre><code> This is not C or C++ code, it's HLSL which doesn't have double nor powf().
https://msdn.microsoft.com/en-us/library/windows/desktop/bb509636(v=vs.85).aspx</code></pre>
</blockquote>
<p>Oh yeah right, I kind of assumed that the symmetry would have both functions; but now that you said it I realize my mistake.</p>
<p>Thank you, and I will read the linked specification!</p>
<p>Best Regards,<br />
Filip</p>
</body>
</html>