From 6147050b90aaaef91b921a832fbf88049f3be8d9 Mon Sep 17 00:00:00 2001 From: tonihei Date: Mon, 22 Jul 2024 09:56:18 -0700 Subject: [PATCH] Delay flush after AudioTrack pause to ramp down volume AudioTrack automatically ramps down volume when pausing. However, when this happens as part of a AudioSink.flush() operation, we need to postpone the actual flush() until the ramp down finished in the audio system. Otherwise audio is just cut off, creating pop sounds. Delaying the release is fine now, because DefaultAudioSink starts creating a new track immediately without waiting for the previous track to be released. Also using the opportunity to add more comments about related quirks of the AudioTrack flush/release handling for more context. PiperOrigin-RevId: 654794818 --- RELEASENOTES.md | 1 + .../exoplayer/audio/DefaultAudioSink.java | 59 ++++++++++++------- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 0285b7f5b4..bf7d64220a 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -40,6 +40,7 @@ * Audio: * Automatically configure CTA-2075 loudness metadata on the codec if present in the media. + * Ensure smooth volume ramp down when seeking. * Video: * `MediaCodecVideoRenderer` avoids decoding samples that are neither rendered nor used as reference by other samples. 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 0817d543c6..f2c878bc0d 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 @@ -24,6 +24,7 @@ import static androidx.media3.exoplayer.audio.AudioCapabilities.getCapabilities; import static java.lang.Math.max; import static java.lang.Math.min; import static java.lang.annotation.ElementType.TYPE_USE; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import android.content.Context; import android.media.AudioDeviceInfo; @@ -75,7 +76,8 @@ import java.lang.annotation.Target; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.util.ArrayDeque; -import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; import org.checkerframework.checker.nullness.qual.RequiresNonNull; @@ -107,6 +109,9 @@ public final class DefaultAudioSink implements AudioSink { */ private static final int REPORT_SKIPPED_SILENCE_DELAY_MS = 100; + /** The time it takes to ramp AudioTrack's volume up or down when pausing or starting to play. */ + private static final int AUDIO_TRACK_VOLUME_RAMP_TIME_MS = 20; + /** * Thrown when the audio track has provided a spurious timestamp, if {@link * #failOnSpuriousAudioTimestamp} is set. @@ -475,9 +480,10 @@ public final class DefaultAudioSink implements AudioSink { private static final Object releaseExecutorLock = new Object(); + @SuppressWarnings("NonFinalStaticField") // Intentional statically shared mutable state @GuardedBy("releaseExecutorLock") @Nullable - private static ExecutorService releaseExecutor; + private static ScheduledExecutorService releaseExecutor; @GuardedBy("releaseExecutorLock") private static int pendingReleaseCount; @@ -1432,6 +1438,9 @@ public final class DefaultAudioSink implements AudioSink { onRoutingChangedListener.release(); onRoutingChangedListener = null; } + // We need to release the audio track on every flush because of known issues on some devices + // See b/7941810 or b/19193985. + // TODO: b/143500232 - Experiment with not releasing AudioTrack on flush. releaseAudioTrackAsync(audioTrack, listener, oldAudioTrackConfig); audioTrack = null; } @@ -1827,27 +1836,37 @@ public final class DefaultAudioSink implements AudioSink { Handler audioTrackThreadHandler = new Handler(Looper.myLooper()); synchronized (releaseExecutorLock) { if (releaseExecutor == null) { - releaseExecutor = Util.newSingleThreadExecutor("ExoPlayer:AudioTrackReleaseThread"); + releaseExecutor = + Util.newSingleThreadScheduledExecutor("ExoPlayer:AudioTrackReleaseThread"); } pendingReleaseCount++; - releaseExecutor.execute( - () -> { - try { - audioTrack.flush(); - audioTrack.release(); - } finally { - if (listener != null && audioTrackThreadHandler.getLooper().getThread().isAlive()) { - audioTrackThreadHandler.post(() -> listener.onAudioTrackReleased(audioTrackConfig)); - } - synchronized (releaseExecutorLock) { - pendingReleaseCount--; - if (pendingReleaseCount == 0) { - releaseExecutor.shutdown(); - releaseExecutor = null; + Future ignored = + releaseExecutor.schedule( + () -> { + try { + // We need to flush the audio track as some devices are known to keep state from + // previous playbacks if the track is not flushed at all (see b/22967293). + audioTrack.flush(); + audioTrack.release(); + } finally { + if (listener != null + && audioTrackThreadHandler.getLooper().getThread().isAlive()) { + audioTrackThreadHandler.post( + () -> listener.onAudioTrackReleased(audioTrackConfig)); + } + synchronized (releaseExecutorLock) { + pendingReleaseCount--; + if (pendingReleaseCount == 0) { + releaseExecutor.shutdown(); + releaseExecutor = null; + } + } } - } - } - }); + }, + // We need to schedule the flush and release with a delay to ensure the audio system + // can completely ramp down the audio output after the preceding pause. + AUDIO_TRACK_VOLUME_RAMP_TIME_MS, + MILLISECONDS); } }