From 45fb5119e85f1ee00eebf568a2d67558e337ea92 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 26 Apr 2026 14:25:25 +0000
Subject: [PATCH] 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 6ffdce4c6f..d641cd8d0c 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 e9bb15e63a..b9b695a559 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