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.

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

Make OOM failures in extractors non-fatal #2780

Closed
PaulWoitaschek opened this issue May 6, 2017 · 8 comments
Closed

Make OOM failures in extractors non-fatal #2780

PaulWoitaschek opened this issue May 6, 2017 · 8 comments

Comments

@PaulWoitaschek
Copy link

PaulWoitaschek commented May 6, 2017

In crashlytics I see some OOM where a really large amount of ram was tried to be allocated.

http://crashes.to/s/b7e79fa945f

That's about 304 MB the player tries to allocate while reading ID3 Data. Maybe the parsing went wrong?

It's ExoPlayer 2.4.0

@ojw28
Copy link
Contributor

ojw28 commented May 8, 2017

According to the link only a single user is affected. It's likely they have a single piece of broken content that they're trying to play repeatedly. We should be able to avoid OOM'ing in this case, but I don't think we can avoid playback failure.

It's not really possible to say without access to the problematic file. Is it possible for you to source it from your user?

@PaulWoitaschek
Copy link
Author

I can't get the file. I use exoplayer to analyze the duration
https://github.com/PaulWoitaschek/MaterialAudiobookPlayer/blob/master/audiobook/src/main/java/de/ph1b/audiobook/misc/DurationAnalyzer.kt

So it's crashing each time he is opening the app.

@PaulWoitaschek
Copy link
Author

Also I see other OOM where exoplayer tries to allocate > 30 MB in com.google.android.exoplayer2.extractor.mp4.AtomParsers.parseStbl (AtomParsers.java:362)

I'm not sure if that is wanted. Since I switched to ExoPlayer about 5% of my users receive crashes due to OOM errors.

@andrewlewis
Copy link
Collaborator

Please provide a link to the content that causes the MP4 parsing error.

@PaulWoitaschek
Copy link
Author

PaulWoitaschek commented May 8, 2017

I don't have access to the content of my users.

The stracktrace for these OOMs is always like this:

Fatal Exception: java.lang.OutOfMemoryError: Failed to allocate a 31891368 byte allocation with 10482944 free bytes and 9MB until OOM
       at com.google.android.exoplayer2.extractor.mp4.AtomParsers.parseStbl(AtomParsers.java:362)
       at com.google.android.exoplayer2.extractor.mp4.Mp4Extractor.processMoovAtom(Mp4Extractor.java:341)
       at com.google.android.exoplayer2.extractor.mp4.Mp4Extractor.processAtomEnded(Mp4Extractor.java:276)
       at com.google.android.exoplayer2.extractor.mp4.Mp4Extractor.readAtomPayload(Mp4Extractor.java:267)
       at com.google.android.exoplayer2.extractor.mp4.Mp4Extractor.read(Mp4Extractor.java:147)
       at com.google.android.exoplayer2.source.ExtractorMediaPeriod$ExtractingLoadable.load(ExtractorMediaPeriod.java:652)
       at com.google.android.exoplayer2.upstream.Loader$LoadTask.run(Loader.java:295)
       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1113)
       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:588)
       at java.lang.Thread.run(Thread.java:818)

So if I understand correctly you are allocation a really huge array here

@ojw28
Copy link
Contributor

ojw28 commented May 8, 2017

Yes, but only if the content actually indicates that we need to. For the first OOM reported here we could add some validation to fail the playback/extraction more gracefully. For the second one I'm not sure.

We should probably be making the OOM failure non-fatal; but I doubt we're going to get to the stage of actually playing this kind of content successfully; the content is likely malformed.

@ojw28 ojw28 changed the title OOM Error in peekId3Data Make OOM failures in extractors non-fatal May 8, 2017
@PaulWoitaschek
Copy link
Author

I'm really happy when the player does not crash on malformed files but just refuses to play them. My player is quite unstable right now.

ojw28 added a commit that referenced this issue May 8, 2017
This is most commonly caused by malformed media, where
the media indicates that something we need to make an
allocation for is *really huge*. Failing playback is
appropriate for this case; killing the process is not.

Issue: #2780

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=155408062
@ojw28 ojw28 closed this as completed May 8, 2017
@PaulWoitaschek
Copy link
Author

Thanks a lot for the fast solution!

ojw28 added a commit that referenced this issue May 11, 2017
This is most commonly caused by malformed media, where
the media indicates that something we need to make an
allocation for is *really huge*. Failing playback is
appropriate for this case; killing the process is not.

Issue: #2780

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=155408062
@google google locked and limited conversation to collaborators Sep 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants