Bug 201339 – [EME] Introduce a Proxy CDM for thread-safe access to CDM instances from background decryption threads

• Component: Media
• Status: RESOLVED FIXED

[Charlie Turner]

[EME] Introduce a Proxy CDM for thread-safe access to CDM instances from background decryption threads

[Charlie Turner]

Created attachment 377700 [details]
Patch

This won't build on Mac ports because I haven't modifieded the CDM instance there. Review on the approach is appreciated, the ClearKey proxy CDM in the future should contain all the GCrypt code in my opinion, right now it wastes a lot of copies shoving all the keys into the background decryptor threads

[Charlie Turner]

CC Jer, this is a spin-off from https://bugs.webkit.org/show_bug.cgi?id=192229#c25

Again it might seem quite pointless, but such a ProxyCDM will be used more heavily with Widevine / Playready.

It also seems like a good idea to move the GCrypt code from our GStreamer-specific decryptor into the cross-platform ClearKey CDM.

The patch is not for landing as it, the comments need tidying, and the approached needs to be blessed.

[Xabier Rodríguez Calvar]

Comment on attachment 377700 [details]
Patch

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

LGTM as a WIP patch.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:1323
> +    GST_DEBUG_OBJECT(m_pipeline.get(), "CDM proxy instance %p dispatched as context", m_cdmInstance->proxyCDM().get());

I'd say ProxyCDM instance...

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:298
> +                GST_DEBUG_OBJECT(self, "received a new CDM proxy instance %p, refcount %u", priv->proxyCDM.get(), priv->proxyCDM->refCount());

ProxyCDM

[Charlie Turner]

Created attachment 377988 [details]
Patch

Attempt #1 to get the build green for Mac ports.

[Charlie Turner]

I don't understand the iOS build failures,

In file included from /Volumes/Data/worker/iOS-12-Build-EWS/build/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.h:30:
/Volumes/Data/worker/iOS-12-Build-EWS/build/Source/WebCore/platform/encryptedmedia/CDMInstance.h:80:30: note: unimplemented pure virtual method 'proxyCDM' in 'CDMInstanceFairPlayStreamingAVFObjC'
    virtual RefPtr<ProxyCDM> proxyCDM() const = 0;

That method is implemented in CDMInstanceFairPlayStreamingAVFObjC,

+    RefPtr<ProxyCDM> proxyCDM() final { return adoptRef(*new ProxyCDMFairPlayStreamingAVFObjC()); }

And builds on Mac ports.

[Charlie Turner]

Created attachment 378056 [details]
Patch

Sneaky method signature diff, Mac port attempt #2

[Xabier Rodríguez Calvar]

Comment on attachment 378056 [details]
Patch

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

LGTM, Jer, Eric, any objections?

> Source/WebCore/platform/encryptedmedia/CDMInstance.h:47
> +// from background threads (I.e. decryptors)

.

[Jer Noble]

Comment on attachment 378056 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.h:52
> +class ProxyCDMFairPlayStreamingAVFObjC final : public ProxyCDM {
> +    virtual ~ProxyCDMFairPlayStreamingAVFObjC() = default;
> +};
> +

This...

> Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.h:70
> +    RefPtr<ProxyCDM> proxyCDM() const final { return adoptRef(*new ProxyCDMFairPlayStreamingAVFObjC()); }

...and this seem unnecessary. Can this just return null?

[Charlie Turner]

Created attachment 378096 [details]
Patch

Do not provide scaffolding for ProxyCDM on Mac ports

[Jer Noble]

(In reply to Charlie Turner from comment #9)
> Created attachment 378096 [details]
> Patch
> 
> Do not provide scaffolding for ProxyCDM on Mac ports

LGTM

[Charlie Turner]

Created attachment 378124 [details]
Patch for landing

[Charlie Turner]

Created attachment 378125 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 378125 [details]
Patch for landing

Clearing flags on attachment: 378125

Committed r249548: <https://trac.webkit.org/changeset/249548>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/55086095>