<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:mcatanzaro@igalia.com" title="Michael Catanzaro <mcatanzaro@igalia.com>"> <span class="fn">Michael Catanzaro</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK][GStreamer] ClearKey EME v1 decryption support"
href="https://bugs.webkit.org/show_bug.cgi?id=154235">bug 154235</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #271332 Flags</td>
<td>review?
</td>
<td>review-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK][GStreamer] ClearKey EME v1 decryption support"
href="https://bugs.webkit.org/show_bug.cgi?id=154235#c10">Comment # 10</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK][GStreamer] ClearKey EME v1 decryption support"
href="https://bugs.webkit.org/show_bug.cgi?id=154235">bug 154235</a>
from <span class="vcard"><a class="email" href="mailto:mcatanzaro@igalia.com" title="Michael Catanzaro <mcatanzaro@igalia.com>"> <span class="fn">Michael Catanzaro</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=271332&action=diff" name="attach_271332" title="patch">attachment 271332</a> <a href="attachment.cgi?id=271332&action=edit" title="patch">[details]</a></span>
patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=271332&action=review">https://bugs.webkit.org/attachment.cgi?id=271332&action=review</a>
<span class="quote">> Source/WebCore/ChangeLog:24
> + disabled by default.</span >
I see the technical advantage to us in upstreaming this code, even if it will never be used by any distros and always disabled by default in OptionsGTK.cmake. But I kinda think it should be enabled by default in FeatureList.pm, for developers, even if it's not going to be used by any distros. Otherwise the build will be broken all the time, and we won't be able to run layout tests on the bot, and without running layout tests we won't notice when the functionality breaks. I understand we'll only be able to test ClearKey, but that seems fine to me, and much better than adding this with no tests.
So, I am going to r- this, hoping not to set back your work, just until we're ready to enable it for development builds and can either add or unskip at least a couple of tests, and pending our transition to GStreamer 1.6 on the bots. (I would love to see that sooner rather than later, because our users have had GStreamer 1.6 for a long time now.)
<span class="quote">> Source/WebCore/PlatformGTK.cmake:830
> + ${LIBGCRYPT_LIBRARIES} -lgpg-error</span >
What is -lgpg-error? If it's not specified in a pkg-config module and is part of libgcrypt, it should probably be added to LIBGCRYPT_LIBRARIES by FindLibGcrypt.cmake.
<span class="quote">> Source/WebCore/PlatformGTK.cmake:837
> +</span >
Nit: no blank line here, please.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/WebKitCommonEncryptionDecryptorGStreamer.h:8
> + * modify it under the terms of the GNU Library General Public</span >
Nit: grab a newer license header from some other file, should say "Lesser General Public License"
<span class="quote">> Source/cmake/OptionsGTK.cmake:380
> +</span >
Nit: No blank line here, please.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>