1345341 - [EME] "Never expire" MediaKeySession expiration is not exposed

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P3)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

The WidevineDecryptor is suppressing OnExpirationChange(0) calls:

https://dxr.mozilla.org/mozilla-central/rev/b7e42143bbbc9dc3e5c05bd1e93b6485ce1d0ad4/dom/media/gmp/widevine-adapter/WidevineDecryptor.cpp#418

I added this so that cases like the Shaka Player Demo, Axiom "Multi-DRM" work.

However, Chromium actually assumes that a time of 0 means "never expire", represented by NaN:

https://bugs.chromium.org/p/chromium/issues/detail?id=679079#c6
https://cs.chromium.org/chromium/src/media/blink/webcontentdecryptionmodulesession_impl.cc?l=499&rcl=0d9346b1e64f41e97cd44a291ae43343bc0edcc7

So we should do the same, given that we're using the same DRM plugin...

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1345341 - Ensure Widevine expiration time of 0 means 'never expire'.

Review commit: https://reviewboard.mozilla.org/r/118042/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/118042/

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8844742 [details]
Bug 1345341 - Ensure Widevine expiration time of 0 means 'never expire'.

https://reviewboard.mozilla.org/r/118042/#review119878

::: dom/media/gmp/GMPCDMProxy.cpp:613
(Diff revision 1)
>      return;
>    }
>    RefPtr<dom::MediaKeySession> session(mKeys->GetSession(aSessionId));
>    if (session) {
> -    session->SetExpiration(static_cast<double>(aExpiryTime));
> +    // Expiry of 0 is interpreted as "never expire". See bug 1345341.
> +    double t = !aExpiryTime ? std::numeric_limits<double>::quiet_NaN()

I prefer |foo == 0| over |!foo| when it comes to integer comparison.

::: dom/media/gmp/GMPCDMProxy.cpp:613
(Diff revision 1)
>      return;
>    }
>    RefPtr<dom::MediaKeySession> session(mKeys->GetSession(aSessionId));
>    if (session) {
> -    session->SetExpiration(static_cast<double>(aExpiryTime));
> +    // Expiry of 0 is interpreted as "never expire". See bug 1345341.
> +    double t = !aExpiryTime ? std::numeric_limits<double>::quiet_NaN()

I wonder whey we don't use std::numeric_limits<double>::infinity() instead to mean "never expire".

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8844742 [details]
Bug 1345341 - Ensure Widevine expiration time of 0 means 'never expire'.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118042/diff/1-2/

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c7208e83a17
Ensure Widevine expiration time of 0 means 'never expire'. r=jwwang

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9c7208e83a17