[EME][Fennec] Instantiate MediaDrmCDMProxy and pass MediaPrefs::PDMAndroidRemoteCodecEnabled() to MediaCDMProxy to determine RemoteMediaDrmBridge or LocalMediaDrmBridge

RESOLVED FIXED in Firefox 52

Status

()

Firefox for Android
Audio/Video
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: JamesCheng, Unassigned)

Tracking

unspecified
Firefox 52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Reporter)

Description

2 years ago
Once bug 1306196, bug 1306185, bug 1306572 got landed, we need to create the MediaDrmBrige instance by the preference to determine it is oop case or non-oop case.
(Reporter)

Updated

2 years ago
Summary: Instantiate RemoteMediaDrmBridge or LocalMediaDrmBridge by MediaPrefs::PDMAndroidRemoteCodecEnabled() → [EME][Fennec] Instantiate RemoteMediaDrmBridge or LocalMediaDrmBridge by MediaPrefs::PDMAndroidRemoteCodecEnabled()
Priority: -- → P2
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Reporter)

Updated

2 years ago
Attachment #8808039 - Flags: review?(cpearce)
Attachment #8808040 - Flags: review?(cpearce)
Attachment #8808041 - Flags: review?(cpearce)
(Reporter)

Updated

2 years ago
Summary: [EME][Fennec] Instantiate RemoteMediaDrmBridge or LocalMediaDrmBridge by MediaPrefs::PDMAndroidRemoteCodecEnabled() → [EME][Fennec] Instantiate MediaDrmCDMProxy and pass MediaPrefs::PDMAndroidRemoteCodecEnabled() to MediaCDMProxy to determine RemoteMediaDrmBridge or LocalMediaDrmBridge

Comment 5

2 years ago
mozreview-review
Comment on attachment 8808040 [details]
Bug 1314530 Part2 - Add isRemote flag to MediaDrmProxy::Create function to determine if it is oop case.

https://reviewboard.mozilla.org/r/90964/#review90720

::: mobile/android/base/java/org/mozilla/gecko/media/MediaDrmProxy.java:253
(Diff revision 1)
>  
>      @WrapForJNI(calledFrom = "gecko")
>      public static MediaDrmProxy create(String keySystem,
> -                                       Callbacks nativeCallbacks) {
> +                                       Callbacks nativeCallbacks,
> +                                       boolean isRemote) {
> +        // TODO: Bug 1307818 wiil implement the instantiate logic by the isRemote flag

1) s/wiil/will
2) (Maybe) rephrase it to 
TODO: Will implement {Local,Remote}MediaDrmBridge instantiation by '''isRemote''' flag in Bug XXX.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 8

2 years ago
mozreview-review
Comment on attachment 8808039 [details]
Bug 1314530 Part1 - Remove IsInPrivateBrowsing() since we did not use it anymore.

https://reviewboard.mozilla.org/r/90962/#review90970
Attachment #8808039 - Flags: review?(cpearce) → review+

Comment 9

2 years ago
mozreview-review
Comment on attachment 8808040 [details]
Bug 1314530 Part2 - Add isRemote flag to MediaDrmProxy::Create function to determine if it is oop case.

https://reviewboard.mozilla.org/r/90964/#review90972
Attachment #8808040 - Flags: review?(cpearce) → review+

Comment 10

2 years ago
mozreview-review
Comment on attachment 8808041 [details]
Bug 1314530 Part3 - Instantiate MediaDrmCDMProxy if it is widevine on fennec.

https://reviewboard.mozilla.org/r/90966/#review90974

::: dom/media/eme/MediaKeys.h:137
(Diff revision 2)
>    bool IsBoundToMediaElement() const;
>  
>  private:
>  
> +  // Instantiate CDMProxy instance.
> +  // It could be MediaDrmCDMProxy(Widevine on Fennec) or GMPCDMProxy(the rest).

s/GMPCDMProxy(the rest)/GMPCDMProxy (the rest)/

::: dom/media/eme/MediaKeys.cpp:344
(Diff revision 2)
> +                                 mConfig.mDistinctiveIdentifier == MediaKeysRequirement::Required,
> +                                 mConfig.mPersistentState == MediaKeysRequirement::Required);
> +  } else
> +#endif
> +  {
> +    proxy= new GMPCDMProxy(this,

s/proxy=/proxy =/
Attachment #8808041 - Flags: review?(cpearce) → review+
Comment hidden (mozreview-request)
(Reporter)

Comment 12

2 years ago
Thanks for your review!
Keywords: checkin-needed

Comment 13

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3879ad77bbcf
Part1 - Remove IsInPrivateBrowsing() since we did not use it anymore. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/13f6bd96aa78
Part2 - Add isRemote flag to MediaDrmProxy::Create function to determine if it is oop case. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/971aaa1646ed
Part3 - Instantiate MediaDrmCDMProxy if it is widevine on fennec. r=cpearce
Keywords: checkin-needed

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3879ad77bbcf
https://hg.mozilla.org/mozilla-central/rev/13f6bd96aa78
https://hg.mozilla.org/mozilla-central/rev/971aaa1646ed
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
You need to log in before you can comment on or make changes to this bug.