[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

--- Comment #8 from Philippe Normand <pnormand at igalia.com> ---
(In reply to comment #7)
> 
> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:896
> > +    if (!equalIgnoringASCIICase(keySystem, "org.w3.clearkey"))
> > +        return MediaPlayer::KeySystemNotSupported;
> 
> Instead of duplication this check here, we could call supportsKeySystem
> instead, or move this check to a helper function and use it from both places.
> 

I'm not sure it's worth.

> 
> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:931
> > +    if (parameters.keySystem.isNull() || parameters.keySystem.isEmpty())
> 
> parameters.keySystem.isEmpty() returns true if it's null
> 

So you mean testing only isNull() is enough?

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:57
> > +void registerWebKitGStreamerElements();
> > +
> 
> Why does this need to be here? Can't this be a static function?
> 

It will need to be called from the future MediaPlayerPrivateGStreamerMSE.

> 
> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:128
> > +    virtual void dispatchDecryptionKey(GstBuffer*);
> 
> Why is this virtual and public?
> 

The MediaPlayerPrivateGStreamerMSE player will need to override it. But maybe it can be private though, I'll check.

I'll process the remain comments later on :) Thanks!

-- 
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/a878f349/attachment.html>