From 38b9a5d441ec9eede9b5224dcf7fb34edc5afcd4 Mon Sep 17 00:00:00 2001 From: tonihei Date: Wed, 8 May 2024 03:10:21 -0700 Subject: [PATCH] Always check audio processor chain for media playout duration When the PlaybackParameters are set to their DEFAULT value, we currently bypass the audio processor chain when determining the output media position, under the assumption that no timestamp change happens in the audio processors. This assumption may not be true as the audio processors can change playout durations on their own accord independent of the provided PlaybackParameters. To correctly reflect any updated playout duration, we can just always check the audio processor chain. The default implementation will continue to assume that only the SonicAudioProcessor changes the playout duration. PiperOrigin-RevId: 631726112 --- .../exoplayer/audio/DefaultAudioSink.java | 8 +- .../exoplayer/audio/DefaultAudioSinkTest.java | 83 +++++++++++++++---- 2 files changed, 72 insertions(+), 19 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 a6753232e4..19f321741c 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 @@ -192,7 +192,9 @@ public final class DefaultAudioSink implements AudioSink { @Override public long getMediaDuration(long playoutDuration) { - return sonicAudioProcessor.getMediaDuration(playoutDuration); + return sonicAudioProcessor.isActive() + ? sonicAudioProcessor.getMediaDuration(playoutDuration) + : playoutDuration; } @Override @@ -1675,9 +1677,7 @@ public final class DefaultAudioSink implements AudioSink { long playoutDurationSinceLastCheckpointUs = positionUs - mediaPositionParameters.audioTrackPositionUs; - if (mediaPositionParameters.playbackParameters.equals(PlaybackParameters.DEFAULT)) { - return mediaPositionParameters.mediaTimeUs + playoutDurationSinceLastCheckpointUs; - } else if (mediaPositionParametersCheckpoints.isEmpty()) { + if (mediaPositionParametersCheckpoints.isEmpty()) { long mediaDurationSinceLastCheckpointUs = audioProcessorChain.getMediaDuration(playoutDurationSinceLastCheckpointUs); return mediaPositionParameters.mediaTimeUs + mediaDurationSinceLastCheckpointUs; diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/audio/DefaultAudioSinkTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/audio/DefaultAudioSinkTest.java index 0805e4d1a4..5b6295ac81 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/audio/DefaultAudioSinkTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/audio/DefaultAudioSinkTest.java @@ -24,18 +24,22 @@ import androidx.media3.common.C; import androidx.media3.common.Format; import androidx.media3.common.MimeTypes; import androidx.media3.common.PlaybackParameters; +import androidx.media3.common.audio.AudioProcessor; +import androidx.media3.common.audio.AudioProcessorChain; import androidx.media3.exoplayer.audio.DefaultAudioSink.DefaultAudioProcessorChain; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.util.Arrays; +import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowSystemClock; /** Unit tests for {@link DefaultAudioSink}. */ @RunWith(AndroidJUnit4.class) @@ -75,12 +79,61 @@ public final class DefaultAudioSinkTest { new DefaultAudioSink.Builder().setAudioProcessors(new TeeAudioProcessor[0]).build(); } + @Test + public void handlesBuffer_updatesPositionUsingAudioProcessorChain() throws Exception { + defaultAudioSink = + new DefaultAudioSink.Builder() + .setAudioProcessorChain( + new AudioProcessorChain() { + @Override + public AudioProcessor[] getAudioProcessors() { + return new AudioProcessor[0]; + } + + @Override + public PlaybackParameters applyPlaybackParameters( + PlaybackParameters playbackParameters) { + return playbackParameters; + } + + @Override + public boolean applySkipSilenceEnabled(boolean skipSilenceEnabled) { + return false; + } + + @Override + public long getMediaDuration(long playoutDuration) { + return playoutDuration * 2; + } + + @Override + public long getSkippedOutputFrameCount() { + return 441; // 0.01 seconds at 44.1 kHz + } + }) + .build(); + configureDefaultAudioSink(CHANNEL_COUNT_STEREO); + retryUntilTrue( + () -> + defaultAudioSink.handleBuffer( + create1Sec44100HzSilenceBuffer(), + /* presentationTimeUs= */ 0, + /* encodedAccessUnitCount= */ 1)); + defaultAudioSink.play(); + ShadowSystemClock.advanceBy(1, TimeUnit.SECONDS); + + long currentPositionUs = defaultAudioSink.getCurrentPositionUs(/* sourceEnded= */ false); + + // Based on audio processor chain: 1 second * 2 + 0.01 seconds + assertThat(currentPositionUs).isEqualTo(2_010_000); + } + @Test public void handlesBufferAfterReset() throws Exception { configureDefaultAudioSink(CHANNEL_COUNT_STEREO); assertThat( defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 0, /* encodedAccessUnitCount= */ 1)) .isTrue(); @@ -91,7 +144,7 @@ public final class DefaultAudioSinkTest { retryUntilTrue( () -> defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 0, /* encodedAccessUnitCount= */ 1)); } @@ -102,7 +155,7 @@ public final class DefaultAudioSinkTest { configureDefaultAudioSink(CHANNEL_COUNT_STEREO); assertThat( defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 0, /* encodedAccessUnitCount= */ 1)) .isTrue(); @@ -113,7 +166,7 @@ public final class DefaultAudioSinkTest { retryUntilTrue( () -> defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 0, /* encodedAccessUnitCount= */ 1)); assertThat(defaultAudioSink.getPlaybackParameters()) @@ -125,7 +178,7 @@ public final class DefaultAudioSinkTest { configureDefaultAudioSink(CHANNEL_COUNT_STEREO); assertThat( defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 0, /* encodedAccessUnitCount= */ 1)) .isTrue(); @@ -136,7 +189,7 @@ public final class DefaultAudioSinkTest { retryUntilTrue( () -> defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 0, /* encodedAccessUnitCount= */ 1)); } @@ -147,7 +200,7 @@ public final class DefaultAudioSinkTest { configureDefaultAudioSink(CHANNEL_COUNT_STEREO); assertThat( defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 0, /* encodedAccessUnitCount= */ 1)) .isTrue(); @@ -158,7 +211,7 @@ public final class DefaultAudioSinkTest { retryUntilTrue( () -> defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 0, /* encodedAccessUnitCount= */ 1)); assertThat(defaultAudioSink.getPlaybackParameters()) @@ -173,7 +226,7 @@ public final class DefaultAudioSinkTest { /* trimEndFrames= */ 0); assertThat( defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 0, /* encodedAccessUnitCount= */ 1)) .isTrue(); @@ -193,7 +246,7 @@ public final class DefaultAudioSinkTest { /* trimEndFrames= */ TRIM_10_MS_FRAME_COUNT); assertThat( defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 0, /* encodedAccessUnitCount= */ 1)) .isTrue(); @@ -213,7 +266,7 @@ public final class DefaultAudioSinkTest { /* trimEndFrames= */ TRIM_10_MS_FRAME_COUNT); assertThat( defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 0, /* encodedAccessUnitCount= */ 1)) .isTrue(); @@ -230,7 +283,7 @@ public final class DefaultAudioSinkTest { configureDefaultAudioSink(CHANNEL_COUNT_STEREO); assertThat( defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 5 * C.MICROS_PER_SECOND, /* encodedAccessUnitCount= */ 1)) .isTrue(); @@ -242,7 +295,7 @@ public final class DefaultAudioSinkTest { retryUntilTrue( () -> defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 8 * C.MICROS_PER_SECOND, /* encodedAccessUnitCount= */ 1)); assertThat(defaultAudioSink.getCurrentPositionUs(/* sourceEnded= */ false)) @@ -336,7 +389,7 @@ public final class DefaultAudioSinkTest { configureDefaultAudioSink(/* channelCount= */ 2); assertThat( defaultAudioSink.handleBuffer( - createDefaultSilenceBuffer(), + create1Sec44100HzSilenceBuffer(), /* presentationTimeUs= */ 5 * C.MICROS_PER_SECOND, /* encodedAccessUnitCount= */ 1)) .isTrue(); @@ -372,7 +425,7 @@ public final class DefaultAudioSinkTest { } /** Creates a one second silence buffer for 44.1 kHz stereo 16-bit audio. */ - private static ByteBuffer createDefaultSilenceBuffer() { + private static ByteBuffer create1Sec44100HzSilenceBuffer() { return ByteBuffer.allocateDirect( SAMPLE_RATE_44_1 * CHANNEL_COUNT_STEREO * BYTES_PER_FRAME_16_BIT) .order(ByteOrder.nativeOrder());