From 683a5b8403966eac75b169e7aa2bafd0d7d3e4a3 Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Thu, 24 Oct 2024 16:27:39 +0100 Subject: [PATCH] Fix review comments --- .../analytics/AnalyticsListener.java | 2 +- .../analytics/DefaultAnalyticsCollector.java | 9 ++-- .../source/MediaSourceEventListener.java | 44 +++++++++++++++++++ .../source/ProgressiveMediaPeriod.java | 2 +- .../source/SingleSampleMediaPeriod.java | 2 +- .../ads/ServerSideAdInsertionMediaSource.java | 2 +- .../source/chunk/ChunkSampleStream.java | 2 +- .../DefaultAnalyticsCollectorTest.java | 7 ++- .../exoplayer/dash/DashMediaSource.java | 2 +- .../exoplayer/hls/HlsSampleStreamWrapper.java | 2 +- .../playlist/DefaultHlsPlaylistTracker.java | 2 +- .../smoothstreaming/SsMediaSource.java | 13 ++---- 12 files changed, 63 insertions(+), 26 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/AnalyticsListener.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/AnalyticsListener.java index 83785bfa05..e885057918 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/AnalyticsListener.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/AnalyticsListener.java @@ -845,7 +845,7 @@ public interface AnalyticsListener { /** * @deprecated Implement {@link #onLoadStarted(EventTime, LoadEventInfo, MediaLoadData, int)} - * instead. + * instead, and check for {@code retryCount == 0} for equivalent behavior. */ @UnstableApi @Deprecated diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/DefaultAnalyticsCollector.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/DefaultAnalyticsCollector.java index 8b3bee79f3..c12e798f54 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/DefaultAnalyticsCollector.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/DefaultAnalyticsCollector.java @@ -407,14 +407,13 @@ public class DefaultAnalyticsCollector implements AnalyticsCollector { MediaLoadData mediaLoadData, int retryCount) { EventTime eventTime = generateMediaPeriodEventTime(windowIndex, mediaPeriodId); - sendEvent( - eventTime, - /* eventFlag= */ C.INDEX_UNSET, - listener -> listener.onLoadStarted(eventTime, loadEventInfo, mediaLoadData)); sendEvent( eventTime, AnalyticsListener.EVENT_LOAD_STARTED, - listener -> listener.onLoadStarted(eventTime, loadEventInfo, mediaLoadData, retryCount)); + listener -> { + listener.onLoadStarted(eventTime, loadEventInfo, mediaLoadData); + listener.onLoadStarted(eventTime, loadEventInfo, mediaLoadData, retryCount); + }); } @Override diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/MediaSourceEventListener.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/MediaSourceEventListener.java index d61cabc12f..4a15005e0a 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/MediaSourceEventListener.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/MediaSourceEventListener.java @@ -223,6 +223,15 @@ public interface MediaSourceEventListener { } } + /** + * @deprecated Use {link {@link #loadStarted(LoadEventInfo, int, int)} instead to pass {@code + * retryCount}. + */ + @Deprecated + public void loadStarted(LoadEventInfo loadEventInfo, @DataType int dataType) { + loadStarted(loadEventInfo, dataType, /* retryCount= */ 0); + } + /** Dispatches {@link #onLoadStarted(int, MediaPeriodId, LoadEventInfo, MediaLoadData, int)}. */ public void loadStarted(LoadEventInfo loadEventInfo, @DataType int dataType, int retryCount) { loadStarted( @@ -237,6 +246,32 @@ public interface MediaSourceEventListener { retryCount); } + /** + * @deprecated Use {link {@link #loadStarted(LoadEventInfo, int, int, Format, int, Object, long, + * long, int)} )} instead to pass {@code retryCount}. + */ + @Deprecated + public void loadStarted( + LoadEventInfo loadEventInfo, + @DataType int dataType, + @C.TrackType int trackType, + @Nullable Format trackFormat, + @C.SelectionReason int trackSelectionReason, + @Nullable Object trackSelectionData, + long mediaStartTimeUs, + long mediaEndTimeUs) { + loadStarted( + loadEventInfo, + new MediaLoadData( + dataType, + trackType, + trackFormat, + trackSelectionReason, + trackSelectionData, + usToMs(mediaStartTimeUs), + usToMs(mediaEndTimeUs))); + } + /** Dispatches {@link #onLoadStarted(int, MediaPeriodId, LoadEventInfo, MediaLoadData, int)}. */ public void loadStarted( LoadEventInfo loadEventInfo, @@ -261,6 +296,15 @@ public interface MediaSourceEventListener { retryCount); } + /** + * @deprecated Use {link {@link #loadStarted(LoadEventInfo, MediaLoadData, int)} instead to pass + * {@code retryCount}. + */ + @Deprecated + public void loadStarted(LoadEventInfo loadEventInfo, MediaLoadData mediaLoadData) { + loadStarted(loadEventInfo, mediaLoadData, /* retryCount= */ 0); + } + /** Dispatches {@link #onLoadStarted(int, MediaPeriodId, LoadEventInfo, MediaLoadData, int)}. */ public void loadStarted( LoadEventInfo loadEventInfo, MediaLoadData mediaLoadData, int retryCount) { diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ProgressiveMediaPeriod.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ProgressiveMediaPeriod.java index a8954050d7..5c525f9798 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ProgressiveMediaPeriod.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ProgressiveMediaPeriod.java @@ -609,7 +609,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; dataSource.getLastResponseHeaders(), elapsedRealtimeMs, loadDurationMs, - /* bytesLoaded= */ 0); + dataSource.getBytesRead()); mediaSourceEventDispatcher.loadStarted( loadEventInfo, C.DATA_TYPE_MEDIA, diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/SingleSampleMediaPeriod.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/SingleSampleMediaPeriod.java index 79e7b7151e..651e296078 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/SingleSampleMediaPeriod.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/SingleSampleMediaPeriod.java @@ -209,7 +209,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; dataSource.getLastResponseHeaders(), elapsedRealtimeMs, loadDurationMs, - /* bytesLoaded= */ 0); + dataSource.getBytesRead()); eventDispatcher.loadStarted( loadEventInfo, C.DATA_TYPE_MEDIA, diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSource.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSource.java index 1b1d96b2f6..f37a86bdfa 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSource.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSource.java @@ -429,7 +429,7 @@ public final class ServerSideAdInsertionMediaSource extends BaseMediaSource LoadEventInfo loadEventInfo, MediaLoadData mediaLoadData, int retryCount) { - // TODO file a bug to track updating this. + // TODO: b/375408535 - Update this to support non-zero retry counts. if (retryCount == 0) { @Nullable MediaPeriodImpl mediaPeriod = diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/chunk/ChunkSampleStream.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/chunk/ChunkSampleStream.java index 6c897a91df..006f63911b 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/chunk/ChunkSampleStream.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/chunk/ChunkSampleStream.java @@ -440,7 +440,7 @@ public class ChunkSampleStream loadable.getResponseHeaders(), elapsedRealtimeMs, loadDurationMs, - /* bytesLoaded= */ 0), + loadable.dataSource.getBytesRead()), loadable.type, primaryTrackType, loadable.trackFormat, diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultAnalyticsCollectorTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultAnalyticsCollectorTest.java index 1c38f410bd..916bb1421e 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultAnalyticsCollectorTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultAnalyticsCollectorTest.java @@ -146,10 +146,9 @@ import org.mockito.InOrder; public final class DefaultAnalyticsCollectorTest { // Deprecated event constants. - private static final long EVENT_PLAYER_STATE_CHANGED = 1L << 63; - private static final long EVENT_SEEK_STARTED = 1L << 62; - // Start from +1 of the MIN because it will collide with 1L << 63 - private static final long DEPRECATED_EVENT_LOAD_STARTED = Long.MIN_VALUE + 1; + private static final long EVENT_PLAYER_STATE_CHANGED = Long.MIN_VALUE; + private static final long EVENT_SEEK_STARTED = Long.MIN_VALUE + 1; + private static final long DEPRECATED_EVENT_LOAD_STARTED = Long.MIN_VALUE + 2; private static final UUID DRM_SCHEME_UUID = UUID.nameUUIDFromBytes(TestUtil.createByteArray(7, 8, 9)); diff --git a/libraries/exoplayer_dash/src/main/java/androidx/media3/exoplayer/dash/DashMediaSource.java b/libraries/exoplayer_dash/src/main/java/androidx/media3/exoplayer/dash/DashMediaSource.java index de8686bb19..3bbbf9929c 100644 --- a/libraries/exoplayer_dash/src/main/java/androidx/media3/exoplayer/dash/DashMediaSource.java +++ b/libraries/exoplayer_dash/src/main/java/androidx/media3/exoplayer/dash/DashMediaSource.java @@ -642,7 +642,7 @@ public final class DashMediaSource extends BaseMediaSource { loadable.getResponseHeaders(), elapsedRealtimeMs, loadDurationMs, - /* bytesLoaded= */ 0), + loadable.bytesLoaded()), loadable.type, retryCount); } diff --git a/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsSampleStreamWrapper.java b/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsSampleStreamWrapper.java index 9863ced91f..60b0333983 100644 --- a/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsSampleStreamWrapper.java +++ b/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsSampleStreamWrapper.java @@ -861,7 +861,7 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull; loadable.getResponseHeaders(), elapsedRealtimeMs, loadDurationMs, - /* bytesLoaded= */ 0), + loadable.bytesLoaded()), loadable.type, trackType, loadable.trackFormat, diff --git a/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/DefaultHlsPlaylistTracker.java b/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/DefaultHlsPlaylistTracker.java index 51c6d04e5e..a36b300bca 100644 --- a/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/DefaultHlsPlaylistTracker.java +++ b/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/DefaultHlsPlaylistTracker.java @@ -260,7 +260,7 @@ public final class DefaultHlsPlaylistTracker loadable.getResponseHeaders(), elapsedRealtimeMs, loadDurationMs, - /* bytesLoaded= */ 0), + loadable.bytesLoaded()), loadable.type, retryCount); } diff --git a/libraries/exoplayer_smoothstreaming/src/main/java/androidx/media3/exoplayer/smoothstreaming/SsMediaSource.java b/libraries/exoplayer_smoothstreaming/src/main/java/androidx/media3/exoplayer/smoothstreaming/SsMediaSource.java index a8e9498740..62a581c7d8 100644 --- a/libraries/exoplayer_smoothstreaming/src/main/java/androidx/media3/exoplayer/smoothstreaming/SsMediaSource.java +++ b/libraries/exoplayer_smoothstreaming/src/main/java/androidx/media3/exoplayer/smoothstreaming/SsMediaSource.java @@ -511,9 +511,9 @@ public final class SsMediaSource extends BaseMediaSource loadable.getResponseHeaders(), elapsedRealtimeMs, loadDurationMs, - /* bytesLoaded= */ 0), + loadable.bytesLoaded()), loadable.type, - /* retryCount= */ 0); + retryCount); } @Override @@ -678,12 +678,7 @@ public final class SsMediaSource extends BaseMediaSource ParsingLoadable loadable = new ParsingLoadable<>( manifestDataSource, manifestUri, C.DATA_TYPE_MANIFEST, manifestParser); - long elapsedRealtimeMs = - manifestLoader.startLoading( - loadable, this, loadErrorHandlingPolicy.getMinimumLoadableRetryCount(loadable.type)); - manifestEventDispatcher.loadStarted( - new LoadEventInfo(loadable.loadTaskId, loadable.dataSpec, elapsedRealtimeMs), - loadable.type, - /* retryCount= */ 0); + manifestLoader.startLoading( + loadable, this, loadErrorHandlingPolicy.getMinimumLoadableRetryCount(loadable.type)); } }