Closed Bug 1306196 Opened 4 years ago Closed 4 years ago

[EME][Fennec] Implement Gecko Java components to bridge Android MediaDrm/MediaCrypto APIs in OOP-Decode mode.

Categories

(Firefox for Android :: Audio/Video, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Firefox 52
Tracking Status
firefox52 --- fixed

People

(Reporter: kikuo, Assigned: kikuo)

References

Details

Attachments

(3 files)

Divide the solution into following steps.

a. Provide aidl for EME APIs [1]
b. Provide remote process stub (RemoteMediaDrmBridgeStub.java) which is responsible to create the bridge to MediaDrm/MediaCrypto. [2]
c. Create a bridge(RemoteMediaDrmBridge.java) in parent side to communicate with remote stub. [3]
d. Provide a uuid (drmStubId) for each created MediaDrmBridge instance. By this uuid, MediaCodec can find corresponding MediaCrypto object to decrypt data. [4][5]

[1] https://github.com/JamesWCCheng/gecko-dev/commit/a874baaf70b5f5cf7db1a66aae435c2c2bc056c5
[2] https://github.com/JamesWCCheng/gecko-dev/commit/fd192b0f1cfa57b7a3b6c6d2e494a14c9e42d3e3
[3] https://github.com/JamesWCCheng/gecko-dev/commit/2841e6f86de63b0dceb05620cc33ccba14dde87e
[4] https://github.com/JamesWCCheng/gecko-dev/commit/dffc11d6816c82f32d52a5fa1645d23539c5bea4
[5] https://github.com/JamesWCCheng/gecko-dev/commit/197928c9091746f1f73d9ea9a07660d6a03957cc
Assignee: nobody → kikuo
Depends on: 1306185
Summary: [EME][Fennec] Support Widevine video playback in OOP decode mode → [EME][Fennec] Implement Gecko Java components to bridge Android MediaDrm/MediaCrypto APIs in OOP-Decode mode.
Priority: -- → P2
Blocks: 1307818
Have a overview on ...
1. The prerequisite part. 
    ==> Local bridge to bridge Implementation.
2. Which part is going to be provided IN THIS BUG.
    ==> Remote bridge & AIDL binders 
3. The next part to be provided.
    ==> MediaCodec get correct MediaCrypto for decryption/decode.
Comment on attachment 8805460 [details]
Bug 1306196-[P1] Provide AIDL definiion for EME functionality on Fennec MediaManager Service.

https://reviewboard.mozilla.org/r/89196/#review88380

As the interface defined in Bug 1306185 Part1 [1] has been r+'d, I'd like to continue the review procedure for remote drm bridge.

P1 is the IPC definition of our Fennec MediaDrmBridge implementation for out-of-process decoding service.
P2 is the proxy-like classes in parent/child side to deliver requests/results.

[1] https://reviewboard.mozilla.org/r/83250/diff/4#index_header
Attachment #8805460 - Flags: review?(nchen)
Attachment #8805460 - Flags: review?(cpearce)
Attachment #8805461 - Flags: review?(nchen)
Attachment #8805461 - Flags: review?(cpearce)
Comment on attachment 8805461 [details]
Bug 1306196-[P2] Provide RemoteMediaDrmBridge & corresponding stub to underlying GeckoMediaDrmBridge implementation.

https://reviewboard.mozilla.org/r/89198/#review88780

::: mobile/android/base/java/org/mozilla/gecko/media/RemoteMediaDrmBridge.java:31
(Diff revision 1)
> +                                     int promiseId,
> +                                     byte[] sessionId,
> +                                     byte[] request) {
> +            assertTrue(mProxyCallbacks != null);
> +            mProxyCallbacks.onSessionCreated(createSessionToken,
> +                                        promiseId,

Indentation of here.

::: mobile/android/base/java/org/mozilla/gecko/media/RemoteMediaDrmBridgeStub.java:23
(Diff revision 1)
> +    private volatile IMediaDrmBridgeCallbacks mCallbacks = null;
> +
> +    // Underlying bridge implmenetaion, i.e. GeckoMediaDrmBrdigeV21.
> +    private GeckoMediaDrm mBridge = null;
> +
> +    // mStubId is initialized during stub construction. It should be an unique

s/an unique/a unique/
Attachment #8805461 - Flags: review?(cpearce) → review+
Comment on attachment 8805460 [details]
Bug 1306196-[P1] Provide AIDL definiion for EME functionality on Fennec MediaManager Service.

https://reviewboard.mozilla.org/r/89200/#review88782
Attachment #8805460 - Flags: review?(cpearce) → review+
Comment on attachment 8805460 [details]
Bug 1306196-[P1] Provide AIDL definiion for EME functionality on Fennec MediaManager Service.

https://reviewboard.mozilla.org/r/89200/#review88910

::: mobile/android/base/java/org/mozilla/gecko/media/SessionKeyInfo.java:11
(Diff revision 1)
>  
> +import android.os.Parcel;
> +import android.os.Parcelable;
>  import org.mozilla.gecko.annotation.WrapForJNI;
>  
>  @WrapForJNI

Move `@WrapForJNI` to the members that need JNI access, so we don't generate JNI bindings for other members like `describeContents()` and `writeToParcel()`.
Attachment #8805460 - Flags: review?(nchen) → review+
Comment on attachment 8805461 [details]
Bug 1306196-[P2] Provide RemoteMediaDrmBridge & corresponding stub to underlying GeckoMediaDrmBridge implementation.

https://reviewboard.mozilla.org/r/89198/#review88912

::: mobile/android/base/java/org/mozilla/gecko/media/RemoteManager.java:159
(Diff revision 1)
> +        try {
> +            IMediaDrmBridge remoteBridge =
> +                mRemote.createRemoteMediaDrmBridge(keySystem, stubId);
> +            return remoteBridge;
> +        } catch (RemoteException e) {
> +            e.printStackTrace();

Use `Log.e(LOGTAG, "MESSAGE", e)`;

::: mobile/android/base/java/org/mozilla/gecko/media/RemoteMediaDrmBridge.java:29
(Diff revision 1)
> +        @Override
> +        public void onSessionCreated(int createSessionToken,
> +                                     int promiseId,
> +                                     byte[] sessionId,
> +                                     byte[] request) {
> +            assertTrue(mProxyCallbacks != null);

You don't need these checks because we'll throw `NullPointerException` in the lines below if `mProxyCallbacks` is null.

::: mobile/android/base/java/org/mozilla/gecko/media/RemoteMediaDrmBridge.java:76
(Diff revision 1)
> +            assertTrue(mProxyCallbacks != null);
> +            mProxyCallbacks.onRejectPromise(promiseId, message);
> +        }
> +    } // CallbacksForwarder
> +
> +    private static void assertTrue(boolean condition) {

Change to package-private access because you're calling it from CallbacksForwarder.

::: mobile/android/base/java/org/mozilla/gecko/media/RemoteMediaDrmBridge.java:111
(Diff revision 1)
> +        if (DEBUG) Log.d(LOGTAG, "createSession()");
> +
> +        try {
> +            mRemote.createSession(createSessionToken, promiseId, initDataType, initData);
> +        } catch (Exception e) {
> +            Log.e(LOGTAG, "Got exception while creating remote session.");

Add `, e);` at the end, and elsewhere

::: mobile/android/base/java/org/mozilla/gecko/media/RemoteMediaDrmBridgeStub.java:75
(Diff revision 1)
> +                                                  promiseId,
> +                                                  sessionId,
> +                                                  request);
> +            } catch (RemoteException e) {
> +                // Dead recipient.
> +                e.printStackTrace();

Use `Log.e(LOGTAG, "MESSAGE", e);` and elsewhere

::: mobile/android/base/java/org/mozilla/gecko/media/RemoteMediaDrmBridgeStub.java:149
(Diff revision 1)
> +                e.printStackTrace();
> +            }
> +        }
> +    }
> +
> +    private static void assertTrue(boolean condition) {

Package-private access.
Attachment #8805461 - Flags: review?(nchen) → review+
Comment on attachment 8805460 [details]
Bug 1306196-[P1] Provide AIDL definiion for EME functionality on Fennec MediaManager Service.

https://reviewboard.mozilla.org/r/89196/#review89302

All issues fixed & patches rebased.
Thanks !
Blocks: 1314530
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4eab781604ad
[P1] Provide AIDL definiion for EME functionality on Fennec MediaManager Service. r=cpearce,jchen
https://hg.mozilla.org/integration/autoland/rev/0aa5b42dccea
[P2] Provide RemoteMediaDrmBridge & corresponding stub to underlying GeckoMediaDrmBridge implementation. r=cpearce,jchen
Keywords: checkin-needed
backed this out because parts depend on bug 1306185 which needed to be backed out
Flags: needinfo?(kikuo)
Comment on attachment 8805460 [details]
Bug 1306196-[P1] Provide AIDL definiion for EME functionality on Fennec MediaManager Service.

https://reviewboard.mozilla.org/r/89196/#review89764

Rebase again for a new try.
Lint error was fixed in bug 1306185.

New try, https://treeherder.mozilla.org/#/jobs?repo=try&revision=54b9034fe1d8, lovely green !
Flags: needinfo?(kikuo)
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4c5036988239
[P1] Provide AIDL definiion for EME functionality on Fennec MediaManager Service. r=cpearce,jchen
https://hg.mozilla.org/integration/autoland/rev/16309437315f
[P2] Provide RemoteMediaDrmBridge & corresponding stub to underlying GeckoMediaDrmBridge implementation. r=cpearce,jchen
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4c5036988239
https://hg.mozilla.org/mozilla-central/rev/16309437315f
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
You need to log in before you can comment on or make changes to this bug.