Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MediaDrm.release not called in all cases #4721

Closed
nahojkap opened this issue Aug 23, 2018 · 8 comments
Closed

MediaDrm.release not called in all cases #4721

nahojkap opened this issue Aug 23, 2018 · 8 comments
Assignees
@AquilesCanta
Labels
bug

Comments

@nahojkap
Copy link

While reviewing playback issues with PlayReady DRM on Android TV devices, I noticed that the MediaDRM#release method is never called when the DRM session manager is released.

Not sure it has an impact but for completeness, the DefaultDrmSession#release method should probably do this when it is called?
@ojw28
Copy link
Contributor

ojw28 commented Aug 24, 2018

In the demo app it's application code that both instantiates and releases the MediaDrm instance, and this is one way of doing it. We do however have static methods like DefaultDrmSessionManager.newFrameworkInstance where the MediaDrm is instantiated internally, and not also released internally, which doesn't seem right.

We need to figure out what the best thing to do here is. It may tie in with some other DRM changes that we need to make.

@ojw28 ojw28 added the bug label Aug 24, 2018
@ojw28 ojw28 changed the title Should call MediaDrm#release for completeness? MediaDrm.release not called in all cases Aug 24, 2018
@nahojkap
Copy link
Author

I see, we are indeed now releasing it as part of our cleanup.

Curious about the 'other DRM changes' you are referring to, anything you can share?

@ram992
Copy link

ram992 commented Sep 7, 2018

Hello @ojw28, can this bug be the cause of issue discussed in #4674 that I am experiencing.

@ojw28
Copy link
Contributor

ojw28 commented Sep 7, 2018 •
edited
Loading

I doubt it (but if you want to find out, why don't you just try using DefaultDrmSessionManager in a way that ensures the instance is released, as in the demo app?).

@ram992
Copy link

ram992 commented Sep 7, 2018

Nice Idea, will try that out ASAP

@ojw28
Copy link
Contributor

ojw28 commented May 31, 2019

DefaultDrmSessionManager and OfflineLicenseHelper both have this problem. @AquilesCanta - How would you feel about adding a release() method to both, which does nothing if the application injected the mediaDrm itself, but releases the mediaDrm if it was constructed internally?

@ojw28 ojw28 mentioned this issue Jul 8, 2019
Closed
@ojw28
Copy link
Contributor

ojw28 commented Jul 8, 2019

@AquilesCanta - It would be good to get this fixed. See the issue duped above for a bit more information on why.

ojw28 pushed a commit that referenced this issue Sep 16, 2019
@AquilesCanta @ojw28
Add ExoMediaDrm.Provider
9face09 
Allows shared ownership of ExoMediaDrms. Shared ownership will
allow users to pre-create ExoMediaDrms in their apps, as opposed
to having the DrmSessionManager create the ExoMediaDrm.

Issue:#4721
PiperOrigin-RevId: 269305850
ojw28 pushed a commit that referenced this issue Sep 16, 2019
@AquilesCanta @ojw28
Add default implementations for ExoMediaDrm.Provider
6ae3e09 
Issue:#4721
PiperOrigin-RevId: 269378440
ojw28 pushed a commit that referenced this issue Sep 21, 2019
@AquilesCanta @ojw28
Add prepare and release to DrmSessionManager
7fc9c84 
The added methods will manage ExoMediaDrms instances.

Issue:#4721
PiperOrigin-RevId: 270335916
ojw28 pushed a commit that referenced this issue Sep 21, 2019
@AquilesCanta @ojw28
Call prepare and release from MediaSources
18b9304 
Issue:#4721
PiperOrigin-RevId: 270342454
ojw28 pushed a commit that referenced this issue Oct 2, 2019
@AquilesCanta @ojw28
Remove DefaultDrmSessionManager factory methods that enforce MediaDrm…
cc8b774 
… leaks

Inline invocations of these methods, which still leaks the MediaDrms.
However, it will be fixed once the DefaultDrmSessionManager API is finalized
and ExoMediaDrm.Providers are introduced.

Issue:#4721
PiperOrigin-RevId: 270681467
ojw28 pushed a commit that referenced this issue Oct 2, 2019
@AquilesCanta @ojw28
Remove mediaDrm methods from DefaultDrmSessionManager
fa80396 
DrmSessionManagers may now be in released state, in which case they
may release their MediaDrm. In that case, it would be invalid to
forward method calls to the underlying MediaDrms. Users should
instead call these methods directly on the MediaDrm.

Issue:#4721
PiperOrigin-RevId: 270963393
ojw28 pushed a commit that referenced this issue Oct 2, 2019
@AquilesCanta @ojw28
Introduce ExoMediaDrm.Provider into DefaultDrmSessionManager
c2bab7a 
Issue:#4721
PiperOrigin-RevId: 271127127
ojw28 pushed a commit that referenced this issue Oct 2, 2019
@AquilesCanta @ojw28
Add DefaultDrmSessionManager.Builder
b50da6d 
Issue:#6334
Issue:#4721
Issue:#6334
Issue:#4867
PiperOrigin-RevId: 271577773
@ojw28
Copy link
Contributor

ojw28 commented Nov 15, 2019

The last fix needed for this was submitted in 51711a0.

@ojw28 ojw28 closed this as completed Nov 15, 2019
@google google locked and limited conversation to collaborators Jan 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@AquilesCanta AquilesCanta

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@nahojkap @ojw28 @AquilesCanta @ram992