From 93fb0fd7287e6c86dcd8cdfca9ad43900db9c9be Mon Sep 17 00:00:00 2001 From: ponzischeme89 Date: Sun, 16 Aug 2026 22:08:43 +1200 Subject: [PATCH] 0.2.70 - Cold start improvements --- CHANGELOG.md | 3 + CLAUDE.md | 25 ++- .../memby/data/EmbyRepository.kt | 122 +++++++++-- .../memby/data/remote/EmbyServiceFactory.kt | 7 +- .../data/remote/GatewayServiceFactory.kt | 10 +- .../memby/data/remote/HttpStack.kt | 19 +- .../ponzischeme89/memby/ui/MainActivity.kt | 17 +- .../memby/ui/player/PlaybackRecovery.kt | 10 + .../memby/ui/player/PlaybackTrace.kt | 13 +- .../memby/ui/player/PlayerActivity.kt | 195 ++++++++++++++---- .../memby/ui/player/PlayerEngine.kt | 4 +- .../memby/ui/player/SeekControls.kt | 21 ++ .../memby/ui/player/PlaybackRecoveryTest.kt | 7 + .../memby/ui/player/PlaybackTraceTest.kt | 7 +- .../memby/ui/player/SeekControlsTest.kt | 22 ++ 15 files changed, 402 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8944ec3..f6cd837 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +## 0.2.70 - 2026-08-16 +- Improvements: Playback "cold start" improvements. + ## 0.2.69 - 2026-08-16 - Fixed: Cached recommendation shelves remain visible while fresh recommendations rebuild, avoiding row flicker and recomposition. - Fixed: “For You” content now loads only when opened, rather than competing with homepage requests. diff --git a/CLAUDE.md b/CLAUDE.md index 1c37dc1..de27494 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1280,8 +1280,11 @@ decoded. Four things exist to hold it down, and each is easy to give back: construction is on the critical path of every launch. It pulls media bytes through `HttpStack` rather than media3's own HttpURLConnection client, so the header, index and offset requests a resume makes reuse one connection instead of repeating the handshake - three times; and it enables constant-bitrate seeking, so a container with no usable seek - table computes the offset instead of reading its way there. Both are borrowed from + three times. PlaybackInfo and media bytes use a dedicated dispatcher, so a launcher full + of queued artwork cannot occupy the stream's per-host slots; it still shares the same + connection pool, retaining the warm connection. It also enables constant-bitrate seeking, + so a container with no usable seek table computes the offset instead of reading its way + there. The connection reuse and seeking choices are borrowed from [Wholphin](https://github.com/damontecres/Wholphin), a Jellyfin TV client under the same GPL-2.0 licence. - **`PlayerActivity.onCreate` is ordered as critical path then decoration**, with the @@ -1312,11 +1315,19 @@ decoded. Four things exist to hold it down, and each is easy to give back: places: the `Playable`, the intent's parameter list, its `putExtra`, and `onCreate`'s `getBooleanExtra`. `subtitleDownloadAvailable` is the worked example of all four. - **`ui/player/PlaybackTrace.kt` says where the time went.** "Playback is slow" is not - actionable; `event=first_frame … activity=…(+…) player=… stream=… prepared=… ready=… - first_frame=…` is. Marks are cumulative from the Play press, a repeated stage keeps the + actionable; `event=first_frame … play_clicked=0 … source_resolved=… player_prepared=… + ready=… first_frame=…` is. Marks are cumulative from the Play press, a repeated stage keeps the first time it was reached, and a stage that never happened is absent rather than zero. `PlaybackTraceSections.kt` names the same two spans for a systrace so `:benchmark` can measure what the log can only report — see "Benchmarks" below. +- **Startup is bounded even when Media3 never throws.** Source resolution has a 15-second + deadline and one fresh foreground retry. After `prepare()`, first frame has a 20-second + deadline; the first expiry stops the player, clears its media items, negotiates a fresh + playback session and prepares again. A second expiry becomes the ordinary retry/exit + error screen. The foreground resolution discards a stale focus-prefetch after a short + grace period rather than awaiting a process-scoped deferred indefinitely. These bounds + are cancelled while the activity is stopped and reinstated on return, so backgrounding + the app is not itself treated as a playback failure. Measured on a Chromecast with Google TV against the NAS gateway, the shape is: `prepare()` → first frame is **over 90%** of a resume, the stream negotiation is ~120 ms @@ -1422,8 +1433,10 @@ gateway's catalogue. Things to preserve: unreachable. A stream that is not seekable falls through to media3 instead: nothing errors and nothing claims to have skipped. - **Only discrete presses count.** A held key repeats at the platform's rate, which is fast - enough to throw somebody minutes down a film they meant to nudge — the repeats are - consumed rather than acted on, so letting go does not open the transport either. + enough to throw somebody minutes down a film they meant to nudge. `DiscreteSeekPresses` + remembers the physical DOWN until its matching UP because some remotes report every held + repeat with `repeatCount == 0`; all repeats are consumed rather than acted on, so letting + go does not open the transport either. - **A pending skip is committed in `onStop`** and dropped by `resetSeekControls` when the episode underneath changes, or the position reported to Emby — and so where the title resumes from — is one the viewer had already skipped past. diff --git a/app/src/main/java/com/ponzischeme89/memby/data/EmbyRepository.kt b/app/src/main/java/com/ponzischeme89/memby/data/EmbyRepository.kt index 2b388d8..5296c32 100644 --- a/app/src/main/java/com/ponzischeme89/memby/data/EmbyRepository.kt +++ b/app/src/main/java/com/ponzischeme89/memby/data/EmbyRepository.kt @@ -44,6 +44,7 @@ import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withTimeoutOrNull import kotlinx.serialization.Serializable import kotlinx.serialization.json.Json import retrofit2.HttpException @@ -317,6 +318,8 @@ class EmbyRepository(private val settings: SettingsStore) { private var cachedApi: EmbyApi? = null private var cachedBaseUrl: String? = null + private var cachedPlaybackApi: EmbyApi? = null + private var cachedPlaybackBaseUrl: String? = null /** * The Memby gateway, when this build has one. Its address is fixed at build time, so @@ -328,7 +331,16 @@ class EmbyRepository(private val settings: SettingsStore) { } } + /** PlaybackInfo is isolated from home rows and artwork on its own dispatcher. */ + private val playbackGatewayApi: GatewayApi? by lazy { + ServerConfig.gatewayUrl?.let { url -> + GatewayServiceFactory.create(url, prioritisePlayback = true) { snapshot.token } + } + } + private fun requireGateway(): GatewayApi = gatewayApi ?: error("No Memby gateway configured") + private fun requirePlaybackGateway(): GatewayApi = + playbackGatewayApi ?: error("No Memby gateway configured") /** True when the backend can return the whole home screen in one request. */ val supportsBatchHome: Boolean get() = ServerConfig.isGateway @@ -347,6 +359,20 @@ class EmbyRepository(private val settings: SettingsStore) { return api } + private fun playbackApiFor(serverUrl: String): EmbyApi { + val base = normalizeServerUrl(serverUrl) + cachedPlaybackApi?.let { if (cachedPlaybackBaseUrl == base) return it } + val api = EmbyServiceFactory.create( + baseUrl = base, + deviceIdProvider = { snapshot.deviceId.ifEmpty { "memby" } }, + tokenProvider = { snapshot.token }, + prioritisePlayback = true, + ) + cachedPlaybackApi = api + cachedPlaybackBaseUrl = base + return api + } + /** * The server all requests and media URLs point at. A build that hardwires an address * (see [ServerConfig]) always wins, so repointing every install is a property change @@ -359,6 +385,11 @@ class EmbyRepository(private val settings: SettingsStore) { return apiFor(url) } + private fun requirePlaybackApi(): EmbyApi { + val url = activeServerUrl ?: error("Not connected to a server") + return playbackApiFor(url) + } + // --- Authentication ------------------------------------------------------ /** @@ -1892,14 +1923,27 @@ class EmbyRepository(private val settings: SettingsStore) { ) } if (inFlight != null) { - val resolved = inFlight.await() - return resolved.copy( - resumePositionMs = launchResumePositionMs( - resolvedPositionMs = resolved.resumePositionMs, - requestedPositionMs = request.resumePositionMs, - ), - ).also { - playableMutex.withLock { playableCache.remove(request.itemId) } + // A focus prefetch is speculative; Play is not. Give a nearly-complete request + // a small grace period, then cancel it and issue a foreground resolution on the + // dedicated playback dispatcher. Awaiting an old process-scoped Deferred with + // no bound is one way a healthy item can inherit stale launcher work forever. + val resolved = withTimeoutOrNull(PLAYABLE_IN_FLIGHT_GRACE_MS) { inFlight.await() } + if (resolved != null) { + return resolved.copy( + resumePositionMs = launchResumePositionMs( + resolvedPositionMs = resolved.resumePositionMs, + requestedPositionMs = request.resumePositionMs, + ), + ).also { + playableMutex.withLock { playableCache.remove(request.itemId) } + } + } + inFlight.cancel() + playableMutex.withLock { + if (playableInFlight[request.itemId] === inFlight) { + playableInFlight.remove(request.itemId) + } + playableCache.remove(request.itemId) } } return resolvePlayableUncached(request) @@ -1927,7 +1971,12 @@ class EmbyRepository(private val settings: SettingsStore) { ): Playable { require(itemId.isNotBlank()) { "A media item is required to refresh playback" } if (!ServerConfig.isGateway) { - val discovery = directPlayback(itemId, resumePositionMs, forceTranscode = forceTranscode) + val discovery = directPlayback( + itemId, + resumePositionMs, + forceTranscode = forceTranscode, + prioritisePlayback = true, + ) return Playable( itemId = itemId, title = title, @@ -1945,7 +1994,7 @@ class EmbyRepository(private val settings: SettingsStore) { ) } - val playback = requireGateway().playback( + val playback = requirePlaybackGateway().playback( itemId = itemId, itemType = "", title = title, @@ -1980,7 +2029,7 @@ class EmbyRepository(private val settings: SettingsStore) { positionMs: Long, ): Playable { if (ServerConfig.isGateway) { - val playback = requireGateway().playback( + val playback = requirePlaybackGateway().playback( itemId = session.itemId, itemType = "", title = title, @@ -2095,13 +2144,18 @@ class EmbyRepository(private val settings: SettingsStore) { } if (item.isSeries) { val userId = snapshot.userId ?: error("Not connected") - val episode = firstNextUpEpisode(userId, item.itemId) ?: firstEpisode(userId, item.itemId) + val episode = firstNextUpEpisode(userId, item.itemId, prioritisePlayback = true) + ?: firstEpisode(userId, item.itemId, prioritisePlayback = true) requireNotNull(episode) { "No episodes found for ${item.title}" } val title = buildString { append(item.title) episode.name.takeIf { it.isNotBlank() }?.let { append(" – $it") } } - val discovery = directPlayback(episode.id, episode.resumePositionMs) + val discovery = directPlayback( + episode.id, + episode.resumePositionMs, + prioritisePlayback = true, + ) return Playable( itemId = episode.id, title = title, @@ -2122,7 +2176,11 @@ class EmbyRepository(private val settings: SettingsStore) { runtimeMs = episode.runTimeTicks?.div(10_000L)?.coerceAtLeast(0L) ?: 0L, ) } - val discovery = directPlayback(item.itemId, item.resumePositionMs) + val discovery = directPlayback( + item.itemId, + item.resumePositionMs, + prioritisePlayback = true, + ) return Playable( itemId = item.itemId, title = item.title, @@ -2598,12 +2656,14 @@ class EmbyRepository(private val settings: SettingsStore) { subtitleStreamIndex: Int? = null, currentPlaySessionId: String? = null, forceTranscode: Boolean = false, + prioritisePlayback: Boolean = false, ): PlaybackDiscovery { val userId = snapshot.userId ?: return PlaybackDiscovery(mediaSourceId = itemId) val serverUrl = activeServerUrl ?: return PlaybackDiscovery(mediaSourceId = itemId) val token = snapshot.token.orEmpty() return runCatching { - val info = requireApi().getPlaybackInfo( + val api = if (prioritisePlayback) requirePlaybackApi() else requireApi() + val info = api.getPlaybackInfo( itemId, userId, body = PlaybackInfoRequest( @@ -2649,12 +2709,19 @@ class EmbyRepository(private val settings: SettingsStore) { url = delivery.url?.let { authenticatedDeliveryUrl(serverUrl, it, token) }, ) } ?: PlaybackDiscovery(mediaSourceId = itemId) - }.getOrElse { PlaybackDiscovery(mediaSourceId = itemId) } + }.getOrElse { error -> + if (error is CancellationException) throw error + PlaybackDiscovery(mediaSourceId = itemId) + } } - private suspend fun firstNextUpEpisode(userId: String, seriesId: String): BaseItem? = + private suspend fun firstNextUpEpisode( + userId: String, + seriesId: String, + prioritisePlayback: Boolean = false, + ): BaseItem? = runCatching { - requireApi().getNextUp( + (if (prioritisePlayback) requirePlaybackApi() else requireApi()).getNextUp( mapOf( "UserId" to userId, "SeriesId" to seriesId, @@ -2663,11 +2730,18 @@ class EmbyRepository(private val settings: SettingsStore) { "EnableUserData" to "true", ), ).items.firstOrNull() - }.getOrNull() + }.getOrElse { error -> + if (error is CancellationException) throw error + null + } - private suspend fun firstEpisode(userId: String, seriesId: String): BaseItem? = + private suspend fun firstEpisode( + userId: String, + seriesId: String, + prioritisePlayback: Boolean = false, + ): BaseItem? = runCatching { - requireApi().getEpisodes( + (if (prioritisePlayback) requirePlaybackApi() else requireApi()).getEpisodes( seriesId, mapOf( "UserId" to userId, @@ -2676,7 +2750,10 @@ class EmbyRepository(private val settings: SettingsStore) { "EnableUserData" to "true", ), ).items.firstOrNull() - }.getOrNull() + }.getOrElse { error -> + if (error is CancellationException) throw error + null + } // --- URL helpers --------------------------------------------------------- @@ -3005,6 +3082,7 @@ private data class CachedIntro(val value: ChapterMarkers) private const val PLAYABLE_CACHE_SIZE = 16 private const val PLAYABLE_CACHE_TTL_MS = 5L * 60L * 1_000L internal const val PLAYABLE_PREFETCH_MAX_AGE_MS = 15_000L +private const val PLAYABLE_IN_FLIGHT_GRACE_MS = 750L // How far back to look for the play that places a Next Up episode in Continue Watching. // This is a household's recent viewing, not its history: a series nobody has touched in // this many plays is not competing for the front of the row anyway. Mirrors the gateway's diff --git a/app/src/main/java/com/ponzischeme89/memby/data/remote/EmbyServiceFactory.kt b/app/src/main/java/com/ponzischeme89/memby/data/remote/EmbyServiceFactory.kt index 2480975..ec94840 100644 --- a/app/src/main/java/com/ponzischeme89/memby/data/remote/EmbyServiceFactory.kt +++ b/app/src/main/java/com/ponzischeme89/memby/data/remote/EmbyServiceFactory.kt @@ -23,6 +23,7 @@ object EmbyServiceFactory { baseUrl: String, deviceIdProvider: () -> String, tokenProvider: () -> String?, + prioritisePlayback: Boolean = false, ): EmbyApi { val contentType = "application/json".toMediaType() @@ -35,9 +36,9 @@ object EmbyServiceFactory { redactHeader("X-Emby-Authorization") } - // Derived from the shared stack, so this keeps the one connection pool and - // dispatcher the artwork loader also uses. See HttpStack. - val client = HttpStack.base.newBuilder() + // Derived from the shared stack, so this keeps the one connection pool. Playback + // may select its isolated dispatcher; ordinary API work shares artwork's. + val client = (if (prioritisePlayback) HttpStack.playback else HttpStack.base).newBuilder() .connectTimeout(15, TimeUnit.SECONDS) .readTimeout(30, TimeUnit.SECONDS) .addInterceptor(EmbyAuthInterceptor(deviceIdProvider, tokenProvider)) diff --git a/app/src/main/java/com/ponzischeme89/memby/data/remote/GatewayServiceFactory.kt b/app/src/main/java/com/ponzischeme89/memby/data/remote/GatewayServiceFactory.kt index 8472b77..5ec9de9 100644 --- a/app/src/main/java/com/ponzischeme89/memby/data/remote/GatewayServiceFactory.kt +++ b/app/src/main/java/com/ponzischeme89/memby/data/remote/GatewayServiceFactory.kt @@ -27,12 +27,16 @@ object GatewayServiceFactory { explicitNulls = false } - fun create(baseUrl: String, tokenProvider: () -> String?): GatewayApi { + fun create( + baseUrl: String, + prioritisePlayback: Boolean = false, + tokenProvider: () -> String?, + ): GatewayApi { val contentType = "application/json".toMediaType() // Derived from the shared stack: artwork in gateway mode is proxied by this very - // host, so the poster fetches reuse the connection this client established. - val client = HttpStack.base.newBuilder() + // host, so both the ordinary and playback dispatchers reuse the same connection. + val client = (if (prioritisePlayback) HttpStack.playback else HttpStack.base).newBuilder() .connectTimeout(10, TimeUnit.SECONDS) // The gateway answers home from Redis in single-digit milliseconds; a long // read timeout here only ever means Emby itself is struggling behind it. diff --git a/app/src/main/java/com/ponzischeme89/memby/data/remote/HttpStack.kt b/app/src/main/java/com/ponzischeme89/memby/data/remote/HttpStack.kt index a8e23bd..7ec4678 100644 --- a/app/src/main/java/com/ponzischeme89/memby/data/remote/HttpStack.kt +++ b/app/src/main/java/com/ponzischeme89/memby/data/remote/HttpStack.kt @@ -8,7 +8,8 @@ import java.util.concurrent.TimeUnit /** * The one HTTP stack in the app. Everything that talks to the network — the Emby API, the * gateway API and Coil's artwork loader — is built from [base], so they share a single - * connection pool, dispatcher and thread pool. + * connection pool. Ordinary work shares [base]'s dispatcher; explicit playback uses its + * own dispatcher so home hydration cannot queue in front of first frame. * * Sharing matters most for artwork. In gateway mode the images are proxied by the same * HTTPS host that serves `/v1/home`, so a poster fetched on a client of its own would @@ -42,6 +43,18 @@ internal object HttpStack { maxRequestsPerHost = MAX_REQUESTS_PER_HOST } + /** + * Playback gets its own dispatcher while retaining the shared connection pool. A full + * launcher can otherwise occupy every per-host slot with posters just before the + * viewer presses Play, leaving both PlaybackInfo and the first media range request in + * the artwork queue. Separate dispatchers make Play an independent lane; the shared + * pool still lets it reuse an already-warm gateway or Emby connection. + */ + private val playbackDispatcher = Dispatcher().apply { + maxRequests = 8 + maxRequestsPerHost = 8 + } + val base: OkHttpClient = OkHttpClient.Builder() .connectionPool(connectionPool) .dispatcher(dispatcher) @@ -50,4 +63,8 @@ internal object HttpStack { .readTimeout(30, TimeUnit.SECONDS) .retryOnConnectionFailure(true) .build() + + val playback: OkHttpClient = base.newBuilder() + .dispatcher(playbackDispatcher) + .build() } diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/MainActivity.kt b/app/src/main/java/com/ponzischeme89/memby/ui/MainActivity.kt index ae4cc06..351a69f 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/MainActivity.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/MainActivity.kt @@ -186,6 +186,8 @@ import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.ensureActive import kotlinx.coroutines.launch +import kotlinx.coroutines.TimeoutCancellationException +import kotlinx.coroutines.withTimeout import kotlinx.coroutines.withTimeoutOrNull import java.util.Date import java.util.Calendar @@ -295,6 +297,7 @@ class MainActivity : ComponentActivity() { * startup path, and everything it gates is already on disk. */ private const val ONBOARDING_CHECK_TIMEOUT_MS = 2_500L +private const val PLAYBACK_SOURCE_RESOLUTION_TIMEOUT_MS = 15_000L private const val UPDATE_CHECK_TIMEOUT_MS = 2_500L // Update policy is live operator state, just like maintenance. The verdict is an in-memory // gateway read, so keeping this close to the maintenance poll means a set that was already @@ -2205,7 +2208,19 @@ private fun HomeScreen( resolveJob = scope.launch { try { runCatching { - val playable = ready ?: repo.resolvePlayableForLaunch(item) + val playable = ready ?: run { + var timeout: TimeoutCancellationException? = null + repeat(2) { + try { + return@run withTimeout(PLAYBACK_SOURCE_RESOLUTION_TIMEOUT_MS) { + repo.resolvePlayableForLaunch(item) + } + } catch (error: TimeoutCancellationException) { + timeout = error + } + } + throw requireNotNull(timeout) + } // A resolution that came back without a stream is a failure with a // success's shape. Handed on, PlayerActivity finds no URL and no // request to resolve one from, closes itself in onCreate, and the diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/player/PlaybackRecovery.kt b/app/src/main/java/com/ponzischeme89/memby/ui/player/PlaybackRecovery.kt index 635b44d..f227cdc 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/player/PlaybackRecovery.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/player/PlaybackRecovery.kt @@ -87,6 +87,16 @@ internal fun automaticRetryDelayMs(attempt: Int): Long? = else -> null } +internal enum class StartupTimeoutAction { RETRY_CLEANLY, SHOW_ERROR } + +/** One automatic replacement of a stale startup stage, then a finite error state. */ +internal fun startupTimeoutAction(completedRecoveries: Int): StartupTimeoutAction = + if (completedRecoveries < 1) { + StartupTimeoutAction.RETRY_CLEANLY + } else { + StartupTimeoutAction.SHOW_ERROR + } + /** One prolonged mid-programme rebuffer gets a lower-risk H.264 stream. */ internal fun shouldRecoverProlongedRebuffer( renderedFirstFrame: Boolean, diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/player/PlaybackTrace.kt b/app/src/main/java/com/ponzischeme89/memby/ui/player/PlaybackTrace.kt index 046dc91..da2b5d1 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/player/PlaybackTrace.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/player/PlaybackTrace.kt @@ -18,7 +18,9 @@ internal class PlaybackTrace( private val startedAtMs: Long, private val clock: () -> Long, ) { - private val marks = LinkedHashMap() + private val marks = LinkedHashMap().apply { + put(PLAY_CLICKED, 0L) + } /** Records [stage] at the current time and returns milliseconds since Play was pressed. */ fun mark(stage: String): Long { @@ -46,17 +48,20 @@ internal class PlaybackTrace( } companion object { + /** The origin carried across the launcher/player activity boundary. */ + const val PLAY_CLICKED = "play_clicked" + /** The viewer's Play press has been handled and the player activity is alive. */ - const val ACTIVITY_CREATED = "activity" + const val ACTIVITY_CREATED = "activity_created" /** The ExoPlayer instance exists and its renderers are built. */ const val PLAYER_BUILT = "player" /** A stream URL is in hand, whether from the launch prefetch or a fresh request. */ - const val STREAM_RESOLVED = "stream" + const val STREAM_RESOLVED = "source_resolved" /** [androidx.media3.exoplayer.ExoPlayer.prepare] has been called. */ - const val PREPARED = "prepared" + const val PREPARED = "player_prepared" /** The player reported it has buffered enough to play. */ const val READY = "ready" diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/player/PlayerActivity.kt b/app/src/main/java/com/ponzischeme89/memby/ui/player/PlayerActivity.kt index 5be4381..446e264 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/player/PlayerActivity.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/player/PlayerActivity.kt @@ -99,6 +99,7 @@ import com.ponzischeme89.memby.ui.ServiceAlertBanner import com.ponzischeme89.memby.ui.randomWelcomeQuote import com.ponzischeme89.memby.ui.theme.MembyTheme import kotlinx.coroutines.Job +import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll import kotlinx.coroutines.coroutineScope @@ -108,6 +109,7 @@ import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.first import kotlinx.coroutines.isActive import kotlinx.coroutines.launch +import kotlinx.coroutines.withTimeout import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json import java.util.Date @@ -184,6 +186,9 @@ class PlayerActivity : ComponentActivity() { private var currentTrailerStartedReported = false private var pendingResolveJob: Job? = null private var pendingResolveGeneration = 0L + private var startupWatchdogJob: Job? = null + private var startupWatchdogGeneration = 0L + private var startupRecoveryAttempt = 0 private var serviceAlertsMounted = false // Open trace spans, or -1 for "not open". Held so they can be closed on destroy: a @@ -389,6 +394,7 @@ class PlayerActivity : ComponentActivity() { private var seekPreview: SeekPreview? = null private var seekCommitJob: Job? = null private var seekHideJob: Job? = null + private val discreteSeekPresses = DiscreteSeekPresses() /** * The frame the skip will land on, drawn beside the words in the same chip. It is a @@ -688,12 +694,14 @@ class PlayerActivity : ComponentActivity() { } override fun onPlayerError(error: PlaybackException) { + cancelStartupWatchdog() MembyDiagnostics.debug("media3_error", "playback" to playSessionId, "item" to itemId, "code" to error.errorCodeName, "exception" to error.cause?.javaClass?.simpleName, "detail" to error.message) handlePlaybackError(error) } override fun onRenderedFirstFrame() { + cancelStartupWatchdog() MembyDiagnostics.info("media3_first_frame", "playback" to playSessionId, "item" to itemId, "startup_ms" to (SystemClock.elapsedRealtime() - requestStartedAtMs)) renderedFirstFrame = true @@ -811,6 +819,10 @@ class PlayerActivity : ComponentActivity() { currentMediaSubtitles = subtitles val prepared = runCatching { require(url.isNotBlank()) { "Playback URL is blank" } + // A retry must not inherit a wedged loader, extractor or decoder. This is the + // in-process equivalent of the state reset an app restart used to provide. + playback.stop() + playback.clearMediaItems() playback.setMediaItem(mediaItem(url, subtitles), positionMs.coerceAtLeast(0L)) playback.playWhenReady = playWhenReady playback.prepare() @@ -840,6 +852,7 @@ class PlayerActivity : ComponentActivity() { firstFrameTraceCookie = PlaybackTraceSections.nextCookie() PlaybackTraceSections.begin(PlaybackTraceSections.FIRST_FRAME, firstFrameTraceCookie) scheduleTrailerStartupTimeout() + scheduleContentStartupTimeout() } private fun resolvePendingTrailer() { @@ -918,6 +931,63 @@ class PlayerActivity : ComponentActivity() { } } + /** + * Media3 can remain in BUFFERING without producing an exception when a vendor decoder, + * extractor or range request wedges. Bound that state. The first expiry throws away + * the entire prepared media stage and negotiates a fresh session; the second becomes a + * retryable screen rather than an infinite spinner. + */ + private fun scheduleContentStartupTimeout() { + cancelStartupWatchdog() + if (pendingTrailerRequest != null || playingNextEpisodePreview || renderedFirstFrame) return + val generation = ++startupWatchdogGeneration + startupWatchdogJob = lifecycleScope.launch { + delay(FIRST_FRAME_TIMEOUT_MS) + if (generation != startupWatchdogGeneration || renderedFirstFrame || + isFinishing || isDestroyed + ) { + return@launch + } + startupWatchdogJob = null + val action = startupTimeoutAction(startupRecoveryAttempt) + Log.w( + PLAYBACK_LOG_TAG, + "event=startup_timeout stage=first_frame item=${itemId.orEmpty()} " + + "attempt=${startupRecoveryAttempt + 1} state=${player?.playbackState?.let(::playbackStateName)} " + + "elapsedMs=${trace.elapsedMs()} ${trace.summary()}", + ) + if (action == StartupTimeoutAction.SHOW_ERROR) { + endFirstFrameTrace() + endLaunchTrace() + showPlaybackError( + PlaybackFailure( + title = getString(R.string.playback_server_unreachable), + detail = getString(R.string.playback_server_unreachable_detail), + canAutoRetry = false, + requiresFreshStream = true, + ), + ) + return@launch + } + + startupRecoveryAttempt += 1 + val positionMs = player?.currentPosition + ?.takeIf { it > 0L } + ?: initialResumePositionMs + // startMedia performs stop + clearMediaItems before preparing the replacement. + retryPlayback( + refreshSource = true, + positionOverrideMs = positionMs, + ) + } + } + + private fun cancelStartupWatchdog() { + startupWatchdogGeneration += 1 + startupWatchdogJob?.cancel() + startupWatchdogJob = null + } + private fun finishTrailerUnavailable() { trailerStartupTimeoutJob?.cancel() setResult(RESULT_TRAILER_UNAVAILABLE, Intent().putExtra(EXTRA_TRAILER_UNAVAILABLE, true)) @@ -953,11 +1023,16 @@ class PlayerActivity : ComponentActivity() { val generation = ++pendingResolveGeneration pendingResolveJob?.cancel() pendingResolveJob = lifecycleScope.launch { - runCatching { ServiceLocator.repository.resolvePlayableForLaunch(request) } - .onSuccess { playable -> - if (generation != pendingResolveGeneration || isFinishing || isDestroyed) { - return@onSuccess + var completedRecoveries = 0 + while (isActive && generation == pendingResolveGeneration) { + val result = runCatching { + withTimeout(SOURCE_RESOLUTION_TIMEOUT_MS) { + ServiceLocator.repository.resolvePlayableForLaunch(request) } + } + val playable = result.getOrNull() + if (playable != null) { + if (generation != pendingResolveGeneration || isFinishing || isDestroyed) return@launch // A resolution with no stream in it is a failure wearing a success's // shape: handed to the player it becomes an empty URI, and what the // viewer is told is whatever media3 makes of that rather than that the @@ -975,7 +1050,7 @@ class PlayerActivity : ComponentActivity() { canAutoRetry = false, ), ) - return@onSuccess + return@launch } pendingRequest = null adoptPlayable(playable) @@ -986,25 +1061,44 @@ class PlayerActivity : ComponentActivity() { positionMs = restoredPositionMs ?: playable.resumePositionMs, playWhenReady = playWhenReady, ) + return@launch } - .onFailure { error -> - if (error is kotlinx.coroutines.CancellationException) throw error - if (generation != pendingResolveGeneration || isFinishing || isDestroyed) { - return@onFailure - } - Log.e( + + val error = requireNotNull(result.exceptionOrNull()) + if (error is TimeoutCancellationException && + startupTimeoutAction(completedRecoveries) == StartupTimeoutAction.RETRY_CLEANLY + ) { + completedRecoveries += 1 + Log.w( PLAYBACK_LOG_TAG, - "event=stream_resolve_failed item=${request.itemId}", - error, + "event=startup_timeout stage=source_resolution item=${request.itemId} " + + "attempt=$completedRecoveries elapsedMs=${trace.elapsedMs()} ${trace.summary()}", ) - showPlaybackError( - PlaybackFailure( - title = getString(R.string.playback_server_unreachable), - detail = getString(R.string.playback_server_unreachable_detail), - canAutoRetry = false, - ), + showPlaybackLoading( + title = getString(R.string.playback_reconnecting), + hint = getString(R.string.playback_refreshing_stream), ) + continue } + if (error is kotlinx.coroutines.CancellationException) throw error + if (generation != pendingResolveGeneration || isFinishing || isDestroyed) { + return@launch + } + Log.e( + PLAYBACK_LOG_TAG, + "event=stream_resolve_failed item=${request.itemId} " + + "timeout=${error is TimeoutCancellationException}", + error, + ) + showPlaybackError( + PlaybackFailure( + title = getString(R.string.playback_server_unreachable), + detail = getString(R.string.playback_server_unreachable_detail), + canAutoRetry = false, + ), + ) + return@launch + } } } @@ -1495,6 +1589,7 @@ class PlayerActivity : ComponentActivity() { return@setOnClickListener } automaticRetryAttempt = 0 + startupRecoveryAttempt = 0 retryPlayback(refreshSource = true) } overlay.findViewById(R.id.playback_error_exit).setOnClickListener { @@ -1565,7 +1660,11 @@ class PlayerActivity : ComponentActivity() { showPlaybackError(failure) } - private fun retryPlayback(refreshSource: Boolean, forceTranscode: Boolean = false) { + private fun retryPlayback( + refreshSource: Boolean, + forceTranscode: Boolean = false, + positionOverrideMs: Long? = null, + ) { val generation = ++retryGeneration retryJob?.cancel() prolongedRebufferJob?.cancel() @@ -1578,7 +1677,9 @@ class PlayerActivity : ComponentActivity() { resolvePendingStream(request, playWhenReady = true) return } - val positionMs = playback.currentPosition.coerceAtLeast(0L) + val positionMs = positionOverrideMs + ?.coerceAtLeast(0L) + ?: playback.currentPosition.coerceAtLeast(0L) showPlaybackLoading( title = getString(R.string.playback_reconnecting), hint = getString( @@ -1601,12 +1702,14 @@ class PlayerActivity : ComponentActivity() { retryJob = lifecycleScope.launch { runCatching { - ServiceLocator.repository.refreshPlayableStream( - itemId = id, - title = playbackTitle, - resumePositionMs = positionMs, - forceTranscode = forceTranscode, - ) + withTimeout(SOURCE_RESOLUTION_TIMEOUT_MS) { + ServiceLocator.repository.refreshPlayableStream( + itemId = id, + title = playbackTitle, + resumePositionMs = positionMs, + forceTranscode = forceTranscode, + ) + } }.onSuccess { refreshed -> if (generation != retryGeneration || itemId != id || isFinishing || isDestroyed) { return@onSuccess @@ -1649,13 +1752,18 @@ class PlayerActivity : ComponentActivity() { ) startMedia(refreshed.url, refreshed.subtitles, positionMs, playWhenReady = true) }.onFailure { refreshError -> - if (refreshError is kotlinx.coroutines.CancellationException) throw refreshError + if (refreshError is kotlinx.coroutines.CancellationException && + refreshError !is TimeoutCancellationException + ) { + throw refreshError + } if (generation != retryGeneration || itemId != id || isFinishing || isDestroyed) { return@onFailure } Log.e( PLAYBACK_LOG_TAG, - "event=stream_refresh_failed item=$id attempt=$automaticRetryAttempt", + "event=stream_refresh_failed item=$id attempt=$automaticRetryAttempt " + + "timeout=${refreshError is TimeoutCancellationException}", refreshError, ) showPlaybackError( @@ -2382,6 +2490,7 @@ class PlayerActivity : ComponentActivity() { /** Drops a pending skip and takes the OSD down. Used when the item underneath changes. */ private fun resetSeekControls() { + discreteSeekPresses.reset() seekCommitJob?.cancel() seekCommitJob = null seekHideJob?.cancel() @@ -3560,14 +3669,19 @@ class PlayerActivity : ComponentActivity() { return true } if (event.keyCode in SEEK_BACK_KEYS || event.keyCode in SEEK_FORWARD_KEYS) { + // Release the gate even if an overlay appeared between DOWN and UP. The event + // still belongs to that overlay when seek controls are inactive. + if (event.action == KeyEvent.ACTION_UP) discreteSeekPresses.keyUp(event.keyCode) if (!seekControlsActive()) return super.dispatchKeyEvent(event) - // Only discrete presses move the film. A held key repeats at the platform's - // own rate, which on these remotes is fast enough to throw somebody minutes - // down a film they meant to nudge — and the repeats are consumed rather than - // passed on, or letting go would open the transport controls on top of the - // OSD that is already answering the question. - if (event.action == KeyEvent.ACTION_DOWN && event.repeatCount == 0) { - nudgeSeek(forward = event.keyCode in SEEK_FORWARD_KEYS) + // Only the first DOWN before its matching UP moves the film. Some remotes do + // not increase repeatCount while held, so checking that value alone can turn + // one hold into a run of 10-second nudges and throw the viewer minutes ahead. + // Consume every event regardless, or release would open the transport over + // the OSD that is already answering the question. + when (event.action) { + KeyEvent.ACTION_DOWN -> if (discreteSeekPresses.keyDown(event.keyCode)) { + nudgeSeek(forward = event.keyCode in SEEK_FORWARD_KEYS) + } } return true } @@ -4498,6 +4612,9 @@ class PlayerActivity : ComponentActivity() { }?.let { request -> resolvePendingStream(request, playWhenReady = true) } + if (!renderedFirstFrame && pendingRequest == null && player?.currentMediaItem != null) { + scheduleContentStartupTimeout() + } if (retryAfterResume) { retryAfterResume = false retryPlayback(refreshSource = true) @@ -4523,8 +4640,11 @@ class PlayerActivity : ComponentActivity() { // Land any pending skip first, or the position reported to Emby — and so where // this title resumes from — is the one the viewer had already skipped past. commitSeek() + // Backgrounding may prevent the matching key-up event reaching this activity. + discreteSeekPresses.reset() pendingResolveGeneration += 1 pendingResolveJob?.cancel() + cancelStartupWatchdog() if (retryJob?.isActive == true) { retryAfterResume = true retryGeneration += 1 @@ -4557,6 +4677,7 @@ class PlayerActivity : ComponentActivity() { endLaunchTrace() stopProgressUploading() pendingResolveJob?.cancel() + cancelStartupWatchdog() prerollTimerJob?.cancel() prerollScheduleJob?.cancel() disposeLocalPreroll(reuse = true) @@ -4812,6 +4933,8 @@ class PlayerActivity : ComponentActivity() { private const val SUBTITLE_BOTTOM_PADDING_FRACTION = 0.095f private val playerJson = Json { ignoreUnknownKeys = true } private const val TRAILER_STARTUP_TIMEOUT_MS = 8_000L + private const val SOURCE_RESOLUTION_TIMEOUT_MS = 15_000L + private const val FIRST_FRAME_TIMEOUT_MS = 20_000L private const val RESULT_TRAILER_UNAVAILABLE = Activity.RESULT_FIRST_USER + 17 /** diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/player/PlayerEngine.kt b/app/src/main/java/com/ponzischeme89/memby/ui/player/PlayerEngine.kt index 962248e..8111f61 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/player/PlayerEngine.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/player/PlayerEngine.kt @@ -99,7 +99,9 @@ internal object PlayerEngine { * No call timeout is set, and none should be — that would cap the length of the film. */ private val streamClient by lazy { - HttpStack.base.newBuilder() + // Media bytes use the dedicated playback dispatcher. Artwork and home hydration + // retain the same connection pool, but can no longer queue in front of first frame. + HttpStack.playback.newBuilder() .connectTimeout(STREAM_CONNECT_TIMEOUT_SECONDS, TimeUnit.SECONDS) .readTimeout(STREAM_READ_TIMEOUT_SECONDS, TimeUnit.SECONDS) .build() diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/player/SeekControls.kt b/app/src/main/java/com/ponzischeme89/memby/ui/player/SeekControls.kt index 1a769e9..a8b07f3 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/player/SeekControls.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/player/SeekControls.kt @@ -27,6 +27,27 @@ private const val SEEK_END_GUARD_MS: Long = 1_000L */ data class SeekPreview(val targetMs: Long, val offsetMs: Long) +/** + * Turns a platform key stream into discrete presses. + * + * Most remotes mark held-key events with a growing repeat count, but some send every + * repeated DOWN as though it were the first. Remembering which key is physically down is + * what keeps either remote to one seek step until the corresponding UP arrives. + */ +class DiscreteSeekPresses { + private val pressedKeys = mutableSetOf() + + fun keyDown(keyCode: Int): Boolean = pressedKeys.add(keyCode) + + fun keyUp(keyCode: Int) { + pressedKeys.remove(keyCode) + } + + fun reset() { + pressedKeys.clear() + } +} + /** * Adds one step to an in-flight preview, or starts one from the playhead. * diff --git a/app/src/test/java/com/ponzischeme89/memby/ui/player/PlaybackRecoveryTest.kt b/app/src/test/java/com/ponzischeme89/memby/ui/player/PlaybackRecoveryTest.kt index 1fec374..c3101cd 100644 --- a/app/src/test/java/com/ponzischeme89/memby/ui/player/PlaybackRecoveryTest.kt +++ b/app/src/test/java/com/ponzischeme89/memby/ui/player/PlaybackRecoveryTest.kt @@ -65,6 +65,13 @@ class PlaybackRecoveryTest { assertNull(automaticRetryDelayMs(3)) } + @Test + fun startupTimeoutGetsOneCleanRecoveryThenStopsSpinning() { + assertEquals(StartupTimeoutAction.RETRY_CLEANLY, startupTimeoutAction(0)) + assertEquals(StartupTimeoutAction.SHOW_ERROR, startupTimeoutAction(1)) + assertEquals(StartupTimeoutAction.SHOW_ERROR, startupTimeoutAction(2)) + } + @Test fun prolongedMidProgrammeRebufferGetsOneCompatibilityFallback() { assertTrue(shouldRecoverProlongedRebuffer(true, false, false, false)) diff --git a/app/src/test/java/com/ponzischeme89/memby/ui/player/PlaybackTraceTest.kt b/app/src/test/java/com/ponzischeme89/memby/ui/player/PlaybackTraceTest.kt index 14082d2..7964af4 100644 --- a/app/src/test/java/com/ponzischeme89/memby/ui/player/PlaybackTraceTest.kt +++ b/app/src/test/java/com/ponzischeme89/memby/ui/player/PlaybackTraceTest.kt @@ -35,7 +35,8 @@ class PlaybackTraceTest { trace.mark(PlaybackTrace.FIRST_FRAME) assertEquals( - "activity=100ms(+100ms) prepared=250ms(+150ms) first_frame=900ms(+650ms)", + "play_clicked=0ms(+0ms) activity_created=100ms(+100ms) " + + "player_prepared=250ms(+150ms) first_frame=900ms(+650ms)", trace.summary(), ) } @@ -53,7 +54,7 @@ class PlaybackTraceTest { trace.mark(PlaybackTrace.READY) clock.nowMs = 4_000L assertEquals(500L, trace.mark(PlaybackTrace.READY)) - assertEquals("ready=500ms(+500ms)", trace.summary()) + assertEquals("play_clicked=0ms(+0ms) ready=500ms(+500ms)", trace.summary()) } /** Stages that never happened are absent, not reported as having taken no time. */ @@ -65,7 +66,7 @@ class PlaybackTraceTest { clock.nowMs = 60L trace.mark(PlaybackTrace.ACTIVITY_CREATED) - assertEquals("activity=60ms(+60ms)", trace.summary()) + assertEquals("play_clicked=0ms(+0ms) activity_created=60ms(+60ms)", trace.summary()) } /** diff --git a/app/src/test/java/com/ponzischeme89/memby/ui/player/SeekControlsTest.kt b/app/src/test/java/com/ponzischeme89/memby/ui/player/SeekControlsTest.kt index e383185..f3fe7fc 100644 --- a/app/src/test/java/com/ponzischeme89/memby/ui/player/SeekControlsTest.kt +++ b/app/src/test/java/com/ponzischeme89/memby/ui/player/SeekControlsTest.kt @@ -9,6 +9,28 @@ class SeekControlsTest { private val duration = 90L * 60_000L + @Test + fun `a held remote key produces only one seek press`() { + val presses = DiscreteSeekPresses() + + assertEquals(true, presses.keyDown(22)) + assertEquals(false, presses.keyDown(22)) + assertEquals(false, presses.keyDown(22)) + + presses.keyUp(22) + assertEquals(true, presses.keyDown(22)) + } + + @Test + fun `reset releases a seek key whose up event was lost`() { + val presses = DiscreteSeekPresses() + + assertEquals(true, presses.keyDown(21)) + presses.reset() + + assertEquals(true, presses.keyDown(21)) + } + @Test fun `a press moves one interval from the playhead`() { val preview = accumulateSeek(