From 6236fd38d03121a1301630e2a0023c8e2d1e8b41 Mon Sep 17 00:00:00 2001 From: rohks Date: Thu, 14 Dec 2023 17:37:16 -0800 Subject: [PATCH] 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/media#888 #minor-release PiperOrigin-RevId: 591099785 (cherry picked from commit 7f6596bab217847617947b61ded64b7aba58a194) --- RELEASENOTES.md | 4 ++++ .../media3/exoplayer/upstream/CmcdData.java | 3 ++- .../exoplayer/dash/DefaultDashChunkSource.java | 2 +- .../dash/DefaultDashChunkSourceTest.java | 17 +++++++++++++++++ .../media3/exoplayer/hls/HlsChunkSource.java | 2 +- .../exoplayer/hls/HlsChunkSourceTest.java | 18 ++++++++++++++++++ .../smoothstreaming/DefaultSsChunkSource.java | 3 ++- .../DefaultSsChunkSourceTest.java | 17 +++++++++++++++++ 8 files changed, 62 insertions(+), 4 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 6c719e1596..4b834093de 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -13,6 +13,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: * Work around an issue where the encoder would throw at configuration time due to setting a high operating rate. 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 index 35f3a1b53a..d84c592522 100644 --- 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 final class CmcdData { * 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/libraries/exoplayer_dash/src/main/java/androidx/media3/exoplayer/dash/DefaultDashChunkSource.java b/libraries/exoplayer_dash/src/main/java/androidx/media3/exoplayer/dash/DefaultDashChunkSource.java index 2873be5093..429e8fc5e2 100644 --- a/libraries/exoplayer_dash/src/main/java/androidx/media3/exoplayer/dash/DefaultDashChunkSource.java +++ b/libraries/exoplayer_dash/src/main/java/androidx/media3/exoplayer/dash/DefaultDashChunkSource.java @@ -410,7 +410,7 @@ public class DefaultDashChunkSource implements DashChunkSource { : new CmcdData.Factory( cmcdConfiguration, trackSelection, - bufferedDurationUs, + max(0, bufferedDurationUs), /* playbackRate= */ loadingInfo.playbackSpeed, /* streamingFormat= */ CmcdData.Factory.STREAMING_FORMAT_DASH, /* isLive= */ manifest.dynamic, diff --git a/libraries/exoplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultDashChunkSourceTest.java b/libraries/exoplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultDashChunkSourceTest.java index 46dcd4891f..7de944c718 100644 --- a/libraries/exoplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultDashChunkSourceTest.java +++ b/libraries/exoplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultDashChunkSourceTest.java @@ -339,6 +339,23 @@ public class DefaultDashChunkSourceTest { "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 index 56ead292c1..9d7eb774e6 100644 --- 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 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; 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 index 9d866b0ec4..a038217103 100644 --- 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 class HlsChunkSourceTest { "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/libraries/exoplayer_smoothstreaming/src/main/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSource.java b/libraries/exoplayer_smoothstreaming/src/main/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSource.java index 1ae37c5495..e350ab64aa 100644 --- a/libraries/exoplayer_smoothstreaming/src/main/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSource.java +++ b/libraries/exoplayer_smoothstreaming/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; @@ -296,7 +297,7 @@ public class DefaultSsChunkSource implements SsChunkSource { new CmcdData.Factory( cmcdConfiguration, trackSelection, - bufferedDurationUs, + max(0, bufferedDurationUs), /* playbackRate= */ loadingInfo.playbackSpeed, /* streamingFormat= */ CmcdData.Factory.STREAMING_FORMAT_SS, /* isLive= */ manifest.isLive, diff --git a/libraries/exoplayer_smoothstreaming/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSourceTest.java b/libraries/exoplayer_smoothstreaming/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSourceTest.java index 70ada1a5d2..c384cd80c7 100644 --- a/libraries/exoplayer_smoothstreaming/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSourceTest.java +++ b/libraries/exoplayer_smoothstreaming/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSourceTest.java @@ -89,6 +89,23 @@ public class DefaultSsChunkSourceTest { "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