From a83ab05aeceb6c99f3b7b19d6fedd20f317e0aa6 Mon Sep 17 00:00:00 2001 From: tonihei Date: Thu, 7 Jul 2022 10:22:56 +0000 Subject: [PATCH] Don't block AudioTrack when waiting for previous release 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 --- .../exoplayer/audio/DefaultAudioSink.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/audio/DefaultAudioSink.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/audio/DefaultAudioSink.java index c1a34adb68..b53d79c47e 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/audio/DefaultAudioSink.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/audio/DefaultAudioSink.java @@ -29,7 +29,6 @@ import android.media.AudioTrack; import android.media.PlaybackParams; import android.media.metrics.LogSessionId; -import android.os.ConditionVariable; import android.os.Handler; import android.os.SystemClock; import android.util.Pair; @@ -44,6 +43,8 @@ import androidx.media3.common.MimeTypes; import androidx.media3.common.PlaybackParameters; import androidx.media3.common.util.Assertions; +import androidx.media3.common.util.Clock; +import androidx.media3.common.util.ConditionVariable; import androidx.media3.common.util.Log; import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; @@ -615,7 +616,8 @@ private DefaultAudioSink(Builder builder) { enableAudioTrackPlaybackParams = Util.SDK_INT >= 23 && builder.enableAudioTrackPlaybackParams; offloadMode = Util.SDK_INT >= 29 ? builder.offloadMode : OFFLOAD_MODE_DISABLED; audioTrackBufferSizeProvider = builder.audioTrackBufferSizeProvider; - releasingConditionVariable = new ConditionVariable(true); + releasingConditionVariable = new ConditionVariable(Clock.DEFAULT); + releasingConditionVariable.open(); audioTrackPositionTracker = new AudioTrackPositionTracker(new PositionTrackerListener()); channelMappingAudioProcessor = new ChannelMappingAudioProcessor(); trimmingAudioProcessor = new TrimmingAudioProcessor(); @@ -840,13 +842,15 @@ private void flushAudioProcessors() { } } - private void initializeAudioTrack() throws InitializationException { - // If we're asynchronously releasing a previous audio track then we block until it has been + private boolean initializeAudioTrack() throws InitializationException { + // If we're asynchronously releasing a previous audio track then we wait until it has been // released. This guarantees that we cannot end up in a state where we have multiple audio // track instances. Without this guarantee it would be possible, in extreme cases, to exhaust // the shared memory that's available for audio track buffers. This would in turn cause the // initialization of the audio track to fail. - releasingConditionVariable.block(); + if (!releasingConditionVariable.isOpen()) { + return false; + } audioTrack = buildAudioTrackWithRetry(); if (isOffloadedPlayback(audioTrack)) { @@ -874,6 +878,7 @@ private void initializeAudioTrack() throws InitializationException { } startMediaTimeUsNeedsInit = true; + return true; } @Override @@ -930,7 +935,10 @@ public boolean handleBuffer( if (!isAudioTrackInitialized()) { try { - initializeAudioTrack(); + if (!initializeAudioTrack()) { + // Not yet ready for initialization of a new AudioTrack. + return false; + } } catch (InitializationException e) { if (e.isRecoverable) { throw e; // Do not delay the exception if it can be recovered at higher level.