Fix sending negative bufferedDurationUs to CmcdData.Factory

When track is changed during playback, `playbackPositionUs` may be in middle of a chunk and `loadPositionUs` should be the start of that chunk. In this situation `loadPositionUs` can be less than the current `playbackPositionUs`, resulting into negative `bufferedDurationUs`. It translates to having no buffer and hence we should send `0` for `bufferedDurationUs` when creating new instances of `CmcdData.Factory`. Issue: androidx#888 #minor-release PiperOrigin-RevId: 591099785
SimonHung · Dec 15, 2023 · 7f6596b · 7f6596b
1 parent 0baf777
commit 7f6596b
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 4 deletions.
diff --git a/RELEASENOTES.md b/RELEASENOTES.md
@@ -36,6 +36,10 @@
     *   Fix issue where track selections after seek to zero in a live stream
         incorrectly let the stream start at its default position
         ([#9347](https://github.com/google/ExoPlayer/issues/9347)).
+    *   Fix the issue where new instances of `CmcdData.Factory` were receiving
+        negative values for `bufferedDurationUs` from chunk sources, resulting
+        in an `IllegalArgumentException`
+        ([#888](https://github.com/androidx/media/issues/888)).
 *   Transformer:
     *   Add support for flattening H.265/HEVC SEF slow motion videos.
     *   Increase transmuxing speed, especially for 'remove video' edits.

diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/upstream/CmcdData.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/upstream/CmcdData.java
@@ -121,7 +121,8 @@ public static final class Factory {
      *     and this one, {@code false} otherwise.
      * @param isBufferEmpty {@code true} if the queue of buffered chunks is empty, {@code false}
      *     otherwise.
-     * @throws IllegalArgumentException If {@code bufferedDurationUs} is negative.
+     * @throws IllegalArgumentException If {@code bufferedDurationUs} is negative or {@code
+     *     playbackRate} is non-positive.
      */
     public Factory(
         CmcdConfiguration cmcdConfiguration,

diff --git a/...s/exoplayer_dash/src/main/java/androidx/media3/exoplayer/dash/DefaultDashChunkSource.java b/...s/exoplayer_dash/src/main/java/androidx/media3/exoplayer/dash/DefaultDashChunkSource.java
@@ -410,7 +410,7 @@ public void getNextChunk(
             : new CmcdData.Factory(
                 cmcdConfiguration,
                 trackSelection,
-                bufferedDurationUs,
+                max(0, bufferedDurationUs),
                 /* playbackRate= */ loadingInfo.playbackSpeed,
                 /* streamingFormat= */ CmcdData.Factory.STREAMING_FORMAT_DASH,
                 /* isLive= */ manifest.dynamic,

diff --git a/...oplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultDashChunkSourceTest.java b/...oplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultDashChunkSourceTest.java
@@ -339,6 +339,23 @@ public void getNextChunk_chunkSourceWithDefaultCmcdConfiguration_setsCmcdHttpReq
             "bl=1000,dl=800,mtp=1000,nor=\"..%2Fvideo_8000_700000.m4s\",nrr=\"0-\"",
             "CMCD-Session",
             "cid=\"mediaId\",pr=1.25,sf=d,sid=\"" + cmcdConfiguration.sessionId + "\",st=v");
+
+    // Playing mid-chunk, where loadPositionUs is less than playbackPositionUs
+    chunkSource.getNextChunk(
+        new LoadingInfo.Builder().setPlaybackPositionUs(5_000_000).setPlaybackSpeed(1.25f).build(),
+        /* loadPositionUs= */ 4_000_000,
+        /* queue= */ ImmutableList.of((MediaChunk) output.chunk),
+        output);
+
+    // buffer length is set to 0 when bufferedDurationUs is negative
+    assertThat(output.chunk.dataSpec.httpRequestHeaders)
+        .containsExactly(
+            "CMCD-Object",
+            "br=700,d=4000,ot=v,tb=1300",
+            "CMCD-Request",
+            "bl=0,dl=0,mtp=1000,nor=\"..%2Fvideo_12000_700000.m4s\",nrr=\"0-\"",
+            "CMCD-Session",
+            "cid=\"mediaId\",pr=1.25,sf=d,sid=\"" + cmcdConfiguration.sessionId + "\",st=v");
   }
 
   @Test

diff --git a/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsChunkSource.java b/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsChunkSource.java
@@ -495,7 +495,7 @@ public void getNextChunk(
           new CmcdData.Factory(
                   cmcdConfiguration,
                   trackSelection,
-                  bufferedDurationUs,
+                  max(0, bufferedDurationUs),
                   /* playbackRate= */ loadingInfo.playbackSpeed,
                   /* streamingFormat= */ CmcdData.Factory.STREAMING_FORMAT_HLS,
                   /* isLive= */ !playlist.hasEndTag,

diff --git a/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/HlsChunkSourceTest.java b/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/HlsChunkSourceTest.java
@@ -234,6 +234,24 @@ public void getNextChunk_chunkSourceWithDefaultCmcdConfiguration_setsCmcdHttpReq
             "bl=1000,dl=800,nor=\"..%2F3.mp4\",nrr=\"0-\"",
             "CMCD-Session",
             "cid=\"mediaId\",pr=1.25,sf=h,sid=\"" + cmcdConfiguration.sessionId + "\",st=v");
+
+    // Playing mid-chunk, where loadPositionUs is less than playbackPositionUs
+    testChunkSource.getNextChunk(
+        new LoadingInfo.Builder().setPlaybackPositionUs(5_000_000).setPlaybackSpeed(1.25f).build(),
+        /* loadPositionUs= */ 4_000_000,
+        /* queue= */ ImmutableList.of((HlsMediaChunk) output.chunk),
+        /* allowEndOfStream= */ true,
+        output);
+
+    // buffer length is set to 0 when bufferedDurationUs is negative
+    assertThat(output.chunk.dataSpec.httpRequestHeaders)
+        .containsExactly(
+            "CMCD-Object",
+            "br=800,d=4000,ot=v,tb=800",
+            "CMCD-Request",
+            "bl=0,dl=0,nor=\"..%2F3.mp4\",nrr=\"0-\"",
+            "CMCD-Session",
+            "cid=\"mediaId\",pr=1.25,sf=h,sid=\"" + cmcdConfiguration.sessionId + "\",st=v");
   }
 
   @Test

diff --git a/...reaming/src/main/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSource.java b/...reaming/src/main/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSource.java
@@ -16,6 +16,7 @@
 package androidx.media3.exoplayer.smoothstreaming;
 
 import static androidx.media3.exoplayer.trackselection.TrackSelectionUtil.createFallbackOptions;
+import static java.lang.Math.max;
 
 import android.net.Uri;
 import android.os.SystemClock;
@@ -319,7 +320,7 @@ public final void getNextChunk(
           new CmcdData.Factory(
                   cmcdConfiguration,
                   trackSelection,
-                  bufferedDurationUs,
+                  max(0, bufferedDurationUs),
                   /* playbackRate= */ loadingInfo.playbackSpeed,
                   /* streamingFormat= */ CmcdData.Factory.STREAMING_FORMAT_SS,
                   /* isLive= */ manifest.isLive,

diff --git a/...ing/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSourceTest.java b/...ing/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSourceTest.java
@@ -89,6 +89,23 @@ public void getNextChunk_chunkSourceWithDefaultCmcdConfiguration_setsCmcdHttpReq
             "bl=1000,dl=500,mtp=1000,nor=\"..%2FFragments(video%3D28660000)\"",
             "CMCD-Session",
             "cid=\"mediaId\",pr=2.00,sf=s,sid=\"" + cmcdConfiguration.sessionId + "\",st=v");
+
+    // Playing mid-chunk, where loadPositionUs is less than playbackPositionUs
+    chunkSource.getNextChunk(
+        new LoadingInfo.Builder().setPlaybackPositionUs(5_000_000).setPlaybackSpeed(2.0f).build(),
+        /* loadPositionUs= */ 4_000_000,
+        /* queue= */ ImmutableList.of((MediaChunk) output.chunk),
+        output);
+
+    // buffer length is set to 0 when bufferedDurationUs is negative
+    assertThat(output.chunk.dataSpec.httpRequestHeaders)
+        .containsExactly(
+            "CMCD-Object",
+            "br=308,d=898,ot=v,tb=1536",
+            "CMCD-Request",
+            "bl=0,dl=0,mtp=1000",
+            "CMCD-Session",
+            "cid=\"mediaId\",pr=2.00,sf=s,sid=\"" + cmcdConfiguration.sessionId + "\",st=v");
   }
 
   @Test