[Webkit-unassigned] [Bug 154235] [GTK][GStreamer] ClearKey EME v1 decryption support

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=154235

Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #271332|review?                     |review-
              Flags|                            |

--- Comment #10 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 271332
  --> https://bugs.webkit.org/attachment.cgi?id=271332
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=271332&action=review

> Source/WebCore/ChangeLog:24
> +        disabled by default.

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.)

> Source/WebCore/PlatformGTK.cmake:830
> +        ${LIBGCRYPT_LIBRARIES} -lgpg-error

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.

> Source/WebCore/PlatformGTK.cmake:837
> +

Nit: no blank line here, please.

> Source/WebCore/platform/graphics/gstreamer/WebKitCommonEncryptionDecryptorGStreamer.h:8
> + * modify it under the terms of the GNU Library General Public

Nit: grab a newer license header from some other file, should say "Lesser General Public License"

> Source/cmake/OptionsGTK.cmake:380
> +

Nit: No blank line here, please.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160215/79b52350/attachment-0001.html>