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

library/core/src/main/java/com/google/android/exoplayer2/extractor/ExtractorUtil.java

+/*
+ * 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.
+   *
+   * <p>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() {}
+}

library/core/src/main/java/com/google/android/exoplayer2/extractor/FlacFrameReader.java

@@ -15,7 +15,7 @@
  */
  */
 package com.google.android.exoplayer2.extractor;
 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.FlacConstants;
 import com.google.android.exoplayer2.util.FlacStreamMetadata;
 import com.google.android.exoplayer2.util.FlacStreamMetadata;
 import com.google.android.exoplayer2.util.ParsableByteArray;
 import com.google.android.exoplayer2.util.ParsableByteArray;
@@ -107,19 +107,8 @@ public final class FlacFrameReader {
     System.arraycopy(
     System.arraycopy(
         frameStartBytes, /* srcPos= */ 0, scratch.data, /* destPos= */ 0, /* length= */ 2);
         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);
     scratch.setLimit(totalBytesPeeked);
 
 
     input.resetPeekPosition();
     input.resetPeekPosition();
@@ -130,6 +119,46 @@ public final class FlacFrameReader {
   }
   }
 
 
   /**
   /**
+   * Returns the number of the first sample in the given frame.
+   *
+   * <p>The read position of {@code input} is left unchanged.
+   *
+   * <p>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.
    * Reads the given block size.
    *
    *
    * @param scratch The array to read the data from, whose position must correspond to the block
    * @param scratch The array to read the data from, whose position must correspond to the block

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.
    *     in the stream must start.
    * @param firstFramePosition The byte offset of the first frame in the stream.
    * @param firstFramePosition The byte offset of the first frame in the stream.
    * @param inputLength The length of the stream in bytes.
    * @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(
   public FlacBinarySearchSeeker(
       FlacStreamMetadata flacStreamMetadata,
       FlacStreamMetadata flacStreamMetadata,
       int frameStartMarker,
       int frameStartMarker,
       long firstFramePosition,
       long firstFramePosition,
-      long inputLength,
-      SampleNumberHolder sampleNumberHolder) {
+      long inputLength) {
     super(
     super(
         /* seekTimestampConverter= */ flacStreamMetadata::getSampleNumber,
         /* seekTimestampConverter= */ flacStreamMetadata::getSampleNumber,
-        new FlacTimestampSeeker(flacStreamMetadata, frameStartMarker, sampleNumberHolder),
+        new FlacTimestampSeeker(flacStreamMetadata, frameStartMarker),
         flacStreamMetadata.getDurationUs(),
         flacStreamMetadata.getDurationUs(),
         /* floorTimePosition= */ 0,
         /* floorTimePosition= */ 0,
         /* ceilingTimePosition= */ flacStreamMetadata.totalSamples,
         /* ceilingTimePosition= */ flacStreamMetadata.totalSamples,
@@ -66,21 +63,16 @@ import java.io.IOException;
 
 
     private final FlacStreamMetadata flacStreamMetadata;
     private final FlacStreamMetadata flacStreamMetadata;
     private final int frameStartMarker;
     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.flacStreamMetadata = flacStreamMetadata;
       this.frameStartMarker = frameStartMarker;
       this.frameStartMarker = frameStartMarker;
-      this.foundFrameSampleNumberHolder = foundFrameSampleNumberHolder;
-      tempSampleNumberHolder = new SampleNumberHolder();
+      sampleNumberHolder = new SampleNumberHolder();
     }
     }
 
 
     @Override
     @Override
