-
Notifications
You must be signed in to change notification settings - Fork 6k
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
defaultAudioLink.flush causes OutOfMemory #10057
Comments
Sorry for the delay in addressing this. What do you think about using a shared single threaded In #8728, @andrewlewis is concerned that
This particular concern only applies when we try to release a player with an |
Yeah, a single threaded |
We wait until a previous AudioTrack has been released before creating a new one. This is currently done with a thread block operation, which may cause ANRs in the extreme case when someone attempts to release the player while this is still blocked. The problem can be avoided by just returning false from DefaultAudioSink.handleBuffer to try again until the previous AudioTrack is released. Reproduction steps to force the issue: 1. Add Thread.sleep(10000); to the AudioTrack release thread. 2. Add this to the demo app: private int positionMs = 0; Handler handler = new Handler(); handler.post(new Runnable() { @OverRide public void run() { player.seekTo(positionMs++); if (positionMs == 10) { player.release(); } else { handler.postDelayed(this, 1000); } } 3. Observe Player release timeout exception. These steps can't be easily captured in a unit test as we can't artifically delay the AudioTrack release from the test. Issue: google/ExoPlayer#10057 PiperOrigin-RevId: 459468912
We wait until a previous AudioTrack has been released before creating a new one. This is currently done with a thread block operation, which may cause ANRs in the extreme case when someone attempts to release the player while this is still blocked. The problem can be avoided by just returning false from DefaultAudioSink.handleBuffer to try again until the previous AudioTrack is released. Reproduction steps to force the issue: 1. Add Thread.sleep(10000); to the AudioTrack release thread. 2. Add this to the demo app: private int positionMs = 0; Handler handler = new Handler(); handler.post(new Runnable() { @OverRide public void run() { player.seekTo(positionMs++); if (positionMs == 10) { player.release(); } else { handler.postDelayed(this, 1000); } } 3. Observe Player release timeout exception. These steps can't be easily captured in a unit test as we can't artifically delay the AudioTrack release from the test. Issue: #10057 PiperOrigin-RevId: 459468912
We currently start a simple Thread to release AudioTracks asynchronously. If many AudioTracks are released at the same time, this may lead to OOM situations because we attempt to create multiple new threads. This can be improved by using a shared SingleThreadExecutor. In the simple case of one simmultaneous release, it's exactly the same behavior as before: create a thread and release it as soon as it's done. For multiple simultanous releases we get the advantage of sharing a single thread to avoid creating more than one at the same time. Issue: google/ExoPlayer#10057 PiperOrigin-RevId: 460698942
We currently start a simple Thread to release AudioTracks asynchronously. If many AudioTracks are released at the same time, this may lead to OOM situations because we attempt to create multiple new threads. This can be improved by using a shared SingleThreadExecutor. In the simple case of one simmultaneous release, it's exactly the same behavior as before: create a thread and release it as soon as it's done. For multiple simultanous releases we get the advantage of sharing a single thread to avoid creating more than one at the same time. Issue: #10057 PiperOrigin-RevId: 460698942
This should be fixed now by automatically using a single thread executor for releases that happen at the same time. |
We wait until a previous AudioTrack has been released before creating a new one. This is currently done with a thread block operation, which may cause ANRs in the extreme case when someone attempts to release the player while this is still blocked. The problem can be avoided by just returning false from DefaultAudioSink.handleBuffer to try again until the previous AudioTrack is released. Reproduction steps to force the issue: 1. Add Thread.sleep(10000); to the AudioTrack release thread. 2. Add this to the demo app: private int positionMs = 0; Handler handler = new Handler(); handler.post(new Runnable() { @OverRide public void run() { player.seekTo(positionMs++); if (positionMs == 10) { player.release(); } else { handler.postDelayed(this, 1000); } } 3. Observe Player release timeout exception. These steps can't be easily captured in a unit test as we can't artifically delay the AudioTrack release from the test. Issue: #10057 PiperOrigin-RevId: 459468912 (cherry picked from commit a80dd60)
We wait until a previous AudioTrack has been released before creating a new one. This is currently done with a thread block operation, which may cause ANRs in the extreme case when someone attempts to release the player while this is still blocked. The problem can be avoided by just returning false from DefaultAudioSink.handleBuffer to try again until the previous AudioTrack is released. Reproduction steps to force the issue: 1. Add Thread.sleep(10000); to the AudioTrack release thread. 2. Add this to the demo app: private int positionMs = 0; Handler handler = new Handler(); handler.post(new Runnable() { @OverRide public void run() { player.seekTo(positionMs++); if (positionMs == 10) { player.release(); } else { handler.postDelayed(this, 1000); } } 3. Observe Player release timeout exception. These steps can't be easily captured in a unit test as we can't artifically delay the AudioTrack release from the test. Issue: google/ExoPlayer#10057 PiperOrigin-RevId: 459468912 (cherry picked from commit a83ab05)
Which release contains this fix, @tonihei ? |
[Edited to correct my previous reply as only part of this made it into 2.18.1, thanks to @icbaker for the heads up] It's unreleased and will most likely go into 2.19.0 (as not all the referenced commits are marked for inclusion in a minor release). |
This is a follow up of the issue 8728
We encounter the same issue in our application. The context is that we are a feed application where we may play several videos at the same time in the feed. We can also play full screen video from time to time. We try to keep reusing the players that have already been created.
There are several circumstances where we will release a player:
Since we have quite a lot videos in our feed, we try to keep using the players and at the same time, we would release players periodically. There are some pages with lots of videos and we could suddenly release lots of players. I think the current implementation of the defaultAudioLink.flush() would lead to too many threads created in some cases in our application.
This is an example of the crash:
![image](https://user-images.githubusercontent.com/12901839/157966844-da8c1774-c46e-43d5-a136-b1ea978e50d9.png)
So it would be great to offer something like this
![image](https://user-images.githubusercontent.com/12901839/157966522-807ce1ec-b831-41fa-ae74-9894492e3cef.png)
The text was updated successfully, but these errors were encountered: