From 841e27ae5cd9427b13ba543eb60be016477aa5b2 Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 14 Feb 2025 04:30:47 -0800 Subject: [PATCH] Move MediaMetricsListener creation and reporting off main thread The creation can be moved to the playback thread (to guarantee it happens in sync other initialization after playback start) and the potentially blocking calls to the reporting methods can be moved to the generic shared BackgroundExecutor (it can't use the playback thread because it no longer exists when the session is ended after the player is released). PiperOrigin-RevId: 726872818 (cherry picked from commit d386e002d2b34817178d088f277ced3bf3943ef2) --- .../media3/exoplayer/ExoPlayerImpl.java | 41 ++++++++-------- .../analytics/MediaMetricsListener.java | 26 +++++++--- .../media3/exoplayer/analytics/PlayerId.java | 49 ++++++++----------- .../exoplayer/DefaultLoadControlTest.java | 15 ++---- 4 files changed, 64 insertions(+), 67 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java index b2a36d06f7..94da63f41b 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java @@ -49,7 +49,6 @@ import android.media.AudioDeviceCallback; import android.media.AudioDeviceInfo; import android.media.AudioManager; import android.media.MediaFormat; -import android.media.metrics.LogSessionId; import android.os.Handler; import android.os.Looper; import android.util.Pair; @@ -354,14 +353,7 @@ import java.util.concurrent.CopyOnWriteArraySet; playbackInfoUpdateHandler.post(() -> handlePlaybackInfo(playbackInfoUpdate)); playbackInfo = PlaybackInfo.createDummy(emptyTrackSelectorResult); analyticsCollector.setPlayer(this.wrappingPlayer, applicationLooper); - PlayerId playerId = - Util.SDK_INT < 31 - ? new PlayerId(builder.playerName) - : Api31.registerMediaMetricsListener( - applicationContext, - /* player= */ this, - builder.usePlatformDiagnostics, - builder.playerName); + PlayerId playerId = new PlayerId(builder.playerName); internalPlayer = new ExoPlayerImplInternal( renderers, @@ -401,6 +393,10 @@ import java.util.concurrent.CopyOnWriteArraySet; if (builder.foregroundModeTimeoutMs > 0) { internalPlayer.experimentalSetForegroundModeTimeoutMs(builder.foregroundModeTimeoutMs); } + if (Util.SDK_INT >= 31) { + Api31.registerMediaMetricsListener( + applicationContext, /* player= */ this, builder.usePlatformDiagnostics, playerId); + } audioSessionIdState = new BackgroundThreadStateHandler<>( @@ -3383,17 +3379,22 @@ import java.util.concurrent.CopyOnWriteArraySet; private static final class Api31 { private Api31() {} - public static PlayerId registerMediaMetricsListener( - Context context, ExoPlayerImpl player, boolean usePlatformDiagnostics, String playerName) { - @Nullable MediaMetricsListener listener = MediaMetricsListener.create(context); - if (listener == null) { - Log.w(TAG, "MediaMetricsService unavailable."); - return new PlayerId(LogSessionId.LOG_SESSION_ID_NONE, playerName); - } - if (usePlatformDiagnostics) { - player.addAnalyticsListener(listener); - } - return new PlayerId(listener.getLogSessionId(), playerName); + public static void registerMediaMetricsListener( + Context context, ExoPlayerImpl player, boolean usePlatformDiagnostics, PlayerId playerId) { + HandlerWrapper playbackThreadHandler = + player.getClock().createHandler(player.getPlaybackLooper(), /* callback= */ null); + playbackThreadHandler.post( + () -> { + @Nullable MediaMetricsListener listener = MediaMetricsListener.create(context); + if (listener == null) { + Log.w(TAG, "MediaMetricsService unavailable."); + return; + } + if (usePlatformDiagnostics) { + player.addAnalyticsListener(listener); + } + playerId.setLogSessionId(listener.getLogSessionId()); + }); } } diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/MediaMetricsListener.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/MediaMetricsListener.java index 89af53033c..11c0f6edf8 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/MediaMetricsListener.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/MediaMetricsListener.java @@ -51,6 +51,7 @@ import androidx.media3.common.Player; import androidx.media3.common.Timeline; import androidx.media3.common.Tracks; import androidx.media3.common.VideoSize; +import androidx.media3.common.util.BackgroundExecutor; import androidx.media3.common.util.NetworkTypeObserver; import androidx.media3.common.util.NullableType; import androidx.media3.common.util.UnstableApi; @@ -76,6 +77,7 @@ import java.net.SocketTimeoutException; import java.net.UnknownHostException; import java.util.HashMap; import java.util.UUID; +import java.util.concurrent.Executor; import org.checkerframework.checker.nullness.qual.EnsuresNonNullIf; import org.checkerframework.checker.nullness.qual.RequiresNonNull; @@ -108,6 +110,7 @@ public final class MediaMetricsListener } private final Context context; + private final Executor backgroundExecutor; private final PlaybackSessionManager sessionManager; private final PlaybackSession playbackSession; private final long startTimeMs; @@ -145,6 +148,7 @@ public final class MediaMetricsListener context = context.getApplicationContext(); this.context = context; this.playbackSession = playbackSession; + backgroundExecutor = BackgroundExecutor.get(); window = new Timeline.Window(); period = new Timeline.Period(); bandwidthBytes = new HashMap<>(); @@ -357,13 +361,14 @@ public final class MediaMetricsListener ErrorInfo errorInfo = getErrorInfo( error, context, /* lastIoErrorForManifest= */ ioErrorType == C.DATA_TYPE_MANIFEST); - playbackSession.reportPlaybackErrorEvent( + PlaybackErrorEvent playbackErrorEvent = new PlaybackErrorEvent.Builder() .setTimeSinceCreatedMillis(realtimeMs - startTimeMs) .setErrorCode(errorInfo.errorCode) .setSubErrorCode(errorInfo.subErrorCode) .setException(error) - .build()); + .build(); + backgroundExecutor.execute(() -> playbackSession.reportPlaybackErrorEvent(playbackErrorEvent)); reportedEventsForCurrentSession = true; pendingPlayerError = null; } @@ -415,11 +420,12 @@ public final class MediaMetricsListener int networkType = getNetworkType(context); if (networkType != currentNetworkType) { currentNetworkType = networkType; - playbackSession.reportNetworkEvent( + NetworkEvent networkEvent = new NetworkEvent.Builder() .setNetworkType(networkType) .setTimeSinceCreatedMillis(realtimeMs - startTimeMs) - .build()); + .build(); + backgroundExecutor.execute(() -> playbackSession.reportNetworkEvent(networkEvent)); } } @@ -436,11 +442,13 @@ public final class MediaMetricsListener if (currentPlaybackState != newPlaybackState) { currentPlaybackState = newPlaybackState; reportedEventsForCurrentSession = true; - playbackSession.reportPlaybackStateEvent( + PlaybackStateEvent playbackStateEvent = new PlaybackStateEvent.Builder() .setState(currentPlaybackState) .setTimeSinceCreatedMillis(realtimeMs - startTimeMs) - .build()); + .build(); + backgroundExecutor.execute( + () -> playbackSession.reportPlaybackStateEvent(playbackStateEvent)); } } @@ -570,7 +578,8 @@ public final class MediaMetricsListener builder.setTrackState(TrackChangeEvent.TRACK_STATE_OFF); } reportedEventsForCurrentSession = true; - playbackSession.reportTrackChangeEvent(builder.build()); + TrackChangeEvent trackChangeEvent = builder.build(); + backgroundExecutor.execute(() -> playbackSession.reportTrackChangeEvent(trackChangeEvent)); } @RequiresNonNull("metricsBuilder") @@ -613,7 +622,8 @@ public final class MediaMetricsListener networkBytes != null && networkBytes > 0 ? PlaybackMetrics.STREAM_SOURCE_NETWORK : PlaybackMetrics.STREAM_SOURCE_UNKNOWN); - playbackSession.reportPlaybackMetrics(metricsBuilder.build()); + PlaybackMetrics playbackMetrics = metricsBuilder.build(); + backgroundExecutor.execute(() -> playbackSession.reportPlaybackMetrics(playbackMetrics)); } metricsBuilder = null; activeSessionId = null; diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/PlayerId.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/PlayerId.java index dd38c4c3a0..232b0a3fd2 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/PlayerId.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/PlayerId.java @@ -33,10 +33,7 @@ public final class PlayerId { /** * A player identifier with unset default values that can be used as a placeholder or for testing. */ - public static final PlayerId UNSET = - Util.SDK_INT < 31 - ? new PlayerId(/* playerName= */ "") - : new PlayerId(LogSessionIdApi31.UNSET, /* playerName= */ ""); + public static final PlayerId UNSET = new PlayerId(/* playerName= */ ""); /** * A name to identify the player. Use {@link Builder#setName(String)} to set the name, otherwise @@ -52,31 +49,13 @@ public final class PlayerId { @Nullable private final Object equalityToken; /** - * Creates an instance for API < 31. + * Creates an instance. * * @param playerName The name of the player, for informational purpose only. */ public PlayerId(String playerName) { - checkState(Util.SDK_INT < 31); - this.name = playerName; - this.logSessionIdApi31 = null; - equalityToken = new Object(); - } - - /** - * Creates an instance for API ≥ 31. - * - * @param logSessionId The {@link LogSessionId} used for this player. - * @param playerName The name of the player, for informational purpose only. - */ - @RequiresApi(31) - public PlayerId(LogSessionId logSessionId, String playerName) { - this(new LogSessionIdApi31(logSessionId), playerName); - } - - private PlayerId(LogSessionIdApi31 logSessionIdApi31, String playerName) { - this.logSessionIdApi31 = logSessionIdApi31; this.name = playerName; + this.logSessionIdApi31 = Util.SDK_INT >= 31 ? new LogSessionIdApi31() : null; equalityToken = new Object(); } @@ -101,19 +80,31 @@ public final class PlayerId { /** Returns the {@link LogSessionId} for this player instance. */ @RequiresApi(31) - public LogSessionId getLogSessionId() { + public synchronized LogSessionId getLogSessionId() { return checkNotNull(logSessionIdApi31).logSessionId; } + /** + * Set the {@link LogSessionId} for this player instance. + * + *

Must not be called if already set. + */ + @RequiresApi(31) + public synchronized void setLogSessionId(LogSessionId logSessionId) { + checkNotNull(logSessionIdApi31).setLogSessionId(logSessionId); + } + @RequiresApi(31) private static final class LogSessionIdApi31 { - public static final LogSessionIdApi31 UNSET = - new LogSessionIdApi31(LogSessionId.LOG_SESSION_ID_NONE); + public LogSessionId logSessionId; - public final LogSessionId logSessionId; + public LogSessionIdApi31() { + this.logSessionId = LogSessionId.LOG_SESSION_ID_NONE; + } - public LogSessionIdApi31(LogSessionId logSessionId) { + public void setLogSessionId(LogSessionId logSessionId) { + checkState(this.logSessionId.equals(LogSessionId.LOG_SESSION_ID_NONE)); this.logSessionId = logSessionId; } } diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/DefaultLoadControlTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/DefaultLoadControlTest.java index 3733a90d08..b0185a9f42 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/DefaultLoadControlTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/DefaultLoadControlTest.java @@ -17,7 +17,6 @@ package androidx.media3.exoplayer; import static com.google.common.truth.Truth.assertThat; -import android.media.metrics.LogSessionId; import androidx.media3.common.C; import androidx.media3.common.Format; import androidx.media3.common.MediaItem; @@ -59,11 +58,7 @@ public class DefaultLoadControlTest { public void setUp() throws Exception { builder = new Builder(); allocator = new DefaultAllocator(true, C.DEFAULT_BUFFER_SEGMENT_SIZE); - playerId = - Util.SDK_INT < 31 - ? new PlayerId(/* playerName= */ "") - : new PlayerId( - /* logSessionId= */ LogSessionId.LOG_SESSION_ID_NONE, /* playerName= */ ""); + playerId = new PlayerId(/* playerName= */ ""); timeline = new SinglePeriodTimeline( /* durationUs= */ 10_000_000L, @@ -130,7 +125,7 @@ public class DefaultLoadControlTest { /* bufferForPlaybackAfterRebufferMs= */ 0); build(); // A second player uses the load control. - PlayerId playerId2 = new PlayerId(LogSessionId.LOG_SESSION_ID_NONE, /* playerName= */ ""); + PlayerId playerId2 = new PlayerId(/* playerName= */ ""); Timeline timeline2 = new FakeTimeline(); MediaSource.MediaPeriodId mediaPeriodId2 = new MediaSource.MediaPeriodId( @@ -731,7 +726,7 @@ public class DefaultLoadControlTest { @Test public void onPrepared_updatesTargetBufferBytes_correctDefaultTargetBufferSize() { - PlayerId playerId2 = new PlayerId(LogSessionId.LOG_SESSION_ID_NONE, /* playerName= */ ""); + PlayerId playerId2 = new PlayerId(/* playerName= */ ""); loadControl = builder.setAllocator(allocator).build(); loadControl.onPrepared(playerId); @@ -743,7 +738,7 @@ public class DefaultLoadControlTest { @Test public void onTrackSelected_updatesTargetBufferBytes_correctTargetBufferSizeFromTrackType() { - PlayerId playerId2 = new PlayerId(LogSessionId.LOG_SESSION_ID_NONE, /* playerName= */ ""); + PlayerId playerId2 = new PlayerId(/* playerName= */ ""); loadControl = builder.setAllocator(allocator).build(); loadControl.onPrepared(playerId); loadControl.onPrepared(playerId2); @@ -791,7 +786,7 @@ public class DefaultLoadControlTest { @Test public void onRelease_removesLoadingStateOfPlayer() { - PlayerId playerId2 = new PlayerId(LogSessionId.LOG_SESSION_ID_NONE, /* playerName= */ ""); + PlayerId playerId2 = new PlayerId(/* playerName= */ ""); loadControl = builder.setAllocator(allocator).build(); loadControl.onPrepared(playerId); loadControl.onPrepared(playerId2);