-    public TimestampSearchResult searchForTimestamp(ExtractorInput input, long targetSampleIndex)
+    public TimestampSearchResult searchForTimestamp(ExtractorInput input, long targetSampleNumber)
         throws IOException, InterruptedException {
         throws IOException, InterruptedException {
       long searchPosition = input.getPosition();
       long searchPosition = input.getPosition();
 
 
@@ -95,11 +87,10 @@ import java.io.IOException;
       long rightFrameFirstSampleNumber = findNextFrame(input);
       long rightFrameFirstSampleNumber = findNextFrame(input);
       long rightFramePosition = input.getPeekPosition();
       long rightFramePosition = input.getPeekPosition();
 
 
-      if (leftFrameFirstSampleNumber <= targetSampleIndex
-          && rightFrameFirstSampleNumber > targetSampleIndex) {
-        foundFrameSampleNumberHolder.sampleNumber = leftFrameFirstSampleNumber;
+      if (leftFrameFirstSampleNumber <= targetSampleNumber
+          && rightFrameFirstSampleNumber > targetSampleNumber) {
         return TimestampSearchResult.targetFoundResult(leftFramePosition);
         return TimestampSearchResult.targetFoundResult(leftFramePosition);
-      } else if (rightFrameFirstSampleNumber <= targetSampleIndex) {
+      } else if (rightFrameFirstSampleNumber <= targetSampleNumber) {
         return TimestampSearchResult.underestimatedResult(
         return TimestampSearchResult.underestimatedResult(
             rightFrameFirstSampleNumber, rightFramePosition);
             rightFrameFirstSampleNumber, rightFramePosition);
       } else {
       } else {
@@ -125,7 +116,7 @@ import java.io.IOException;
     private long findNextFrame(ExtractorInput input) throws IOException, InterruptedException {
     private long findNextFrame(ExtractorInput input) throws IOException, InterruptedException {
       while (input.getPeekPosition() < input.getLength() - FlacConstants.MIN_FRAME_HEADER_SIZE
       while (input.getPeekPosition() < input.getLength() - FlacConstants.MIN_FRAME_HEADER_SIZE
           && !FlacFrameReader.checkFrameHeaderFromPeek(
           && !FlacFrameReader.checkFrameHeaderFromPeek(
-              input, flacStreamMetadata, frameStartMarker, tempSampleNumberHolder)) {
+              input, flacStreamMetadata, frameStartMarker, sampleNumberHolder)) {
         input.advancePeekPosition(1);
         input.advancePeekPosition(1);
       }
       }
 
 
@@ -134,7 +125,7 @@ import java.io.IOException;
         return flacStreamMetadata.totalSamples;
         return flacStreamMetadata.totalSamples;
       }
       }
 
 
-      return tempSampleNumberHolder.sampleNumber;
+      return sampleNumberHolder.sampleNumber;
     }
     }
   }
   }
 }
 }

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;
       state = STATE_READ_ID3_METADATA;
       currentFrameFirstSampleNumber = 0;
       currentFrameFirstSampleNumber = 0;
     } else if (binarySearchSeeker != null) {
     } else if (binarySearchSeeker != null) {
+      currentFrameFirstSampleNumber = SAMPLE_NUMBER_UNKNOWN;
       binarySearchSeeker.setSeekTargetUs(timeUs);
       binarySearchSeeker.setSeekTargetUs(timeUs);
     }
     }
     currentFrameBytesWritten = 0;
     currentFrameBytesWritten = 0;
@@ -243,9 +244,14 @@ public final class FlacExtractor implements Extractor {
 
 
     // Handle pending seek if necessary.
     // Handle pending seek if necessary.
     if (binarySearchSeeker != null && binarySearchSeeker.isSeeking()) {
     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.
     // Copy more bytes into the scratch.
@@ -300,11 +306,7 @@ public final class FlacExtractor implements Extractor {
     }
     }
     binarySearchSeeker =
     binarySearchSeeker =
         new FlacBinarySearchSeeker(
         new FlacBinarySearchSeeker(
-            flacStreamMetadata,
-            frameStartMarker,
-            firstFramePosition,
-            streamLength,
-            sampleNumberHolder);
+            flacStreamMetadata, frameStartMarker, firstFramePosition, streamLength);
     return binarySearchSeeker.getSeekMap();
     return binarySearchSeeker.getSeekMap();
   }
   }
 
 

library/core/src/test/java/com/google/android/exoplayer2/extractor/ExtractorUtilTest.java

+/*
+ * 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);
+  }
+}