MP3 IndexSeeker: handle seek to non-yet-read frames

Issue: #6787
PiperOrigin-RevId: 291953855

RELEASENOTES.md

@@ -22,6 +22,11 @@
 * DRM: Add support for attaching DRM sessions to clear content in the demo app.
 * Downloads: Merge downloads in `SegmentDownloader` to improve overall download
   speed ([#5978](https://github.com/google/ExoPlayer/issues/5978)).
+* MP3: Add `IndexSeeker` for accurate seeks in VBR streams
+  ([#6787](https://github.com/google/ExoPlayer/issues/6787)).
+  This seeker is enabled by passing `FLAG_ENABLE_INDEX_SEEKING` to the
+  `Mp3Extractor`. It may require to scan a significant portion of the file for
+  seeking, which may be costly on large files.
 * MP4: Store the Android capture frame rate only in `Format.metadata`.
   `Format.frameRate` now stores the calculated frame rate.
 * Testing

library/extractor/src/main/java/com/google/android/exoplayer2/extractor/mp3/IndexSeeker.java

@@ -15,6 +15,7 @@
  */
 package com.google.android.exoplayer2.extractor.mp3;
 
+import androidx.annotation.VisibleForTesting;
 import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.extractor.SeekPoint;
 import com.google.android.exoplayer2.util.LongArray;
@@ -23,7 +24,8 @@ import com.google.android.exoplayer2.util.Util;
 /** MP3 seeker that builds a time-to-byte mapping as the stream is read. */
 /* package */ final class IndexSeeker implements Seeker {
 
-  private static final long MIN_TIME_BETWEEN_POINTS_US = C.MICROS_PER_SECOND / 10;
+  @VisibleForTesting
+  /* package */ static final long MIN_TIME_BETWEEN_POINTS_US = C.MICROS_PER_SECOND / 10;
 
   private final long durationUs;
   private final long dataEndPosition;
@@ -85,11 +87,21 @@ import com.google.android.exoplayer2.util.Util;
    * @param position The position corresponding to the seek point to add in bytes.
    */
   public void maybeAddSeekPoint(long timeUs, long position) {
-    long lastTimeUs = timesUs.get(timesUs.size() - 1);
+    if (isTimeUsInIndex(timeUs)) {
-    if (timeUs - lastTimeUs < MIN_TIME_BETWEEN_POINTS_US) {
       return;
     }
     timesUs.add(timeUs);
     positions.add(position);
   }
+
+  /**
+   * Returns whether {@code timeUs} (in microseconds) is included in the index.
+   *
+   * <p>A point is included in the index if it is equal to another point, between 2 points, or
+   * sufficiently close to the last point.
+   */
+  public boolean isTimeUsInIndex(long timeUs) {
+    long lastIndexedTimeUs = timesUs.get(timesUs.size() - 1);
+    return timeUs - lastIndexedTimeUs < MIN_TIME_BETWEEN_POINTS_US;
+  }
 }

library/extractor/src/main/java/com/google/android/exoplayer2/extractor/mp3/Mp3Extractor.java

@@ -21,6 +21,7 @@ import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.Format;
 import com.google.android.exoplayer2.ParserException;
 import com.google.android.exoplayer2.audio.MpegAudioUtil;
+import com.google.android.exoplayer2.extractor.DummyTrackOutput;
 import com.google.android.exoplayer2.extractor.Extractor;
 import com.google.android.exoplayer2.extractor.ExtractorInput;
 import com.google.android.exoplayer2.extractor.ExtractorOutput;
@@ -78,6 +79,15 @@ public final class Mp3Extractor implements Extractor {
   public static final int FLAG_ENABLE_CONSTANT_BITRATE_SEEKING = 1;
   /**
    * Flag to force index seeking, consisting in building a time-to-byte mapping as the file is read.
+   *
+   * <p>This seeker may require to scan a significant portion of the file to compute a seek point.
+   * Therefore, it should only be used if:
+   *
+   * <ul>
+   *   <li>the file is small, or
+   *   <li>the bitrate is variable (or the type of bitrate is unknown) and the seeking metadata
+   *       provided in the file is not precise enough (or is not present).
+   * </ul>
    */
   public static final int FLAG_ENABLE_INDEX_SEEKING = 1 << 1;
   /**
@@ -121,21 +131,27 @@ public final class Mp3Extractor implements Extractor {
   private final MpegAudioUtil.Header synchronizedHeader;
   private final GaplessInfoHolder gaplessInfoHolder;
   private final Id3Peeker id3Peeker;
+  private final TrackOutput skippingTrackOutput;
 
-  // Extractor outputs.
   private @MonotonicNonNull ExtractorOutput extractorOutput;
-  private @MonotonicNonNull TrackOutput trackOutput;
+  private @MonotonicNonNull TrackOutput realTrackOutput;
+  // currentTrackOutput is set to skippingTrackOutput or to realTrackOutput, depending if the data
+  // read must be sent to the output.
+  private @MonotonicNonNull TrackOutput currentTrackOutput;
 
   private int synchronizedHeaderData;
 
   @Nullable private Metadata metadata;
-  private @MonotonicNonNull Seeker seeker;
-  private boolean disableSeeking;
   private long basisTimeUs;
   private long samplesRead;
   private long firstSamplePosition;
   private int sampleBytesRemaining;
 
+  private @MonotonicNonNull Seeker seeker;
+  private boolean disableSeeking;
+  private boolean isSeekInProgress;
+  private long seekTimeUs;
+
   public Mp3Extractor() {
     this(0);
   }
@@ -160,6 +176,7 @@ public final class Mp3Extractor implements Extractor {
     gaplessInfoHolder = new GaplessInfoHolder();
     basisTimeUs = C.TIME_UNSET;
     id3Peeker = new Id3Peeker();
+    skippingTrackOutput = new DummyTrackOutput();
   }
 
   // Extractor implementation.
@@ -172,7 +189,8 @@ public final class Mp3Extractor implements Extractor {
   @Override
   public void init(ExtractorOutput output) {
     extractorOutput = output;
-    trackOutput = extractorOutput.track(0, C.TRACK_TYPE_AUDIO);
+    realTrackOutput = extractorOutput.track(0, C.TRACK_TYPE_AUDIO);
+    currentTrackOutput = realTrackOutput;
     extractorOutput.endTracks();
   }
 
@@ -182,6 +200,11 @@ public final class Mp3Extractor implements Extractor {
     basisTimeUs = C.TIME_UNSET;
     samplesRead = 0;
     sampleBytesRemaining = 0;
+    seekTimeUs = timeUs;
+    if (seeker instanceof IndexSeeker && !((IndexSeeker) seeker).isTimeUsInIndex(timeUs)) {
+      isSeekInProgress = true;
+      currentTrackOutput = skippingTrackOutput;
+    }
   }
 
   @Override
@@ -203,7 +226,7 @@ public final class Mp3Extractor implements Extractor {
     if (seeker == null) {
       seeker = computeSeeker(input);
       extractorOutput.seekMap(seeker);
-      trackOutput.format(
+      currentTrackOutput.format(
           Format.createAudioSampleFormat(
               /* id= */ null,
               synchronizedHeader.mimeType,
@@ -242,7 +265,7 @@ public final class Mp3Extractor implements Extractor {
 
   // Internal methods.
 
-  @RequiresNonNull({"trackOutput", "seeker"})
+  @RequiresNonNull({"currentTrackOutput", "realTrackOutput", "seeker"})
   private int readSample(ExtractorInput extractorInput) throws IOException, InterruptedException {
     if (sampleBytesRemaining == 0) {
       extractorInput.resetPeekPosition();
@@ -267,11 +290,20 @@ public final class Mp3Extractor implements Extractor {
         }
       }
       sampleBytesRemaining = synchronizedHeader.frameSize;
-      maybeAddSeekPointToIndexSeeker(
+      if (seeker instanceof IndexSeeker) {
-          computeTimeUs(samplesRead + synchronizedHeader.samplesPerFrame),
+        IndexSeeker indexSeeker = (IndexSeeker) seeker;
-          extractorInput.getPosition() + synchronizedHeader.frameSize);
+        // Add seek point corresponding to the next frame instead of the current one to be able to
+        // start writing to the realTrackOutput on time when a seek is in progress.
+        indexSeeker.maybeAddSeekPoint(
+            computeTimeUs(samplesRead + synchronizedHeader.samplesPerFrame),
+            extractorInput.getPosition() + synchronizedHeader.frameSize);
+        if (isSeekInProgress && indexSeeker.isTimeUsInIndex(seekTimeUs)) {
+          isSeekInProgress = false;
+          currentTrackOutput = realTrackOutput;
+        }
+      }
     }
-    int bytesAppended = trackOutput.sampleData(extractorInput, sampleBytesRemaining, true);
+    int bytesAppended = currentTrackOutput.sampleData(extractorInput, sampleBytesRemaining, true);
     if (bytesAppended == C.RESULT_END_OF_INPUT) {
       return RESULT_END_OF_INPUT;
     }
@@ -279,7 +311,7 @@ public final class Mp3Extractor implements Extractor {
     if (sampleBytesRemaining > 0) {
       return RESULT_CONTINUE;
     }
-    trackOutput.sampleMetadata(
+    currentTrackOutput.sampleMetadata(
         computeTimeUs(samplesRead), C.BUFFER_FLAG_KEY_FRAME, synchronizedHeader.frameSize, 0, null);
     samplesRead += synchronizedHeader.samplesPerFrame;
     sampleBytesRemaining = 0;
@@ -290,13 +322,6 @@ public final class Mp3Extractor implements Extractor {
     return basisTimeUs + samplesRead * C.MICROS_PER_SECOND / synchronizedHeader.sampleRate;
   }
 
-  private void maybeAddSeekPointToIndexSeeker(long timeUs, long position) {
-    if (!(seeker instanceof IndexSeeker)) {
-      return;
-    }
-    ((IndexSeeker) seeker).maybeAddSeekPoint(timeUs, position);
-  }
-
   private boolean synchronize(ExtractorInput input, boolean sniffing)
       throws IOException, InterruptedException {
     int validFrameCount = 0;
@@ -488,9 +513,10 @@ public final class Mp3Extractor implements Extractor {
     return new ConstantBitrateSeeker(input.getLength(), input.getPosition(), synchronizedHeader);
   }
 
-  @EnsuresNonNull({"extractorOutput", "trackOutput"})
+  @EnsuresNonNull({"extractorOutput", "currentTrackOutput", "realTrackOutput"})
   private void assertInitialized() {
-    Assertions.checkStateNotNull(trackOutput);
+    Assertions.checkStateNotNull(realTrackOutput);
+    Util.castNonNull(currentTrackOutput);
     Util.castNonNull(extractorOutput);
   }
 

library/extractor/src/test/java/com/google/android/exoplayer2/extractor/mp3/IndexSeekerTest.java

@@ -0,0 +1,187 @@
+/*
+ * Copyright (C) 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.extractor.mp3;
+
+import static com.google.android.exoplayer2.extractor.mp3.Mp3Extractor.FLAG_ENABLE_INDEX_SEEKING;
+import static com.google.common.truth.Truth.assertThat;
+
+import android.net.Uri;
+import androidx.test.core.app.ApplicationProvider;
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+import com.google.android.exoplayer2.C;
+import com.google.android.exoplayer2.extractor.SeekMap;
+import com.google.android.exoplayer2.testutil.FakeExtractorOutput;
+import com.google.android.exoplayer2.testutil.FakeTrackOutput;
+import com.google.android.exoplayer2.testutil.TestUtil;
+import com.google.android.exoplayer2.upstream.DefaultDataSource;
+import com.google.android.exoplayer2.upstream.DefaultDataSourceFactory;
+import com.google.android.exoplayer2.util.Util;
+import java.io.IOException;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/** Tests for {@link IndexSeeker}. */
+@RunWith(AndroidJUnit4.class)
+public class IndexSeekerTest {
+
+  private static final String TEST_FILE = "mp3/bear-vbr.mp3";
+
+  private Mp3Extractor extractor;
+  private FakeExtractorOutput extractorOutput;
+  private DefaultDataSource dataSource;
+
+  @Before
+  public void setUp() throws Exception {
+    extractor = new Mp3Extractor(FLAG_ENABLE_INDEX_SEEKING);
+    extractorOutput = new FakeExtractorOutput();
+    dataSource =
+        new DefaultDataSourceFactory(ApplicationProvider.getApplicationContext(), "UserAgent")
+            .createDataSource();
+  }
+
+  @Test
+  public void mp3ExtractorReads_returnSeekableSeekMap() throws Exception {
+    Uri fileUri = TestUtil.buildAssetUri(TEST_FILE);
+
+    SeekMap seekMap = TestUtil.extractSeekMap(extractor, extractorOutput, dataSource, fileUri);
+
+    assertThat(seekMap.getDurationUs()).isEqualTo(2_808_000);
+    assertThat(seekMap.isSeekable()).isTrue();
+  }
+
+  @Test
+  public void seeking_handlesSeekToZero() throws Exception {
+    String fileName = TEST_FILE;
+    Uri fileUri = TestUtil.buildAssetUri(fileName);
+    SeekMap seekMap = TestUtil.extractSeekMap(extractor, extractorOutput, dataSource, fileUri);
+    FakeTrackOutput trackOutput = extractorOutput.trackOutputs.get(0);
+
+    long targetSeekTimeUs = 0;
+    int extractedFrameIndex =
+        TestUtil.seekToTimeUs(
+            extractor, seekMap, targetSeekTimeUs, dataSource, trackOutput, fileUri);
+
+    assertThat(extractedFrameIndex).isNotEqualTo(C.INDEX_UNSET);
+    assertFirstFrameAfterSeekIsWithinMinDifference(
+        fileName, trackOutput, targetSeekTimeUs, extractedFrameIndex);
+    assertFirstFrameAfterSeekHasCorrectData(fileName, trackOutput, extractedFrameIndex);
+  }
+
+  @Test
+  public void seeking_handlesSeekToEof() throws Exception {
+    String fileName = TEST_FILE;
+    Uri fileUri = TestUtil.buildAssetUri(fileName);
+    SeekMap seekMap = TestUtil.extractSeekMap(extractor, extractorOutput, dataSource, fileUri);
+    FakeTrackOutput trackOutput = extractorOutput.trackOutputs.get(0);
+
+    long targetSeekTimeUs = seekMap.getDurationUs();
+    int extractedFrameIndex =
+        TestUtil.seekToTimeUs(
+            extractor, seekMap, targetSeekTimeUs, dataSource, trackOutput, fileUri);
+
+    assertThat(extractedFrameIndex).isNotEqualTo(C.INDEX_UNSET);
+    assertFirstFrameAfterSeekIsWithinMinDifference(
+        fileName, trackOutput, targetSeekTimeUs, extractedFrameIndex);
+    assertFirstFrameAfterSeekHasCorrectData(fileName, trackOutput, extractedFrameIndex);
+  }
+
+  @Test
+  public void seeking_handlesSeekingBackward() throws Exception {
+    String fileName = TEST_FILE;
+    Uri fileUri = TestUtil.buildAssetUri(fileName);
+    SeekMap seekMap = TestUtil.extractSeekMap(extractor, extractorOutput, dataSource, fileUri);
+    FakeTrackOutput trackOutput = extractorOutput.trackOutputs.get(0);
+
+    long firstSeekTimeUs = 1_234_000;
+    TestUtil.seekToTimeUs(extractor, seekMap, firstSeekTimeUs, dataSource, trackOutput, fileUri);
+    long targetSeekTimeUs = 987_000;
+    int extractedFrameIndex =
+        TestUtil.seekToTimeUs(
+            extractor, seekMap, targetSeekTimeUs, dataSource, trackOutput, fileUri);
+
+    assertThat(extractedFrameIndex).isNotEqualTo(C.INDEX_UNSET);
+    assertFirstFrameAfterSeekIsWithinMinDifference(
+        fileName, trackOutput, targetSeekTimeUs, extractedFrameIndex);
+    assertFirstFrameAfterSeekHasCorrectData(fileName, trackOutput, extractedFrameIndex);
+  }
+
+  @Test
+  public void seeking_handlesSeekingForward() throws Exception {
+    String fileName = TEST_FILE;
+    Uri fileUri = TestUtil.buildAssetUri(fileName);
+    SeekMap seekMap = TestUtil.extractSeekMap(extractor, extractorOutput, dataSource, fileUri);
+    FakeTrackOutput trackOutput = extractorOutput.trackOutputs.get(0);
+
+    long firstSeekTimeUs = 987_000;
+    TestUtil.seekToTimeUs(extractor, seekMap, firstSeekTimeUs, dataSource, trackOutput, fileUri);
+    long targetSeekTimeUs = 1_234_000;
+    int extractedFrameIndex =
+        TestUtil.seekToTimeUs(
+            extractor, seekMap, targetSeekTimeUs, dataSource, trackOutput, fileUri);
+
+    assertThat(extractedFrameIndex).isNotEqualTo(C.INDEX_UNSET);
+    assertFirstFrameAfterSeekIsWithinMinDifference(
+        fileName, trackOutput, targetSeekTimeUs, extractedFrameIndex);
+    assertFirstFrameAfterSeekHasCorrectData(fileName, trackOutput, extractedFrameIndex);
+  }
+
+  private static void assertFirstFrameAfterSeekIsWithinMinDifference(
+      String fileName,
+      FakeTrackOutput trackOutput,
+      long targetSeekTimeUs,
+      int firstFrameIndexAfterSeek)
+      throws IOException, InterruptedException {
+    FakeTrackOutput expectedTrackOutput = getExpectedTrackOutput(fileName);
+    int exactFrameIndex = getFrameIndex(expectedTrackOutput, targetSeekTimeUs);
+    long exactFrameTimeUs = expectedTrackOutput.getSampleTimeUs(exactFrameIndex);
+    long foundTimeUs = trackOutput.getSampleTimeUs(firstFrameIndexAfterSeek);
+
+    assertThat(exactFrameTimeUs - foundTimeUs).isAtMost(IndexSeeker.MIN_TIME_BETWEEN_POINTS_US);
+  }
+
+  private static void assertFirstFrameAfterSeekHasCorrectData(
+      String fileName, FakeTrackOutput trackOutput, int firstFrameIndexAfterSeek)
+      throws IOException, InterruptedException {
+    FakeTrackOutput expectedTrackOutput = getExpectedTrackOutput(fileName);
+    long foundTimeUs = trackOutput.getSampleTimeUs(firstFrameIndexAfterSeek);
+    int foundFrameIndex = getFrameIndex(expectedTrackOutput, foundTimeUs);
+
+    trackOutput.assertSample(
+        firstFrameIndexAfterSeek,
+        expectedTrackOutput.getSampleData(foundFrameIndex),
+        expectedTrackOutput.getSampleTimeUs(foundFrameIndex),
+        expectedTrackOutput.getSampleFlags(foundFrameIndex),
+        expectedTrackOutput.getSampleCryptoData(foundFrameIndex));
+  }
+
+  private static FakeTrackOutput getExpectedTrackOutput(String fileName)
+      throws IOException, InterruptedException {
+    return TestUtil.extractAllSamplesFromFile(
+            new Mp3Extractor(FLAG_ENABLE_INDEX_SEEKING),
+            ApplicationProvider.getApplicationContext(),
+            fileName)
+        .trackOutputs
+        .get(0);
+  }
+
+  private static int getFrameIndex(FakeTrackOutput trackOutput, long targetSeekTimeUs) {
+    List<Long> frameTimes = trackOutput.getSampleTimesUs();
+    return Util.binarySearchFloor(
+        frameTimes, targetSeekTimeUs, /* inclusive= */ true, /* stayInBounds= */ false);
+  }
+}