From 02af670a0f78d8bcab5f1c8ec3d7515685141cc6 Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Mon, 20 Apr 2020 08:15:18 +0100 Subject: [PATCH] Fix gapless playback Audio processors are now flushed twice after reconfiguration. The second flush call cleared the pending trim start bytes so transitions between tracks were no longer gapless. Fix this by removing logic to clear pending trim bytes on flush. As a result we may trim data incorrectly if there is a flush before any data has been handled for seeking to a non-zero position, but this edge case will happen rarely and the effect shouldn't be noticeable. PiperOrigin-RevId: 307344357 --- .../audio/TrimmingAudioProcessor.java | 18 ++-- .../audio/TrimmingAudioProcessorTest.java | 101 ++++++++++++++++++ 2 files changed, 111 insertions(+), 8 deletions(-) create mode 100644 library/core/src/test/java/com/google/android/exoplayer2/audio/TrimmingAudioProcessorTest.java diff --git a/library/core/src/main/java/com/google/android/exoplayer2/audio/TrimmingAudioProcessor.java b/library/core/src/main/java/com/google/android/exoplayer2/audio/TrimmingAudioProcessor.java index 8d84325d93..f630c267e6 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/audio/TrimmingAudioProcessor.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/audio/TrimmingAudioProcessor.java @@ -155,18 +155,20 @@ import java.nio.ByteBuffer; @Override protected void onFlush() { if (reconfigurationPending) { - // This is the initial flush after reconfiguration. Prepare to trim bytes from the start/end. + // Flushing activates the new configuration, so prepare to trim bytes from the start/end. reconfigurationPending = false; endBuffer = new byte[trimEndFrames * inputAudioFormat.bytesPerFrame]; pendingTrimStartBytes = trimStartFrames * inputAudioFormat.bytesPerFrame; - } else { - // This is a flush during playback (after the initial flush). We assume this was caused by a - // seek to a non-zero position and clear pending start bytes. This assumption may be wrong (we - // may be seeking to zero), but playing data that should have been trimmed shouldn't be - // noticeable after a seek. Ideally we would check the timestamp of the first input buffer - // queued after flushing to decide whether to trim (see also [Internal: b/77292509]). - pendingTrimStartBytes = 0; } + + // TODO(internal b/77292509): Flushing occurs to activate a configuration (handled above) but + // also when seeking within a stream. This implementation currently doesn't handle seek to start + // (where we need to trim at the start again), nor seeks to non-zero positions before start + // trimming has occurred (where we should set pendingTrimStartBytes to zero). These cases can be + // fixed by trimming in queueInput based on timestamp, once that information is available. + + // Any data in the end buffer should no longer be output if we are playing from a different + // position, so discard it and refill the buffer using new input. endBufferSize = 0; } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/audio/TrimmingAudioProcessorTest.java b/library/core/src/test/java/com/google/android/exoplayer2/audio/TrimmingAudioProcessorTest.java new file mode 100644 index 0000000000..19a1ad19c3 --- /dev/null +++ b/library/core/src/test/java/com/google/android/exoplayer2/audio/TrimmingAudioProcessorTest.java @@ -0,0 +1,101 @@ +/* + * Copyright 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.android.exoplayer2.audio; + +import static com.google.common.truth.Truth.assertThat; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.android.exoplayer2.C; +import com.google.android.exoplayer2.audio.AudioProcessor.AudioFormat; +import java.nio.ByteBuffer; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** Unit tests for {@link TrimmingAudioProcessor}. */ +@RunWith(AndroidJUnit4.class) +public final class TrimmingAudioProcessorTest { + + private static final AudioFormat AUDIO_FORMAT = + new AudioFormat(/* sampleRate= */ 44100, /* channelCount= */ 2, C.ENCODING_PCM_16BIT); + private static final int TRACK_ONE_UNTRIMMED_FRAME_COUNT = 1024; + private static final int TRACK_ONE_TRIM_START_FRAME_COUNT = 64; + private static final int TRACK_ONE_TRIM_END_FRAME_COUNT = 32; + private static final int TRACK_TWO_TRIM_START_FRAME_COUNT = 128; + private static final int TRACK_TWO_TRIM_END_FRAME_COUNT = 16; + + private static final int TRACK_ONE_BUFFER_SIZE_BYTES = + AUDIO_FORMAT.bytesPerFrame * TRACK_ONE_UNTRIMMED_FRAME_COUNT; + private static final int TRACK_ONE_TRIMMED_BUFFER_SIZE_BYTES = + TRACK_ONE_BUFFER_SIZE_BYTES + - AUDIO_FORMAT.bytesPerFrame + * (TRACK_ONE_TRIM_START_FRAME_COUNT + TRACK_ONE_TRIM_END_FRAME_COUNT); + + private TrimmingAudioProcessor trimmingAudioProcessor; + + @Before + public void setUp() { + trimmingAudioProcessor = new TrimmingAudioProcessor(); + } + + @After + public void tearDown() { + trimmingAudioProcessor.reset(); + } + + @Test + public void flushTwice_trimsStartAndEnd() throws Exception { + trimmingAudioProcessor.setTrimFrameCount( + TRACK_ONE_TRIM_START_FRAME_COUNT, TRACK_ONE_TRIM_END_FRAME_COUNT); + trimmingAudioProcessor.configure(AUDIO_FORMAT); + trimmingAudioProcessor.flush(); + trimmingAudioProcessor.flush(); + + int outputSizeBytes = feedAndDrainAudioProcessorToEndOfTrackOne(); + + assertThat(trimmingAudioProcessor.getTrimmedFrameCount()) + .isEqualTo(TRACK_ONE_TRIM_START_FRAME_COUNT + TRACK_ONE_TRIM_END_FRAME_COUNT); + assertThat(outputSizeBytes).isEqualTo(TRACK_ONE_TRIMMED_BUFFER_SIZE_BYTES); + } + + /** + * Feeds and drains the audio processor up to the end of track one, returning the total output + * size in bytes. + */ + private int feedAndDrainAudioProcessorToEndOfTrackOne() throws Exception { + // Feed and drain the processor, simulating a gapless transition to another track. + ByteBuffer inputBuffer = ByteBuffer.allocate(TRACK_ONE_BUFFER_SIZE_BYTES); + int outputSize = 0; + while (!trimmingAudioProcessor.isEnded()) { + if (inputBuffer.hasRemaining()) { + trimmingAudioProcessor.queueInput(inputBuffer); + if (!inputBuffer.hasRemaining()) { + // Reconfigure for a next track then begin draining. + trimmingAudioProcessor.setTrimFrameCount( + TRACK_TWO_TRIM_START_FRAME_COUNT, TRACK_TWO_TRIM_END_FRAME_COUNT); + trimmingAudioProcessor.configure(AUDIO_FORMAT); + trimmingAudioProcessor.queueEndOfStream(); + } + } + ByteBuffer outputBuffer = trimmingAudioProcessor.getOutput(); + outputSize += outputBuffer.remaining(); + outputBuffer.clear(); + } + trimmingAudioProcessor.reset(); + return outputSize; + } +}