From b9c977574513be596259a0ff8bb58349e7985c33 Mon Sep 17 00:00:00 2001 From: kimvde Date: Thu, 12 Dec 2019 20:06:59 +0000 Subject: [PATCH] Refactor sample number computation in FLAC seeking Retrieve the sample number in the extractor instead of passing a holder to FlacBinarySearchSeeker. This change makes the code easier to understand and is required to implement the seeking from the seek table. PiperOrigin-RevId: 285241862 --- .../exoplayer2/extractor/ExtractorUtil.java | 52 ++++++++++++ .../exoplayer2/extractor/FlacFrameReader.java | 57 +++++++++---- .../flac/FlacBinarySearchSeeker.java | 31 +++---- .../extractor/flac/FlacExtractor.java | 18 +++-- .../extractor/ExtractorUtilTest.java | 80 +++++++++++++++++++ 5 files changed, 196 insertions(+), 42 deletions(-) create mode 100644 library/core/src/main/java/com/google/android/exoplayer2/extractor/ExtractorUtil.java create mode 100644 library/core/src/test/java/com/google/android/exoplayer2/extractor/ExtractorUtilTest.java diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/ExtractorUtil.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ExtractorUtil.java new file mode 100644 index 0000000000..3867a0fded --- /dev/null +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ExtractorUtil.java @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2019 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.extractor; + +import com.google.android.exoplayer2.C; +import java.io.IOException; + +/** Extractor related utility methods. */ +/* package */ final class ExtractorUtil { + + /** + * Peeks {@code length} bytes from the input peek position, or all the bytes to the end of the + * input if there was less than {@code length} bytes left. + * + *

If an exception is thrown, there is no guarantee on the peek position. + * + * @param input The stream input to peek the data from. + * @param target A target array into which data should be written. + * @param offset The offset into the target array at which to write. + * @param length The maximum number of bytes to peek from the input. + * @return The number of bytes peeked. + * @throws IOException If an error occurs peeking from the input. + * @throws InterruptedException If the thread has been interrupted. + */ + public static int peekToLength(ExtractorInput input, byte[] target, int offset, int length) + throws IOException, InterruptedException { + int totalBytesPeeked = 0; + while (totalBytesPeeked < length) { + int bytesPeeked = input.peek(target, offset + totalBytesPeeked, length - totalBytesPeeked); + if (bytesPeeked == C.RESULT_END_OF_INPUT) { + break; + } + totalBytesPeeked += bytesPeeked; + } + return totalBytesPeeked; + } + + private ExtractorUtil() {} +} diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/FlacFrameReader.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/FlacFrameReader.java index 6d7ce7efe8..58397b3e21 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/FlacFrameReader.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/FlacFrameReader.java @@ -15,7 +15,7 @@ */ package com.google.android.exoplayer2.extractor; -import com.google.android.exoplayer2.C; +import com.google.android.exoplayer2.ParserException; import com.google.android.exoplayer2.util.FlacConstants; import com.google.android.exoplayer2.util.FlacStreamMetadata; import com.google.android.exoplayer2.util.ParsableByteArray; @@ -107,19 +107,8 @@ public final class FlacFrameReader { System.arraycopy( frameStartBytes, /* srcPos= */ 0, scratch.data, /* destPos= */ 0, /* length= */ 2); - int totalBytesPeeked = 2; - while (totalBytesPeeked < FlacConstants.MAX_FRAME_HEADER_SIZE) { - int bytesPeeked = - input.peek( - scratch.data, - totalBytesPeeked, - FlacConstants.MAX_FRAME_HEADER_SIZE - totalBytesPeeked); - if (bytesPeeked == C.RESULT_END_OF_INPUT) { - // The size of the last frame is less than MAX_FRAME_HEADER_SIZE. - break; - } - totalBytesPeeked += bytesPeeked; - } + int totalBytesPeeked = + ExtractorUtil.peekToLength(input, scratch.data, 2, FlacConstants.MAX_FRAME_HEADER_SIZE - 2); scratch.setLimit(totalBytesPeeked); input.resetPeekPosition(); @@ -129,6 +118,46 @@ public final class FlacFrameReader { scratch, flacStreamMetadata, frameStartMarker, sampleNumberHolder); } + /** + * Returns the number of the first sample in the given frame. + * + *

The read position of {@code input} is left unchanged. + * + *

If no exception is thrown, the peek position is aligned with the read position. Otherwise, + * there is no guarantee on the peek position. + * + * @param input Input stream to get the sample number from (starting from the read position). + * @return The frame first sample number. + * @throws ParserException If an error occurs parsing the sample number. + * @throws IOException If peeking from the input fails. + * @throws InterruptedException If interrupted while peeking from input. + */ + public static long getFirstSampleNumber( + ExtractorInput input, FlacStreamMetadata flacStreamMetadata) + throws IOException, InterruptedException { + input.resetPeekPosition(); + input.advancePeekPosition(1); + byte[] blockingStrategyByte = new byte[1]; + input.peekFully(blockingStrategyByte, 0, 1); + boolean isBlockSizeVariable = (blockingStrategyByte[0] & 1) == 1; + input.advancePeekPosition(2); + + int maxUtf8SampleNumberSize = isBlockSizeVariable ? 7 : 6; + ParsableByteArray scratch = new ParsableByteArray(maxUtf8SampleNumberSize); + int totalBytesPeeked = + ExtractorUtil.peekToLength(input, scratch.data, 0, maxUtf8SampleNumberSize); + scratch.setLimit(totalBytesPeeked); + input.resetPeekPosition(); + + SampleNumberHolder sampleNumberHolder = new SampleNumberHolder(); + if (!checkAndReadFirstSampleNumber( + scratch, flacStreamMetadata, isBlockSizeVariable, sampleNumberHolder)) { + throw new ParserException(); + } + + return sampleNumberHolder.sampleNumber; + } + /** * Reads the given block size. * diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/flac/FlacBinarySearchSeeker.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/flac/FlacBinarySearchSeeker.java index c63077261c..82e1636baf 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/flac/FlacBinarySearchSeeker.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/flac/FlacBinarySearchSeeker.java @@ -40,18 +40,15 @@ import java.io.IOException; * in the stream must start. * @param firstFramePosition The byte offset of the first frame in the stream. * @param inputLength The length of the stream in bytes. - * @param sampleNumberHolder A holder used to retrieve the sample number of a frame found by a - * seeking operation. */ public FlacBinarySearchSeeker( FlacStreamMetadata flacStreamMetadata, int frameStartMarker, long firstFramePosition, - long inputLength, - SampleNumberHolder sampleNumberHolder) { + long inputLength) { super( /* seekTimestampConverter= */ flacStreamMetadata::getSampleNumber, - new FlacTimestampSeeker(flacStreamMetadata, frameStartMarker, sampleNumberHolder), + new FlacTimestampSeeker(flacStreamMetadata, frameStartMarker), flacStreamMetadata.getDurationUs(), /* floorTimePosition= */ 0, /* ceilingTimePosition= */ flacStreamMetadata.totalSamples, @@ -66,21 +63,16 @@ import java.io.IOException; private final FlacStreamMetadata flacStreamMetadata; private final int frameStartMarker; - private final SampleNumberHolder foundFrameSampleNumberHolder; - private final SampleNumberHolder tempSampleNumberHolder; + private final SampleNumberHolder sampleNumberHolder; - private FlacTimestampSeeker( - FlacStreamMetadata flacStreamMetadata, - int frameStartMarker, - SampleNumberHolder foundFrameSampleNumberHolder) { + private FlacTimestampSeeker(FlacStreamMetadata flacStreamMetadata, int frameStartMarker) { this.flacStreamMetadata = flacStreamMetadata; this.frameStartMarker = frameStartMarker; - this.foundFrameSampleNumberHolder = foundFrameSampleNumberHolder; - tempSampleNumberHolder = new SampleNumberHolder(); + sampleNumberHolder = new SampleNumberHolder(); } @Override - public TimestampSearchResult searchForTimestamp(ExtractorInput input, long targetSampleIndex) + public TimestampSearchResult searchForTimestamp(ExtractorInput input, long targetSampleNumber) throws IOException, InterruptedException { long searchPosition = input.getPosition(); @@ -95,11 +87,10 @@ import java.io.IOException; long rightFrameFirstSampleNumber = findNextFrame(input); long rightFramePosition = input.getPeekPosition(); - if (leftFrameFirstSampleNumber <= targetSampleIndex - && rightFrameFirstSampleNumber > targetSampleIndex) { - foundFrameSampleNumberHolder.sampleNumber = leftFrameFirstSampleNumber; + if (leftFrameFirstSampleNumber <= targetSampleNumber + && rightFrameFirstSampleNumber > targetSampleNumber) { return TimestampSearchResult.targetFoundResult(leftFramePosition); - } else if (rightFrameFirstSampleNumber <= targetSampleIndex) { + } else if (rightFrameFirstSampleNumber <= targetSampleNumber) { return TimestampSearchResult.underestimatedResult( rightFrameFirstSampleNumber, rightFramePosition); } else { @@ -125,7 +116,7 @@ import java.io.IOException; private long findNextFrame(ExtractorInput input) throws IOException, InterruptedException { while (input.getPeekPosition() < input.getLength() - FlacConstants.MIN_FRAME_HEADER_SIZE && !FlacFrameReader.checkFrameHeaderFromPeek( - input, flacStreamMetadata, frameStartMarker, tempSampleNumberHolder)) { + input, flacStreamMetadata, frameStartMarker, sampleNumberHolder)) { input.advancePeekPosition(1); } @@ -134,7 +125,7 @@ import java.io.IOException; return flacStreamMetadata.totalSamples; } - return tempSampleNumberHolder.sampleNumber; + return sampleNumberHolder.sampleNumber; } } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/flac/FlacExtractor.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/flac/FlacExtractor.java index ae0f50ab5b..078fdddbd1 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/flac/FlacExtractor.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/flac/FlacExtractor.java @@ -177,6 +177,7 @@ public final class FlacExtractor implements Extractor { state = STATE_READ_ID3_METADATA; currentFrameFirstSampleNumber = 0; } else if (binarySearchSeeker != null) { + currentFrameFirstSampleNumber = SAMPLE_NUMBER_UNKNOWN; binarySearchSeeker.setSeekTargetUs(timeUs); } currentFrameBytesWritten = 0; @@ -243,9 +244,14 @@ public final class FlacExtractor implements Extractor { // Handle pending seek if necessary. if (binarySearchSeeker != null && binarySearchSeeker.isSeeking()) { - int seekResult = binarySearchSeeker.handlePendingSeek(input, seekPosition); - currentFrameFirstSampleNumber = sampleNumberHolder.sampleNumber; - return seekResult; + return binarySearchSeeker.handlePendingSeek(input, seekPosition); + } + + // Set current frame first sample number if it became unknown after seeking. + if (currentFrameFirstSampleNumber == SAMPLE_NUMBER_UNKNOWN) { + currentFrameFirstSampleNumber = + FlacFrameReader.getFirstSampleNumber(input, flacStreamMetadata); + return Extractor.RESULT_CONTINUE; } // Copy more bytes into the scratch. @@ -300,11 +306,7 @@ public final class FlacExtractor implements Extractor { } binarySearchSeeker = new FlacBinarySearchSeeker( - flacStreamMetadata, - frameStartMarker, - firstFramePosition, - streamLength, - sampleNumberHolder); + flacStreamMetadata, frameStartMarker, firstFramePosition, streamLength); return binarySearchSeeker.getSeekMap(); } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/extractor/ExtractorUtilTest.java b/library/core/src/test/java/com/google/android/exoplayer2/extractor/ExtractorUtilTest.java new file mode 100644 index 0000000000..c8e8f869d8 --- /dev/null +++ b/library/core/src/test/java/com/google/android/exoplayer2/extractor/ExtractorUtilTest.java @@ -0,0 +1,80 @@ +/* + * Copyright (C) 2019 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.extractor; + +import static com.google.common.truth.Truth.assertThat; + +import android.net.Uri; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.android.exoplayer2.C; +import com.google.android.exoplayer2.testutil.FakeDataSource; +import com.google.android.exoplayer2.upstream.DataSpec; +import java.util.Arrays; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** Unit test for {@link ExtractorUtil}. */ +@RunWith(AndroidJUnit4.class) +public class ExtractorUtilTest { + + private static final String TEST_URI = "http://www.google.com"; + private static final byte[] TEST_DATA = new byte[] {0, 1, 2, 3, 4, 5, 6, 7, 8}; + + @Test + public void testPeekToLengthEndNotReached() throws Exception { + FakeDataSource testDataSource = new FakeDataSource(); + testDataSource + .getDataSet() + .newDefaultData() + .appendReadData(Arrays.copyOfRange(TEST_DATA, 0, 3)) + .appendReadData(Arrays.copyOfRange(TEST_DATA, 3, 6)) + .appendReadData(Arrays.copyOfRange(TEST_DATA, 6, 9)); + testDataSource.open(new DataSpec(Uri.parse(TEST_URI))); + ExtractorInput input = new DefaultExtractorInput(testDataSource, 0, C.LENGTH_UNSET); + byte[] target = new byte[TEST_DATA.length]; + int offset = 2; + int length = 4; + + int bytesPeeked = ExtractorUtil.peekToLength(input, target, offset, length); + + assertThat(bytesPeeked).isEqualTo(length); + assertThat(input.getPeekPosition()).isEqualTo(length); + assertThat(Arrays.copyOfRange(target, offset, offset + length)) + .isEqualTo(Arrays.copyOf(TEST_DATA, length)); + } + + @Test + public void testPeekToLengthEndReached() throws Exception { + FakeDataSource testDataSource = new FakeDataSource(); + testDataSource + .getDataSet() + .newDefaultData() + .appendReadData(Arrays.copyOfRange(TEST_DATA, 0, 3)) + .appendReadData(Arrays.copyOfRange(TEST_DATA, 3, 6)) + .appendReadData(Arrays.copyOfRange(TEST_DATA, 6, 9)); + testDataSource.open(new DataSpec(Uri.parse(TEST_URI))); + ExtractorInput input = new DefaultExtractorInput(testDataSource, 0, C.LENGTH_UNSET); + byte[] target = new byte[TEST_DATA.length]; + int offset = 0; + int length = TEST_DATA.length + 1; + + int bytesPeeked = ExtractorUtil.peekToLength(input, target, offset, length); + + assertThat(bytesPeeked).isEqualTo(TEST_DATA.length); + assertThat(input.getPeekPosition()).isEqualTo(TEST_DATA.length); + assertThat(target).isEqualTo(TEST_DATA); + } +}