From 8e60a79eaf7d73c8d7696681d0dc2eeaf4df4987 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Apr 2026 03:46:49 +0000 Subject: [PATCH 1/8] perf(video): reduce jitter and per-recomposition work in note video pipeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tightens the hot path that runs whenever a video appears inside a note in the feed (RichText -> ZoomableContentView -> VideoView). - VideoView: resolve aspect ratio once per (uri, dim), and prime MediaAspectRatioCache from the imeta dim tag so repeat appearances (PiP, dialog, list re-enter) don't have to wait for ExoPlayer's onVideoSizeChanged before reserving layout space. - VideoView: key the manual "tap to show" toggle on videoUri so a recycled feed slot doesn't inherit stale state from the previous video. - VideoViewInner: hoist proxyPortForVideo() into a remember(videoUri) — the result was being recomputed every recomposition only to be dropped by GetMediaItem's URI-keyed remember. - RenderVideoPlayer: stop holding container size in compose state. The size is only read in onDoubleTap, so a non-state IntArray holder removes a recomposition of the whole player tree on every layout pass. Also memoize isLiveStreaming() so the .m3u8 substring scan doesn't run on every recomposition. - RenderTopButtons: same isLiveStreaming() memoization. - GetVideoController: switch the remaining non-lambda Log.d call to the lambda overload so the message string isn't formatted when the log level is filtered out. --- .../playback/composable/GetVideoController.kt | 2 +- .../playback/composable/RenderVideoPlayer.kt | 12 +++++----- .../service/playback/composable/VideoView.kt | 22 ++++++++++++++----- .../playback/composable/VideoViewInner.kt | 7 +++++- .../composable/controls/RenderTopButtons.kt | 2 +- 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/GetVideoController.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/GetVideoController.kt index b561a4538..9c8bd568f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/GetVideoController.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/GetVideoController.kt @@ -52,7 +52,7 @@ fun GetVideoController( if (BackgroundMedia.isPlaying()) { // There is a video playing, start this one on mute. state.controller.volume = 0f - Log.d("PlaybackService", "OnEach Muted due to BackgroundMedia.isPlaying") + Log.d("PlaybackService") { "OnEach Muted due to BackgroundMedia.isPlaying" } } else { // There is no other video playing. Use the default mute state to // decide if sound is on or not. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt index 723899fd8..c86b117e4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt @@ -34,7 +34,6 @@ import androidx.compose.ui.geometry.Size import androidx.compose.ui.input.pointer.pointerInput import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.layout.onSizeChanged -import androidx.compose.ui.unit.IntSize import androidx.media3.common.Player import androidx.media3.common.util.UnstableApi import androidx.media3.ui.compose.ContentFrame @@ -90,19 +89,22 @@ fun RenderVideoPlayer( hasBlurhash: Boolean = false, accountViewModel: AccountViewModel, ) { - val containerSize = remember { mutableStateOf(IntSize.Zero) } - val isLive = isLiveStreaming(mediaItem.src.videoUri) + // Hold the container size in a non-state holder so layout passes don't trigger an + // unnecessary recomposition of the whole player tree just to update a value that is only + // ever read inside the onDoubleTap callback below. + val containerWidth = remember { intArrayOf(0) } + val isLive = remember(mediaItem.src.videoUri) { isLiveStreaming(mediaItem.src.videoUri) } Box( modifier = borderModifier - .onSizeChanged { containerSize.value = it } + .onSizeChanged { containerWidth[0] = it.width } .pointerInput(isLive, controllerState) { detectTapGestures( onTap = { controllerVisible.value = !controllerVisible.value }, onDoubleTap = { offset -> if (!isLive) { - val isLeftSide = offset.x < containerSize.value.width / 2 + val isLeftSide = offset.x < containerWidth[0] / 2 if (isLeftSide) { controllerState.controller.seekBackward() } else { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoView.kt index 885ecaaba..0160a1246 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoView.kt @@ -105,15 +105,29 @@ fun VideoView( thumbhash: String? = null, ) { val initialAutoStart = if (alwaysShowVideo) true else accountViewModel.settings.startVideoPlayback() - val automaticallyStartPlayback = remember { mutableStateOf(initialAutoStart) } + // Reset the manual-show toggle when the video URI changes so a recycled feed slot + // doesn't inherit "tapped to show" state from a prior video. + val automaticallyStartPlayback = remember(videoUri) { mutableStateOf(initialAutoStart) } // Once the video is being shown, only honor the user's autoplay preference when it was auto-loaded. // If the user manually tapped the download button, they want it to play. val autoplay = alwaysShowVideo || (initialAutoStart && accountViewModel.settings.autoPlayVideos()) || (!initialAutoStart && automaticallyStartPlayback.value) - if (blurhash == null && thumbhash == null) { - val ratio = dimensions?.aspectRatio() ?: MediaAspectRatioCache.get(videoUri) + // Resolve the aspect ratio once per composition. Prime the URL-keyed cache from the imeta + // dim tag so the next time this video appears (PiP, dialog, list re-enter) the cache hits + // without waiting for ExoPlayer's onVideoSizeChanged. + val ratio = + remember(videoUri, dimensions) { + val fromDim = dimensions?.takeIf { it.hasSize() } + if (fromDim != null) { + MediaAspectRatioCache.add(videoUri, fromDim.width, fromDim.height) + fromDim.aspectRatio() + } else { + MediaAspectRatioCache.get(videoUri) + } + } + if (blurhash == null && thumbhash == null) { val modifier = if (ratio != null && automaticallyStartPlayback.value) { Modifier.aspectRatio(ratio) @@ -149,8 +163,6 @@ fun VideoView( } } } else { - val ratio = dimensions?.aspectRatio() ?: MediaAspectRatioCache.get(videoUri) - val modifier = if (ratio != null) { Modifier.aspectRatio(ratio) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoViewInner.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoViewInner.kt index 8923783f0..49e5651a4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoViewInner.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoViewInner.kt @@ -59,6 +59,11 @@ fun VideoViewInner( // keeps a copy of the value to avoid recompositions here when the DEFAULT value changes val muted = remember(videoUri) { DEFAULT_MUTED_SETTING.value } + // The proxy port is decided once per video URI; recomputing on every recomposition does + // pointless work (the result is anyway dropped because GetMediaItem.remember is keyed on the + // URI alone, so the cached MediaItemData is locked in on the first frame). + val proxyPort = remember(videoUri) { accountViewModel.httpClientBuilder.proxyPortForVideo(videoUri) } + GetMediaItem( videoUri = videoUri, title = title, @@ -67,7 +72,7 @@ fun VideoViewInner( callbackUri = nostrUriCallback, mimeType = mimeType, aspectRatio = aspectRatio, - proxyPort = accountViewModel.httpClientBuilder.proxyPortForVideo(videoUri), + proxyPort = proxyPort, keepPlaying = true, waveformData = waveform, isLiveStream = isLiveStream, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt index 5c4d4250e..d7c1645f0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt @@ -105,7 +105,7 @@ fun RenderTopButtons( accountViewModel: AccountViewModel, ) { val context = LocalContext.current - val isLive = isLiveStreaming(mediaData.videoUri) + val isLive = remember(mediaData.videoUri) { isLiveStreaming(mediaData.videoUri) } val pipSupported = remember { context.packageManager.hasSystemFeature(PackageManager.FEATURE_PICTURE_IN_PICTURE) From a11ba2e55d40c2aca72f7149eaca05fc9f5d453a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Apr 2026 04:01:59 +0000 Subject: [PATCH 2/8] perf(video): warm up ExoPlayer pool, tune LoadControl, fix notification fall-through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small infrastructure fixes that support the eager-prepare model (every visible feed video calls setMediaItem + prepare immediately so it's ready when the user scrolls to it). - ExoPlayerPool.create(): the warmup that builds poolStartingSize players up front was already written but never invoked. Wire it from PlaybackService.lazyPool() the first time a pool is requested. The builds now run on the pool's main-looper scope with a yield() between each so they're spread across frames instead of stalling the UI in one ~150–600 ms burst. Idempotent via an AtomicBoolean. - ExoPlayerBuilder: install a feed-tuned DefaultLoadControl (10s/15s/750ms/2000ms) instead of the 50s/50s/2.5s/5s defaults. Every visible video preloads, so 5 simultaneous players were each trying to buffer 50s ahead — fighting for network and burning ~30 MB of buffer per HD player. Capping at 15s keeps the active video smooth, lets it start playing as soon as ~750 ms is buffered, and slashes peak memory on feeds with several preloads. - PlaybackService.onUpdateNotification: the third forEachIndexed loop was missing its return, so on the muted-but-playing fallback path super.onUpdateNotification was called once per playing session instead of once total. With multiple feed videos preloading simultaneously this was hammering the notification system every time a player changed state. Match the first two loops by returning after the first match. Also drop the unused `idx` from forEachIndexed. --- .../playback/playerPool/ExoPlayerBuilder.kt | 26 +++++++++++++++++++ .../playback/playerPool/ExoPlayerPool.kt | 23 ++++++++++++++-- .../playback/service/PlaybackService.kt | 26 +++++++++++++++---- 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/ExoPlayerBuilder.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/ExoPlayerBuilder.kt index 4ceb4497b..eaa31e31e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/ExoPlayerBuilder.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/ExoPlayerBuilder.kt @@ -24,6 +24,7 @@ import android.content.Context import androidx.annotation.OptIn import androidx.media3.common.util.UnstableApi import androidx.media3.datasource.DataSource +import androidx.media3.exoplayer.DefaultLoadControl import androidx.media3.exoplayer.ExoPlayer import com.vitorpamplona.amethyst.model.MediaAspectRatioCache import com.vitorpamplona.amethyst.service.playback.diskCache.VideoCache @@ -42,10 +43,35 @@ class ExoPlayerBuilder( .Builder(context) .apply { setMediaSourceFactory(CustomMediaSourceFactory(videoCache, dataSourceFactory)) + setLoadControl(feedTunedLoadControl()) }.build() .apply { addListener(AspectRatioCacher(MediaAspectRatioCache)) addListener(KeepVideosPlaying(this)) addListener(CurrentPlayPositionCacher(this, VideoViewedPositionCache)) } + + companion object { + // Default DefaultLoadControl buffers 50s ahead before slowing down. Every visible video + // in the feed is prepared eagerly so it's ready when the user scrolls to it; with the + // default settings 5 simultaneous preloads would fight for ~250s of buffer between them + // and chew through ~30+ MB per HD player. Feed playback is optimized for "the active + // video plays smoothly while a few neighbours stay warm," so we cap the buffer at ~15s + // and let playback kick in as soon as ~750 ms is buffered. Fullscreen still gets a + // healthy buffer because seeks-within-15s are virtually instant from disk cache. + private fun feedTunedLoadControl() = + DefaultLoadControl + .Builder() + .setBufferDurationsMs( + // minBufferMs = + 10_000, + // maxBufferMs = + 15_000, + // bufferForPlaybackMs = + 750, + // bufferForPlaybackAfterRebufferMs = + 2_000, + ).setPrioritizeTimeOverSizeThresholds(true) + .build() + } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/ExoPlayerPool.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/ExoPlayerPool.kt index 4a32d8066..962ab5331 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/ExoPlayerPool.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/ExoPlayerPool.kt @@ -34,7 +34,9 @@ import kotlinx.coroutines.cancel import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.yield import java.util.concurrent.ConcurrentLinkedQueue +import java.util.concurrent.atomic.AtomicBoolean @OptIn(UnstableApi::class) class ExoPlayerPool( @@ -54,9 +56,26 @@ class ExoPlayerPool( private val mutex = Mutex() + // Guards against firing the warmup more than once if create() is called repeatedly + // (e.g. on reconfiguration or when both pools share a startup hook). + private val warmupStarted = AtomicBoolean(false) + + /** + * Pre-warms the pool with [poolStartingSize] ExoPlayer instances on the main looper, yielding + * between each build so the warmup is spread across frames instead of stalling the UI in one + * burst. ExoPlayer must be constructed on the same thread that will operate it (the main + * thread for this pool), so we cannot fan out across IO threads here. Idempotent — additional + * calls are no-ops. + */ fun create(context: Context) { - while (playerPool.size < poolStartingSize) { - playerPool.offer(builder.build(context)) + if (!warmupStarted.compareAndSet(false, true)) return + scope.launch { + while (playerPool.size < poolStartingSize) { + playerPool.offer(builder.build(context)) + // Hand the frame back so an in-flight onGetSession / acquirePlayer / layout + // pass isn't blocked behind the next build. + yield() + } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackService.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackService.kt index 9b259e69c..f323e01bc 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackService.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackService.kt @@ -105,7 +105,15 @@ class PlaybackService : MediaSessionService() { val blossomServerResolver = Amethyst.instance.blossomResolver // creates new - return newPool(videoCache, okHttpClient, blossomServerResolver).also { poolNoProxy = it } + return newPool(videoCache, okHttpClient, blossomServerResolver) + .also { + poolNoProxy = it + // Kick off the player pool warmup as soon as we know this pool is being used. + // It runs async on the main looper, yielding between builds, so the very first + // session still acquires synchronously while subsequent ones can grab a warm + // ExoPlayer instead of paying the build cost on the main thread. + it.exoPlayerPool.create(applicationContext) + } } else { poolWithProxy?.let { return it } @@ -116,7 +124,11 @@ class PlaybackService : MediaSessionService() { val videoCache = Amethyst.instance.videoCache val blossomServerResolver = Amethyst.instance.blossomResolver - return newPool(videoCache, okHttpClient, blossomServerResolver).also { poolWithProxy = it } + return newPool(videoCache, okHttpClient, blossomServerResolver) + .also { + poolWithProxy = it + it.exoPlayerPool.create(applicationContext) + } } } @@ -161,23 +173,27 @@ class PlaybackService : MediaSessionService() { return } - playing.forEachIndexed { idx, it -> + playing.forEach { if (it.session.player.isPlaying && it.session.player.volume > 0 && it.session.id == BackgroundMedia.bgInstance?.id) { super.onUpdateNotification(it.session, startInForegroundRequired) return } } - playing.forEachIndexed { idx, it -> + playing.forEach { if (it.session.player.isPlaying && it.session.player.volume > 0) { super.onUpdateNotification(it.session, startInForegroundRequired) return } } - playing.forEachIndexed { idx, it -> + // Falls through to the first muted-but-playing session. Earlier this loop missed + // its return and called super.onUpdateNotification once per playing session, + // hammering the notification system whenever multiple feed videos were preloading. + playing.forEach { if (it.session.player.isPlaying) { super.onUpdateNotification(it.session, startInForegroundRequired) + return } } } From c9a19b90f016374c2452e1b9af87f2534864435f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Apr 2026 08:49:48 +0000 Subject: [PATCH 3/8] perf(video): retain warm ExoPlayers and clear up the controller hot path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the remaining audit items so the eager-prepare model also pays off on scroll-back. The big change is keeping the most recent N feed players paused-with-buffer instead of stop()'ing them on release. P0 — Warm-slot ExoPlayer pool: - ExoPlayerPool now retains up to N (default 3) paused players keyed by the mediaId they last loaded. Acquire takes an optional preferredMediaId hint and returns the matching warm player intact; only the cold fallback path runs stop()/clearMediaItems(). Warm slots count against the device's MediaCodec budget (poolSize) so the cold cap is poolSize - warmSize, with warm slots themselves capped at poolSize-1 to guarantee there's always at least one cold slot for a brand-new URI. - Plumb videoUri through connection hints (PlaybackServiceClient -> PlaybackService.onGetSession -> MediaSessionPool.getSession -> ExoPlayerPool.acquirePlayer) so the service can find a warm match. Constants for the bundle keys live on PlaybackService. - GetVideoController.onEach now checks state.controller.currentMediaItem?.mediaId before calling setMediaItem. On a warm hit it leaves the player and its buffer alone — calling setMediaItem in that case would reset the player and undo the whole point of the warm pool. STATE_IDLE survivors still get a re-prepare. P1 — Stop rebuilding the MediaController on transient lifecycle dips: - GetVideoController.collectAsStateWithLifecycle was tearing down and re-binding the MediaController every time the activity lifecycle dropped below STARTED (system dialogs, briefly switching apps, notification shade). Switch to plain collectAsState — the controller now lives until the composable actually leaves composition, so a brief lifecycle dip no longer costs a full IPC rebind + buffer reload. Real backgrounding still tears down via composable disposal. P2 — Smaller fixes flushed at the same time: - GetVideoController: only write controller.volume when it differs from target. Combined with the new dedup, several feed videos preloading no longer fire one volume IPC per ready callback. - CurrentPlayPositionCacher: the resume threshold was `5 * 60`, which in milliseconds is 300 ms — i.e. "always seek". Bump to 5_000 (5 s) so trivially short clips don't pay an extra seek + buffer flush at STATE_READY just to land 100 ms away from where they started. - MediaSessionPool: stop allocating a fresh DataSourceBitmapLoader per session — the loader has no per-session state, so it's now a single lazy instance shared across all sessions in a pool. - MediaSessionPool.cleanupUnused was racy: concurrent releases all won the time check and each launched a redundant sweep coroutine. Replace with a CAS-guarded AtomicLong on a nano-precision timestamp. --- .../playback/composable/GetVideoController.kt | 45 +++-- .../playback/playerPool/ExoPlayerPool.kt | 162 ++++++++++++++---- .../playback/playerPool/MediaSessionPool.kt | 60 ++++--- .../positions/CurrentPlayPositionCacher.kt | 9 +- .../playback/service/PlaybackService.kt | 12 +- .../playback/service/PlaybackServiceClient.kt | 10 +- 6 files changed, 231 insertions(+), 67 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/GetVideoController.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/GetVideoController.kt index 9c8bd568f..869dc653d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/GetVideoController.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/GetVideoController.kt @@ -21,10 +21,11 @@ package com.vitorpamplona.amethyst.service.playback.composable import androidx.compose.runtime.Composable +import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.remember import androidx.compose.ui.platform.LocalContext -import androidx.lifecycle.compose.collectAsStateWithLifecycle +import androidx.media3.common.Player import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.LoadedMediaItem import com.vitorpamplona.amethyst.service.playback.pip.BackgroundMedia import com.vitorpamplona.amethyst.service.playback.service.PlaybackServiceClient @@ -49,25 +50,43 @@ fun GetVideoController( ).onEach { state -> Log.d("PlaybackService") { "Controller instance: ${state.controller}" } - if (BackgroundMedia.isPlaying()) { - // There is a video playing, start this one on mute. - state.controller.volume = 0f - Log.d("PlaybackService") { "OnEach Muted due to BackgroundMedia.isPlaying" } - } else { - // There is no other video playing. Use the default mute state to - // decide if sound is on or not. - state.controller.volume = if (muted) 0f else 1f - Log.d("PlaybackService") { "OnEach $muted" } + // The default ExoPlayer volume is 1f and the MediaSessionPool reset lambda + // sets it to 0f when the player is acquired, so the controller arrives at 0f. + // Read first and only push an IPC if the value actually needs to change — + // with several feed videos preloading at once each volume= write was a + // round-trip to the service for nothing. + val targetVolume = + when { + BackgroundMedia.isPlaying() -> 0f + muted -> 0f + else -> 1f + } + if (state.controller.volume != targetVolume) { + state.controller.volume = targetVolume + Log.d("PlaybackService") { "OnEach volume=$targetVolume" } } if (play) { state.controller.playWhenReady = true } - state.controller.setMediaItem(mediaItem.item) - state.controller.prepare() + // Warm-pool fast path: when the underlying ExoPlayer was retained paused-with- + // buffer for this exact MediaItem, the MediaController's local mirror already + // shows the matching mediaId. Calling setMediaItem in that case would reset the + // player and discard the buffer — exactly what the warm pool exists to avoid. + // We still re-prepare if the player ended up IDLE somehow (e.g. it was demoted + // to cold and resurfaced, or hit an error before we attached). + val targetMediaId = mediaItem.item.mediaId + val needsLoad = state.controller.currentMediaItem?.mediaId != targetMediaId + if (needsLoad) { + state.controller.setMediaItem(mediaItem.item) + state.controller.prepare() + } else if (state.controller.playbackState == Player.STATE_IDLE) { + Log.d("PlaybackService") { "Warm controller in STATE_IDLE — re-preparing" } + state.controller.prepare() + } } - }.collectAsStateWithLifecycle(null) + }.collectAsState(null) controllerState?.let { inner(it) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/ExoPlayerPool.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/ExoPlayerPool.kt index 962ab5331..dbdc7f101 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/ExoPlayerPool.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/ExoPlayerPool.kt @@ -42,10 +42,35 @@ import java.util.concurrent.atomic.AtomicBoolean class ExoPlayerPool( val builder: ExoPlayerBuilder, private val poolSize: Int, + // Requested ceiling on paused-with-buffer players retained across releases. Each retained + // player keeps its decoder and LoadControl buffer alive, which costs both memory and a + // MediaCodec instance, so warm slots count against the same [poolSize] codec budget as + // cold players (see [warmSlotsCap]). Default 3 keeps the most recent few feed videos hot + // for scroll-back without monopolizing the device's decoder pool. + requestedWarmSlots: Int = DEFAULT_WARM_SLOTS, ) { - private val playerPool = ConcurrentLinkedQueue() + // Cap warm slots at poolSize-1 so there's always at least one slot available for a cold + // (cleared) player; otherwise a feed full of unique URIs would starve the cold pool and + // every new URI would force a fresh ExoPlayer build. + private val warmSlotsCap = requestedWarmSlots.coerceAtMost((poolSize - 1).coerceAtLeast(0)) + + // Idle players that have been stop()'d and clearMediaItems()'d — ready to be re-prepared + // with any URI. Maintained as a FIFO so the oldest cleared instance is reused first. + private val coldPool = ConcurrentLinkedQueue() private val poolStartingSize = 3 + // Most-recent paused players, indexed by the mediaId of the MediaItem they still hold. + // ArrayDeque is used as an LRU: head = oldest, tail = newest. Access is guarded by + // [warmPoolLock] (a plain monitor, since both acquire and release callers run on the + // service's main thread but we don't want to require the suspending [mutex] in acquire). + private data class WarmPlayer( + val mediaId: String, + val player: ExoPlayer, + ) + + private val warmPool = ArrayDeque(warmSlotsCap.coerceAtLeast(1)) + private val warmPoolLock = Any() + // Exists to avoid exceptions stopping the coroutine val exceptionHandler = CoroutineExceptionHandler { _, throwable -> @@ -70,8 +95,8 @@ class ExoPlayerPool( fun create(context: Context) { if (!warmupStarted.compareAndSet(false, true)) return scope.launch { - while (playerPool.size < poolStartingSize) { - playerPool.offer(builder.build(context)) + while (coldPool.size < poolStartingSize) { + coldPool.offer(builder.build(context)) // Hand the frame back so an in-flight onGetSession / acquirePlayer / layout // pass isn't blocked behind the next build. yield() @@ -79,15 +104,40 @@ class ExoPlayerPool( } } - fun acquirePlayer(context: Context): ExoPlayer { - if (playerPool.isEmpty()) { - // If the pool is empty, create a new player (or handle it differently) - return builder.build(context) + /** + * Acquire a player. When [preferredMediaId] matches a warm entry, returns that player intact + * — it still holds its MediaItem and any populated LoadControl buffer, so the caller can + * skip [androidx.media3.common.Player.setMediaItem] / [androidx.media3.common.Player.prepare] + * and resume immediately. Falls back to a cold (cleared) player or a freshly built one. + */ + fun acquirePlayer( + context: Context, + preferredMediaId: String? = null, + ): ExoPlayer { + if (preferredMediaId != null) { + val warm = takeWarm(preferredMediaId) + if (warm != null) { + Log.d("PlaybackService") { "ExoPlayerPool warm hit: $preferredMediaId" } + return warm + } } - - return playerPool.poll() ?: builder.build(context) + return coldPool.poll() ?: builder.build(context) } + private fun takeWarm(mediaId: String): ExoPlayer? = + synchronized(warmPoolLock) { + // Iterate from the newest end so a duplicated URI returns the freshest player. + val it = warmPool.listIterator(warmPool.size) + while (it.hasPrevious()) { + val entry = it.previous() + if (entry.mediaId == mediaId) { + it.remove() + return@synchronized entry.player + } + } + null + } + fun releasePlayerAsync(player: ExoPlayer) { scope.launch { releasePlayer(player) @@ -96,27 +146,70 @@ class ExoPlayerPool( suspend fun releasePlayer(player: ExoPlayer) { mutex.withLock { - if (!player.isReleased) { + if (player.isReleased) return@withLock + + val mediaId = player.currentMediaItem?.mediaId + if (mediaId != null && warmSlotsCap > 0) { + // Warm path: keep the player paused but loaded so a quick scroll-back to the + // same video resumes from the existing buffer instead of re-fetching from disk + // cache and re-priming the decoder. player.pause() - player.stop() - player.clearVideoSurface() - player.clearMediaItems() - - // Clear any video quality overrides so the next video starts with Auto - player.trackSelectionParameters = - player.trackSelectionParameters - .buildUpon() - .clearOverridesOfType(C.TRACK_TYPE_VIDEO) - .build() - - if (playerPool.size < poolSize) { - if (!playerPool.contains(player)) { - playerPool.add(player) - } - } else { - player.release() // Release if pool is full. + val evicted = pushWarm(mediaId, player) + if (evicted != null) { + Log.d("PlaybackService") { "ExoPlayerPool warm evict: ${evicted.mediaId}" } + demoteToCold(evicted.player) } + return@withLock } + + demoteToCold(player) + } + } + + private fun pushWarm( + mediaId: String, + player: ExoPlayer, + ): WarmPlayer? = + synchronized(warmPoolLock) { + // If the same URI is already warm (rare — duplicate VideoView in another scroller), + // drop the older entry so it can be demoted; the freshest copy wins. + val duplicate = warmPool.indexOfFirst { it.mediaId == mediaId } + val displaced = + if (duplicate >= 0) { + warmPool.removeAt(duplicate) + } else if (warmPool.size >= warmSlotsCap) { + warmPool.removeFirst() + } else { + null + } + warmPool.addLast(WarmPlayer(mediaId, player)) + displaced + } + + private fun demoteToCold(player: ExoPlayer) { + if (player.isReleased) return + player.pause() + player.stop() + player.clearVideoSurface() + player.clearMediaItems() + + // Clear any video quality overrides so the next video starts with Auto + player.trackSelectionParameters = + player.trackSelectionParameters + .buildUpon() + .clearOverridesOfType(C.TRACK_TYPE_VIDEO) + .build() + + // Total idle (cold + warm) must respect the device-derived poolSize cap so we don't + // exceed the MediaCodec instance budget. Warm slots get first dibs; cold gets the rest. + val warmSize = synchronized(warmPoolLock) { warmPool.size } + val coldCap = (poolSize - warmSize).coerceAtLeast(0) + if (coldPool.size < coldCap) { + if (!coldPool.contains(player)) { + coldPool.add(player) + } + } else { + player.release() // Release if pool is full. } } @@ -124,11 +217,22 @@ class ExoPlayerPool( scope .launch { mutex.withLock { - playerPool.forEach { it.release() } - playerPool.clear() + val warmSnapshot = + synchronized(warmPoolLock) { + val copy = warmPool.toList() + warmPool.clear() + copy + } + warmSnapshot.forEach { it.player.release() } + coldPool.forEach { it.release() } + coldPool.clear() } }.invokeOnCompletion { scope.cancel() } } + + companion object { + private const val DEFAULT_WARM_SLOTS = 3 + } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/MediaSessionPool.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/MediaSessionPool.kt index 501354b11..cda75c6ce 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/MediaSessionPool.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/MediaSessionPool.kt @@ -37,13 +37,14 @@ import com.google.common.util.concurrent.Futures import com.google.common.util.concurrent.ListenableFuture import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.MediaItemCache import com.vitorpamplona.amethyst.ui.MainActivity -import com.vitorpamplona.quartz.utils.TimeUtils import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel import kotlinx.coroutines.launch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicLong class SessionListener( val session: MediaSession, @@ -72,7 +73,22 @@ class MediaSessionPool( private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Main + exceptionHandler) val globalCallback = MediaSessionCallback(this, appContext) - var lastCleanup = TimeUtils.now() + + // Last cleanup timestamp in nanos, guarded by CAS so concurrent releaseSession() calls + // can't all win the time check and each launch a redundant scope.launch sweep. + private val lastCleanupNs = AtomicLong(System.nanoTime()) + + // The bitmap loader is stateless w.r.t. the session; a fresh allocation per session was + // pure noise. ExoPlayer's DEFAULT_EXECUTOR_SERVICE is a process-wide singleton, the + // dataSourceFactory is owned by the pool, and the appContext is already retained. + @OptIn(UnstableApi::class) + private val sharedBitmapLoader by lazy { + DataSourceBitmapLoader + .Builder(appContext) + .setExecutorService(DataSourceBitmapLoader.DEFAULT_EXECUTOR_SERVICE.get()) + .setDataSourceFactory(dataSourceFactory) + .build() + } // protects from LruCache killing playing sessions private val playingMap = mutableMapOf() @@ -102,18 +118,16 @@ class MediaSessionPool( id: String, keepPlaying: Boolean, context: Context, + // Best-effort affinity hint: when the pool still has a paused player carrying this + // exact mediaId (matches MediaItem.mediaId, which is the videoUri), the warm player + // is reused so the populated buffer survives. Null falls back to a cold acquire. + preferredMediaId: String?, ): MediaSession { val mediaSession = MediaSession - .Builder(context, exoPlayerPool.acquirePlayer(context)) + .Builder(context, exoPlayerPool.acquirePlayer(context, preferredMediaId)) .apply { - setBitmapLoader( - DataSourceBitmapLoader - .Builder(context) - .setExecutorService(DataSourceBitmapLoader.DEFAULT_EXECUTOR_SERVICE.get()) - .setDataSourceFactory(dataSourceFactory) - .build(), - ) + setBitmapLoader(sharedBitmapLoader) setId(id) setCallback(globalCallback) }.build() @@ -142,16 +156,17 @@ class MediaSessionPool( } fun cleanupUnused() { - if (lastCleanup < TimeUtils.oneMinuteAgo()) { - lastCleanup = TimeUtils.now() - scope.launch { - val snap = cache.snapshot() - snap.values.forEach { - if (it.session.connectedControllers.isEmpty()) { - releaseSession(it.session) - } + val now = System.nanoTime() + val previous = lastCleanupNs.get() + if (now - previous < CLEANUP_INTERVAL_NS) return + // CAS so only one caller actually launches the sweep when many releases fire at once. + if (!lastCleanupNs.compareAndSet(previous, now)) return + scope.launch { + val snap = cache.snapshot() + snap.values.forEach { + if (it.session.connectedControllers.isEmpty()) { + releaseSession(it.session) } - lastCleanup = TimeUtils.now() } } } @@ -175,13 +190,14 @@ class MediaSessionPool( id: String, keepPlaying: Boolean, context: Context, + preferredMediaId: String? = null, ): MediaSession { val existingSession = playingMap.get(id) ?: cache.get(id) if (existingSession != null) { return existingSession.session } - return newSession(id, keepPlaying, context) + return newSession(id, keepPlaying, context, preferredMediaId) } fun playingContent() = playingMap.values @@ -234,4 +250,8 @@ class MediaSessionPool( } } } + + companion object { + private val CLEANUP_INTERVAL_NS = TimeUnit.MINUTES.toNanos(1) + } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/positions/CurrentPlayPositionCacher.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/positions/CurrentPlayPositionCacher.kt index 837f85971..4734d7a69 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/positions/CurrentPlayPositionCacher.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/positions/CurrentPlayPositionCacher.kt @@ -66,7 +66,14 @@ class CurrentPlayPositionCacher( Player.STATE_READY -> { if (!isLiveStreaming) { cache.get(uri)?.let { lastPosition -> - if (abs(player.currentPosition - lastPosition) > 5 * 60) { + // Restore the saved position only if it's meaningfully far from + // the player's current position. Position values are in + // milliseconds, so the previous `5 * 60` constant was a 300 ms + // threshold — small enough to trigger a seek (and an extra buffer + // flush right at playback start) for almost any saved position. + // 5 s gives the user a perceptible "resumed where I left off" + // without forcing a re-seek for trivially short clips. + if (abs(player.currentPosition - lastPosition) > 5_000) { player.seekTo(lastPosition) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackService.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackService.kt index f323e01bc..795f04157 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackService.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackService.kt @@ -204,7 +204,17 @@ class PlaybackService : MediaSessionService() { val id = controllerInfo.connectionHints.getString("id") ?: return null val proxyPort = controllerInfo.connectionHints.getInt("proxyPort") val keepPlaying = controllerInfo.connectionHints.getBoolean("keepPlaying", true) + // Optional warm-pool affinity hint: when the pool still has a paused ExoPlayer + // holding this exact URI, the new session reuses it so the buffer survives. + val preferredMediaId = controllerInfo.connectionHints.getString(HINT_VIDEO_URI) val manager = lazyPool(proxyPort) - return manager.getSession(id, keepPlaying, applicationContext) + return manager.getSession(id, keepPlaying, applicationContext, preferredMediaId) + } + + companion object { + const val HINT_ID = "id" + const val HINT_PROXY_PORT = "proxyPort" + const val HINT_KEEP_PLAYING = "keepPlaying" + const val HINT_VIDEO_URI = "videoUri" } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackServiceClient.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackServiceClient.kt index 3bd87f509..aa74bfaf1 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackServiceClient.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackServiceClient.kt @@ -56,11 +56,15 @@ object PlaybackServiceClient { Bundle().apply { // link the id with the client's id to make sure it can return the // same session on background media. - putString("id", id) - putBoolean("keepPlaying", keepPlaying) + putString(PlaybackService.HINT_ID, id) + putBoolean(PlaybackService.HINT_KEEP_PLAYING, keepPlaying) proxyPort?.let { - putInt("proxyPort", it) + putInt(PlaybackService.HINT_PROXY_PORT, it) } + // Carry the URI so the service can ask the player pool for an existing warm + // (paused-with-buffer) ExoPlayer that already holds this MediaItem. Falls back + // gracefully — if no warm match exists, the pool returns a cold player. + putString(PlaybackService.HINT_VIDEO_URI, videoUri) } val session = SessionToken(appContext, ComponentName(appContext, PlaybackService::class.java)) From c6b275e7fea5b3743eb0f9c1e032c3c43d7084bb Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Apr 2026 12:37:56 +0000 Subject: [PATCH 4/8] perf(video): tighten remember keys and stabilize controller-overlay tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-4 audit cleanups. Each item is small but each runs on the hot path that recomposes during every active video, so they add up while scrolling. P1 — DimensionTag identity invalidating remember: - DimensionTag (in quartz) is a regular class with no equals override, so reference equality means a freshly parsed tag for the same event is != to the previous one. The remember(videoUri, dimensions) blocks added in the earlier perf commits were re-running their lambda on every recompose. Switch to primitive (width, height) keys in VideoView and GifVideoView so the cache lookups + MediaAspectRatioCache writes only fire when the dimensions actually change. P1 — Static gradient brushes: - TopGradientOverlay / BottomGradientOverlay were calling Brush.verticalGradient(colors = colors) inside the modifier chain, which allocated a fresh Brush on every recomposition while the controllers were visible (i.e. on every active video most of the time). Pre-build both brushes as file-level vals so they're allocated exactly once per process. P2 — ImmutableList for action collections: - RenderTopButtons / AnimatedOverflowMenuButton / OverflowMenuButton were passing List across composable boundaries. Plain List is unstable in Compose, forcing the overflow tree to recompose any time an unrelated parent state (volume, tracks, controllerVisible) ticked. Use ImmutableList end-to-end via toImmutableList() at the producer side. P2 — videoPlayerButtonItemsFlow remember: - accountViewModel.videoPlayerButtonItemsFlow() was being called fresh every recomposition, with the result handed straight to collectAsStateWithLifecycle. Hoist the call into remember(accountViewModel) so the flow reference is stable. P2 — MuteButton dispatcher cleanup: - The 2-second hold timer was using LaunchedEffect { launch(Dispatchers.IO) { delay(2000); holdOn.value = false } }. The wrapped launch was just redundant dispatcher hopping — delay() doesn't hold a thread and the Compose write is fine on Main. Inline it. --- .../service/playback/composable/VideoView.kt | 15 +++++--- .../composable/controls/GradientOverlay.kt | 37 ++++++++++++------- .../composable/controls/MuteButton.kt | 11 +++--- .../composable/controls/OverflowMenu.kt | 8 ++-- .../composable/controls/RenderTopButtons.kt | 11 +++++- .../amethyst/ui/components/GifVideoView.kt | 14 ++++++- 6 files changed, 65 insertions(+), 31 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoView.kt index 0160a1246..d7263930d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoView.kt @@ -115,13 +115,16 @@ fun VideoView( // Resolve the aspect ratio once per composition. Prime the URL-keyed cache from the imeta // dim tag so the next time this video appears (PiP, dialog, list re-enter) the cache hits - // without waiting for ExoPlayer's onVideoSizeChanged. + // without waiting for ExoPlayer's onVideoSizeChanged. Keys are primitive width/height so + // a freshly parsed DimensionTag instance for the same event doesn't re-run this lambda — + // DimensionTag uses reference equality, not structural. + val dimW = dimensions?.width + val dimH = dimensions?.height val ratio = - remember(videoUri, dimensions) { - val fromDim = dimensions?.takeIf { it.hasSize() } - if (fromDim != null) { - MediaAspectRatioCache.add(videoUri, fromDim.width, fromDim.height) - fromDim.aspectRatio() + remember(videoUri, dimW, dimH) { + if (dimW != null && dimH != null && dimW > 0 && dimH > 0) { + MediaAspectRatioCache.add(videoUri, dimW, dimH) + dimW.toFloat() / dimH.toFloat() } else { MediaAspectRatioCache.get(videoUri) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/GradientOverlay.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/GradientOverlay.kt index 938aa8ef9..be5d946f8 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/GradientOverlay.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/GradientOverlay.kt @@ -38,24 +38,33 @@ import androidx.compose.ui.unit.dp private val FadeIn = fadeIn() private val FadeOut = fadeOut() -private val TopGradientColors = - listOf( - Color.Black.copy(alpha = 0.6f), - Color.Black.copy(alpha = 0.3f), - Color.Transparent, +// Both gradient brushes are static; pre-build them once at class init so we don't allocate a +// new Brush on every recomposition while the controllers are visible (which is most of the +// time during playback / interaction). +private val TopGradientBrush = + Brush.verticalGradient( + colors = + listOf( + Color.Black.copy(alpha = 0.6f), + Color.Black.copy(alpha = 0.3f), + Color.Transparent, + ), ) -private val BottomGradientColors = - listOf( - Color.Transparent, - Color.Black.copy(alpha = 0.4f), - Color.Black.copy(alpha = 0.7f), +private val BottomGradientBrush = + Brush.verticalGradient( + colors = + listOf( + Color.Transparent, + Color.Black.copy(alpha = 0.4f), + Color.Black.copy(alpha = 0.7f), + ), ) @Composable private fun GradientOverlay( controllerVisible: State, - colors: List, + brush: Brush, height: Dp, modifier: Modifier = Modifier, ) { @@ -70,7 +79,7 @@ private fun GradientOverlay( Modifier .fillMaxWidth() .height(height) - .background(brush = Brush.verticalGradient(colors = colors)), + .background(brush = brush), ) } } @@ -80,11 +89,11 @@ fun TopGradientOverlay( controllerVisible: State, modifier: Modifier = Modifier, height: Dp = 80.dp, -) = GradientOverlay(controllerVisible, TopGradientColors, height, modifier) +) = GradientOverlay(controllerVisible, TopGradientBrush, height, modifier) @Composable fun BottomGradientOverlay( controllerVisible: State, modifier: Modifier = Modifier, height: Dp = 120.dp, -) = GradientOverlay(controllerVisible, BottomGradientColors, height, modifier) +) = GradientOverlay(controllerVisible, BottomGradientBrush, height, modifier) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/MuteButton.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/MuteButton.kt index 5c44ba3c8..787c7e30a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/MuteButton.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/MuteButton.kt @@ -47,9 +47,7 @@ import com.vitorpamplona.amethyst.ui.theme.Size30Modifier import com.vitorpamplona.amethyst.ui.theme.Size50Modifier import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn import com.vitorpamplona.amethyst.ui.theme.VolumeBottomIconSize -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay -import kotlinx.coroutines.launch @Preview @Composable @@ -79,11 +77,12 @@ fun MuteButton( ) } + // LaunchedEffect already runs on Main, and delay() suspends without holding a thread, so + // the previous launch(Dispatchers.IO) was just unnecessary dispatcher hopping for a state + // mutation that's also fine on Main. LaunchedEffect(key1 = controllerVisible) { - launch(Dispatchers.IO) { - delay(2000) - holdOn.value = false - } + delay(2000) + holdOn.value = false } val mutedInstance = remember(startingMuteState) { mutableStateOf(startingMuteState) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/OverflowMenu.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/OverflowMenu.kt index b8a115a5c..1bf719a92 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/OverflowMenu.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/OverflowMenu.kt @@ -50,6 +50,8 @@ import com.vitorpamplona.amethyst.ui.theme.Size20Modifier import com.vitorpamplona.amethyst.ui.theme.Size50Modifier import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn import com.vitorpamplona.amethyst.ui.theme.VolumeBottomIconSize +import kotlinx.collections.immutable.ImmutableList +import kotlinx.collections.immutable.persistentListOf private val FadeIn = fadeIn() private val FadeOut = fadeOut() @@ -60,7 +62,7 @@ fun OverflowMenuButtonPreview() { ThemeComparisonColumn { Box(Modifier.background(BitcoinOrange)) { OverflowMenuButton( - actions = listOf(VideoPlayerAction.Share, VideoPlayerAction.Download, VideoPlayerAction.PictureInPicture), + actions = persistentListOf(VideoPlayerAction.Share, VideoPlayerAction.Download, VideoPlayerAction.PictureInPicture), startingMuteState = false, onFullscreenClick = {}, onMuteClick = {}, @@ -76,7 +78,7 @@ fun OverflowMenuButtonPreview() { @Composable fun AnimatedOverflowMenuButton( controllerVisible: State, - actions: List, + actions: ImmutableList, startingMuteState: Boolean, onFullscreenClick: (() -> Unit)?, onMuteClick: () -> Unit, @@ -107,7 +109,7 @@ fun AnimatedOverflowMenuButton( @Composable fun OverflowMenuButton( - actions: List, + actions: ImmutableList, startingMuteState: Boolean, onFullscreenClick: (() -> Unit)?, onMuteClick: () -> Unit, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt index d7c1645f0..5d182bd84 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt @@ -70,6 +70,7 @@ import com.vitorpamplona.amethyst.ui.theme.PinBottomIconSize import com.vitorpamplona.amethyst.ui.theme.Size20Modifier import com.vitorpamplona.amethyst.ui.theme.Size50Modifier import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn +import kotlinx.collections.immutable.toImmutableList @Preview @Composable @@ -194,7 +195,10 @@ fun RenderTopButtons( modifier: Modifier, accountViewModel: AccountViewModel, ) { - val buttonItems by accountViewModel.videoPlayerButtonItemsFlow().collectAsStateWithLifecycle() + // Hold the StateFlow itself across recompositions so collectAsStateWithLifecycle isn't + // keyed on the result of a property getter call that happens every recompose. + val buttonItemsFlow = remember(accountViewModel) { accountViewModel.videoPlayerButtonItemsFlow() } + val buttonItems by buttonItemsFlow.collectAsStateWithLifecycle() val shareDialogVisible = remember { mutableStateOf(false) } val saveAction = rememberSaveMediaAction { context -> @@ -212,17 +216,22 @@ fun RenderTopButtons( } val canFullscreen = onZoomClick != null + // ImmutableList so Compose can treat the action lists as stable parameters when they're + // passed through to AnimatedOverflowMenuButton — a plain List is unstable and forces the + // overflow tree to recompose whenever any unrelated parent state ticks. val topBarActions = remember(buttonItems, canFullscreen, hasMultipleQualities, isLive, pipSupported) { buttonItems .filter { it.location == VideoButtonLocation.TopBar && isAvailable(it.action) } .map { it.action } + .toImmutableList() } val overflowActions = remember(buttonItems, canFullscreen, hasMultipleQualities, isLive, pipSupported) { buttonItems .filter { it.location == VideoButtonLocation.OverflowMenu && isAvailable(it.action) } .map { it.action } + .toImmutableList() } Row(modifier) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/GifVideoView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/GifVideoView.kt index 1b33389f0..e9bb15e63 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/GifVideoView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/GifVideoView.kt @@ -35,6 +35,7 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.SideEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -68,7 +69,18 @@ fun GifVideoView( accountViewModel: AccountViewModel, thumbhash: String? = null, ) { - val ratio = dimensions?.aspectRatio() ?: MediaAspectRatioCache.get(videoUri) + // Keys are primitive width/height so a freshly parsed DimensionTag instance for the same + // event doesn't re-run this lambda — DimensionTag uses reference equality, not structural. + val dimW = dimensions?.width + val dimH = dimensions?.height + val ratio = + remember(videoUri, dimW, dimH) { + if (dimW != null && dimH != null && dimH > 0) { + dimW.toFloat() / dimH.toFloat() + } else { + MediaAspectRatioCache.get(videoUri) + } + } val autoPlay = accountViewModel.settings.autoPlayVideos() val borderModifier = if (roundedCorner) MaterialTheme.colorScheme.imageModifier else Modifier val context = LocalContext.current From ba1a1bfc12f1193ff36a19572d9985218dd45f2d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Apr 2026 13:15:08 +0000 Subject: [PATCH 5/8] perf(video): shorten VideoCache warmup delay from 10s to 1.5s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing background warmup in Amethyst.initiate() deferred the lazy videoCache touch by 10 seconds. SimpleCache's constructor opens a SQLite index via StandaloneDatabaseProvider and walks every cached span on disk — a few hundred ms on a populated 4 GB cache — so we really do not want that running on the main thread. But 10 s is long enough that a fast user (or a deep link / push notification that lands directly on a video-bearing screen) can win the `lazy { }` race and trigger init on the main thread inside PlaybackService.onGetSession, which is exactly the hitch the warmup was meant to prevent. Drop to 1.5 s — long enough to let the urgent first-paint work above (account load, image loader, ui state, robohash) breathe, short enough that a typical user can't scroll and tap a video before the warmup wins. Document the trade-off in a comment so the timing isn't a magic number. --- .../java/com/vitorpamplona/amethyst/AppModules.kt | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt index 275ddffe0..5ea95f359 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt @@ -525,10 +525,16 @@ class AppModules( } } - // initializes diskcache on an IO thread. + // Warms the video cache off the main thread. SimpleCache's constructor opens a SQLite + // index over StandaloneDatabaseProvider and walks every cached span on disk — up to a + // few hundred ms on a populated 4 GB cache — so leaving it for the first session's + // onGetSession would do that work on the main thread. The short delay keeps the IO + // dispatcher free for the urgent first-paint work above (account load, image loader, + // ui state, robohash) while still landing the warmup well before a typical user can + // scroll to and tap a video. The previous 10 s delay was long enough that a fast user + // (or a deep link) could lose the lazy { } race and trigger main-thread init. applicationIOScope.launch { - // Prepares video cache later - delay(10_000) + delay(1_500) videoCache } } From d7bd78cc322dac172538a2b99d723642b8fd01cd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Apr 2026 13:55:53 +0000 Subject: [PATCH 6/8] chore(video): correctness and hygiene cleanups in playback layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-up of the small leftovers from the audit. None move the needle on their own; together they remove a real cancellation bug and tighten the playback types. - PlaybackServiceClient.executorService: Executors.newCachedThreadPool() → Executors.newSingleThreadExecutor(). The work per callback is Future.get() on an already-completed future plus a non-blocking trySend; a single thread is plenty. The previous unbounded pool could spin up a thread per concurrent video, each lingering for the 60 s keep-alive afterwards. - MediaControllerState.controller: var → val. The field was never reassigned anywhere (grep confirms), and a non-observable var on a @Stable class is a footgun — Compose can't see writes to a plain var, so any future write would silently miss recomposition. - MediaControllerState.currrentMedia() → currentMedia(). Typo. Updated the single caller in PipVideoView. - LoadThumbAndThenVideoView: real cancellation bug fix. The Coil fetch was launched into AccountViewModel.viewModelScope via a side helper (loadThumb), so a scroll-away didn't cancel the in-flight image request — wasted bandwidth and a late callback writing into stale state. Inline the Coil call into the LaunchedEffect's own scope so cancellation propagates, and key the effect on thumbUri so a recycled audio-track slot with a new cover doesn't stall on the prior Pair(true, ...) gate. Drop the now-unused AccountViewModel.loadThumb and its only-here imports. --- .../composable/LoadThumbAndThenVideoView.kt | 88 +++++++++---------- .../composable/MediaControllerState.kt | 5 +- .../service/playback/pip/PipVideoView.kt | 2 +- .../playback/service/PlaybackServiceClient.kt | 7 +- .../ui/screen/loggedIn/AccountViewModel.kt | 27 ------ 5 files changed, 52 insertions(+), 77 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/LoadThumbAndThenVideoView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/LoadThumbAndThenVideoView.kt index 9bcabaf0d..0189aa04a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/LoadThumbAndThenVideoView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/LoadThumbAndThenVideoView.kt @@ -29,7 +29,14 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalContext +import coil3.asDrawable +import coil3.imageLoader +import coil3.request.ImageRequest import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel +import com.vitorpamplona.quartz.utils.Log +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import kotlin.coroutines.cancellation.CancellationException @Composable fun LoadThumbAndThenVideoView( @@ -45,56 +52,47 @@ fun LoadThumbAndThenVideoView( accountViewModel: AccountViewModel, onDialog: (() -> Unit)? = null, ) { - var loadingFinished by remember { mutableStateOf>(Pair(false, null)) } + var loadingFinished by remember(thumbUri) { mutableStateOf>(Pair(false, null)) } val context = LocalContext.current - LaunchedEffect(Unit) { - accountViewModel.loadThumb( - context, - thumbUri, - onReady = { - loadingFinished = - if (it != null) { - Pair(true, it) - } else { - Pair(true, null) + // Run the Coil fetch in this LaunchedEffect's scope (was previously launched into the + // AccountViewModel's viewModelScope, which meant a scroll-away wouldn't cancel the in-flight + // image request — wasted bandwidth, plus the late callback wrote into a state that no + // longer mattered). Keying on thumbUri also makes the effect re-fire when a recycled slot + // gets a new audio track with a new cover instead of stalling on the stale Pair(true, ...). + LaunchedEffect(thumbUri) { + loadingFinished = + try { + val request = ImageRequest.Builder(context).data(thumbUri).build() + val drawable = + withContext(Dispatchers.IO) { + context.imageLoader + .execute(request) + .image + ?.asDrawable(context.resources) } - }, - onError = { loadingFinished = Pair(true, null) }, - ) + Pair(true, drawable) + } catch (e: Exception) { + if (e is CancellationException) throw e + Log.e("VideoView", "Fail to load cover $thumbUri", e) + Pair(true, null) + } } if (loadingFinished.first) { - if (loadingFinished.second != null) { - VideoView( - videoUri = videoUri, - mimeType = mimeType, - title = title, - thumb = VideoThumb(loadingFinished.second), - roundedCorner = roundedCorner, - contentScale = contentScale, - artworkUri = thumbUri, - authorName = authorName, - nostrUriCallback = nostrUriCallback, - isLiveStream = isLiveStream, - accountViewModel = accountViewModel, - onDialog = onDialog, - ) - } else { - VideoView( - videoUri = videoUri, - mimeType = mimeType, - title = title, - thumb = null, - roundedCorner = roundedCorner, - contentScale = contentScale, - artworkUri = thumbUri, - authorName = authorName, - nostrUriCallback = nostrUriCallback, - isLiveStream = isLiveStream, - accountViewModel = accountViewModel, - onDialog = onDialog, - ) - } + VideoView( + videoUri = videoUri, + mimeType = mimeType, + title = title, + thumb = loadingFinished.second?.let { VideoThumb(it) }, + roundedCorner = roundedCorner, + contentScale = contentScale, + artworkUri = thumbUri, + authorName = authorName, + nostrUriCallback = nostrUriCallback, + isLiveStream = isLiveStream, + accountViewModel = accountViewModel, + onDialog = onDialog, + ) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/MediaControllerState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/MediaControllerState.kt index 4a353a6fe..701ff382c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/MediaControllerState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/MediaControllerState.kt @@ -31,14 +31,13 @@ import kotlin.uuid.Uuid class MediaControllerState( // each composable has an ID. val id: String = Uuid.random().toString(), - // This is filled after the controller returns from this class - var controller: Player, + val controller: Player, // visibility onscreen val visibility: VisibilityData = VisibilityData(), ) { fun isPlaying() = controller.isPlaying - fun currrentMedia() = controller.currentMediaItem?.mediaId + fun currentMedia() = controller.currentMediaItem?.mediaId fun toggleMute() { controller.volume = if (controller.volume == 0f) 1f else 0f diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/pip/PipVideoView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/pip/PipVideoView.kt index 59a425981..aa9c54081 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/pip/PipVideoView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/pip/PipVideoView.kt @@ -58,7 +58,7 @@ fun RenderPipVideo( val modifier = remember { val ratio = - controller.currrentMedia()?.let { + controller.currentMedia()?.let { MediaAspectRatioCache.get(it) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackServiceClient.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackServiceClient.kt index aa74bfaf1..6ffdce4c6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackServiceClient.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackServiceClient.kt @@ -36,7 +36,12 @@ import kotlin.uuid.ExperimentalUuidApi import kotlin.uuid.Uuid object PlaybackServiceClient { - val executorService: ExecutorService = Executors.newCachedThreadPool() + // Runs the MediaController.buildAsync() completion callbacks. The work per callback is + // trivial — Future.get() on an already-completed future plus a non-blocking trySend into + // the callbackFlow channel — so a single thread is plenty. The previous newCachedThreadPool + // could spin up an unbounded number of threads when many videos appeared at once, each + // sticking around for the executor's keep-alive (60s) afterwards. + val executorService: ExecutorService = Executors.newSingleThreadExecutor() fun shutdown() { executorService.shutdown() diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index c944b071f..673454184 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -22,7 +22,6 @@ package com.vitorpamplona.amethyst.ui.screen.loggedIn import android.annotation.SuppressLint import android.content.Context -import android.graphics.drawable.Drawable import android.os.Handler import android.os.Looper import android.util.LruCache @@ -34,9 +33,6 @@ import androidx.compose.runtime.rememberCoroutineScope import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.viewModelScope -import coil3.asDrawable -import coil3.imageLoader -import coil3.request.ImageRequest import com.vitorpamplona.amethyst.AccountInfo import com.vitorpamplona.amethyst.Amethyst import com.vitorpamplona.amethyst.LocalPreferences @@ -1578,29 +1574,6 @@ class AccountViewModel( super.onCleared() } - fun loadThumb( - context: Context, - thumbUri: String, - onReady: (Drawable?) -> Unit, - onError: (String?) -> Unit, - ) { - viewModelScope.launch(Dispatchers.IO) { - try { - val request = ImageRequest.Builder(context).data(thumbUri).build() - val myCover = - context.imageLoader - .execute(request) - .image - ?.asDrawable(context.resources) - onReady(myCover) - } catch (e: Exception) { - if (e is CancellationException) throw e - Log.e("VideoView", "Fail to load cover $thumbUri", e) - onError(e.message) - } - } - } - fun loadMentions( mentions: ImmutableList, onReady: (ImmutableList) -> Unit, From 45fb5119e85f1ee00eebf568a2d67558e337ea92 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Apr 2026 14:25:25 +0000 Subject: [PATCH 7/8] revert(video): two cleanups from the round-4 pass that didn't earn their keep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - GifVideoView: revert the dimensions remember() to the original one-line expression. Unlike VideoView's equivalent block, GifVideoView only *reads* — there's no MediaAspectRatioCache.add() side effect to gate. The replaced code spent three slot reads + three equality checks per recompose to skip an int division and an LruCache.get(), neither of which allocates. It was a wash at best, a small loss at worst. The original is simpler and roughly the same cost. - PlaybackServiceClient: bump the executor from newSingleThreadExecutor() back up to newFixedThreadPool(4). The work per listener is genuinely trivial in the steady state, but a single thread leaves us exposed to one stuck listener (e.g. the defensive 5s controllerFuture.get() timeout actually firing) stalling every other video on screen behind it. With a feed often holding several visible videos at once, that's a real regression risk. A fixed pool of 4 keeps us bounded against churn while letting independent listeners proceed in parallel. --- .../playback/service/PlaybackServiceClient.kt | 13 ++++++++----- .../amethyst/ui/components/GifVideoView.kt | 18 +++++------------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackServiceClient.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackServiceClient.kt index 6ffdce4c6..d641cd8d0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackServiceClient.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackServiceClient.kt @@ -37,11 +37,14 @@ import kotlin.uuid.Uuid object PlaybackServiceClient { // Runs the MediaController.buildAsync() completion callbacks. The work per callback is - // trivial — Future.get() on an already-completed future plus a non-blocking trySend into - // the callbackFlow channel — so a single thread is plenty. The previous newCachedThreadPool - // could spin up an unbounded number of threads when many videos appeared at once, each - // sticking around for the executor's keep-alive (60s) afterwards. - val executorService: ExecutorService = Executors.newSingleThreadExecutor() + // trivial in the steady state (Future.get() on an already-completed future + a non-blocking + // trySend into this video's own callbackFlow channel), so the IPC bind itself dominates and + // happens on Media3's own threads regardless. We size the pool small enough to stay bounded + // under churn but parallel enough that one stuck listener (e.g. the defensive 5s get() + // timeout actually firing) can't stall the rest of the videos onscreen behind it. The + // original newCachedThreadPool was unbounded and could spin up a thread per concurrent + // video, each lingering for the 60s keep-alive afterwards. + val executorService: ExecutorService = Executors.newFixedThreadPool(4) fun shutdown() { executorService.shutdown() diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/GifVideoView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/GifVideoView.kt index e9bb15e63..b9b695a55 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/GifVideoView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/GifVideoView.kt @@ -35,7 +35,6 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.SideEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue -import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -69,18 +68,11 @@ fun GifVideoView( accountViewModel: AccountViewModel, thumbhash: String? = null, ) { - // Keys are primitive width/height so a freshly parsed DimensionTag instance for the same - // event doesn't re-run this lambda — DimensionTag uses reference equality, not structural. - val dimW = dimensions?.width - val dimH = dimensions?.height - val ratio = - remember(videoUri, dimW, dimH) { - if (dimW != null && dimH != null && dimH > 0) { - dimW.toFloat() / dimH.toFloat() - } else { - MediaAspectRatioCache.get(videoUri) - } - } + // Pure read path — DimensionTag.aspectRatio() is a one-line int division and + // MediaAspectRatioCache.get() is a synchronized LruCache lookup. Wrapping this in + // remember() to avoid the recompute would cost more (slot read + N equality checks) + // than the work it saves; that's why this stays as a plain expression. + val ratio = dimensions?.aspectRatio() ?: MediaAspectRatioCache.get(videoUri) val autoPlay = accountViewModel.settings.autoPlayVideos() val borderModifier = if (roundedCorner) MaterialTheme.colorScheme.imageModifier else Modifier val context = LocalContext.current From a8e7c6c598060c33b9a91e2d6090532cc768e6a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Apr 2026 14:27:22 +0000 Subject: [PATCH 8/8] revert(video): drop the videoPlayerButtonItemsFlow remember The remember(accountViewModel) { ... } I added was cargo-cult. The getter is just a chain of val property accesses (account.settings.syncedSettings.videoPlayer.buttonItems) returning the same StateFlow instance every call. collectAsStateWithLifecycle keys on that flow reference, which is identity-stable, so re-calling the getter on every recompose costs nothing meaningful and doesn't cause a re-subscription. Inline back to the original one-liner. --- .../service/playback/composable/controls/RenderTopButtons.kt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt index 5d182bd84..c694c688c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt @@ -195,10 +195,7 @@ fun RenderTopButtons( modifier: Modifier, accountViewModel: AccountViewModel, ) { - // Hold the StateFlow itself across recompositions so collectAsStateWithLifecycle isn't - // keyed on the result of a property getter call that happens every recompose. - val buttonItemsFlow = remember(accountViewModel) { accountViewModel.videoPlayerButtonItemsFlow() } - val buttonItems by buttonItemsFlow.collectAsStateWithLifecycle() + val buttonItems by accountViewModel.videoPlayerButtonItemsFlow().collectAsStateWithLifecycle() val shareDialogVisible = remember { mutableStateOf(false) } val saveAction = rememberSaveMediaAction { context ->