Merge pull request #3667 from vitorpamplona/fix/video-decoder-budget-and-aspect-ratio

fix(video): decoder-budget exhaustion and black bars on live streams

amethyst/src/main/java/com/vitorpamplona/amethyst/model/MediaAspectRatioCache.kt

@@ -21,6 +21,8 @@
 package com.vitorpamplona.amethyst.model
 
 import androidx.collection.LruCache
+import androidx.compose.runtime.MutableState
+import androidx.compose.runtime.mutableStateOf
 
 interface MutableMediaAspectRatioCache {
     fun get(url: String): Float?
@@ -32,10 +34,27 @@ interface MutableMediaAspectRatioCache {
     )
 }
 
+/**
+ * Aspect ratios keyed by media URL, learned from imeta `dim` tags up front or from the decoder once
+ * a first frame lands.
+ *
+ * Entries are snapshot state, so a composable that calls [get] **during composition** recomposes
+ * when the real dimensions arrive later. That matters because players and image loaders only report
+ * size after the first frame decodes: a caller that sized itself off a plain cache miss would stay
+ * wrong for the whole visit and only look right the *next* time the media is opened. Note this only
+ * works for reads made in composition — a read from inside `remember { }` is cached by `remember`
+ * itself and won't pick the update up.
+ */
 object MediaAspectRatioCache : MutableMediaAspectRatioCache {
-    val mediaAspectRatioCacheByUrl = LruCache<String, Float>(1000)
+    private val cache = LruCache<String, MutableState<Float?>>(1000)
 
-    override fun get(url: String): Float? = mediaAspectRatioCacheByUrl.get(url)
+    // get-then-put has to be atomic, so the compound op is guarded even though LruCache is itself
+    // thread-safe. A miss still stores a slot: that empty slot is what the caller observes until
+    // add() fills it in.
+    @Synchronized
+    private fun entry(url: String): MutableState<Float?> = cache.get(url) ?: mutableStateOf<Float?>(null).also { cache.put(url, it) }
+
+    override fun get(url: String): Float? = entry(url).value
 
     override fun add(
         url: String,
@@ -43,7 +62,7 @@ object MediaAspectRatioCache : MutableMediaAspectRatioCache {
         height: Int,
     ) {
         if (height > 1) {
-            mediaAspectRatioCacheByUrl.put(url, width.toFloat() / height.toFloat())
+            entry(url).value = width.toFloat() / height.toFloat()
         }
     }
 }

amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoView.kt

@@ -124,6 +124,11 @@ fun VideoView(
     // DimensionTag uses reference equality, not structural.
     val dimW = dimensions?.width
     val dimH = dimensions?.height
+    // Deliberately snapshotted in a remember rather than observing MediaAspectRatioCache: when the
+    // ratio flips null -> known mid-playback this branch both adds an aspectRatio and emits an
+    // extra Spacer, and restructuring the children around a live AndroidView leaves the player's
+    // TextureView on a stale surface (the video redraws at native size in the corner). The
+    // enclosing box in ZoomableContentView is what sizes the player, and that one does observe.
     val ratio =
         remember(videoUri, dimW, dimH) {
             if (dimW != null && dimH != null && dimW > 0 && dimH > 0) {

amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/ExoPlayerPool.kt

@@ -37,6 +37,7 @@ import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.yield
 import java.util.concurrent.ConcurrentLinkedQueue
 import java.util.concurrent.atomic.AtomicBoolean
+import java.util.concurrent.atomic.AtomicInteger
 
 @OptIn(UnstableApi::class)
 class ExoPlayerPool(
@@ -71,6 +72,10 @@ class ExoPlayerPool(
     private val warmPool = ArrayDeque<WarmPlayer>(warmSlotsCap.coerceAtLeast(1))
     private val warmPoolLock = Any()
 
+    init {
+        livePools.add(this)
+    }
+
     // Exists to avoid exceptions stopping the coroutine
     val exceptionHandler =
         CoroutineExceptionHandler { _, throwable ->
@@ -127,15 +132,53 @@ class ExoPlayerPool(
                     Log.d("PlaybackService") { "ExoPlayerPool discarding errored warm player: $preferredMediaId (${error.errorCodeName})" }
                     PcmTapRegistry.unregisterPlayer(warm)
                     warm.release()
+                    liveDecoders.decrementAndGet()
                 } else {
                     Log.d("PlaybackService") { "ExoPlayerPool warm hit: $preferredMediaId" }
+                    // Already counted against the decoder budget for as long as it sat warm.
                     return warm
                 }
             }
         }
+        ensureDecoderHeadroom()
+        liveDecoders.incrementAndGet()
         return coldPool.poll() ?: builder.build(context)
     }
 
+    /**
+     * Frees decoder headroom before a cold or freshly built player is handed out.
+     *
+     * Every player that still holds a prepared MediaItem — checked out or merely warm — owns a
+     * MediaCodec instance, and devices advertise a hard ceiling on those (the emulator's
+     * c2.goldfish.h264.decoder declares `concurrent-instances max="4"`). Past that ceiling
+     * MediaCodec.start() fails with NO_MEMORY and the video surfaces as "can't load", so the
+     * budget has to be enforced at acquisition rather than only at retention.
+     *
+     * Warm players are a scroll-back cache, so they are what gives way: demoting one to cold
+     * stop()s it and releases its codec. This pool's own entries go first, then any other pool's
+     * — [PlaybackService] keeps a separate pool for direct and for Tor-proxied traffic, and both
+     * draw on the one per-process pile of decoders.
+     */
+    private fun ensureDecoderHeadroom() {
+        while (liveDecoders.get() >= poolSize) {
+            if (!evictOldestWarm() && !evictOldestWarmElsewhere()) return
+        }
+    }
+
+    private fun evictOldestWarm(): Boolean {
+        val oldest = synchronized(warmPoolLock) { warmPool.removeFirstOrNull() } ?: return false
+        Log.d("PlaybackService") { "ExoPlayerPool decoder-budget evict: ${oldest.mediaId}" }
+        demoteToCold(oldest.player)
+        return true
+    }
+
+    private fun evictOldestWarmElsewhere(): Boolean {
+        livePools.forEach { pool ->
+            if (pool !== this && pool.evictOldestWarm()) return true
+        }
+        return false
+    }
+
     private fun takeWarm(mediaId: String): ExoPlayer? =
         synchronized(warmPoolLock) {
             // Iterate from the newest end so a duplicated URI returns the freshest player.
@@ -170,6 +213,7 @@ class ExoPlayerPool(
                 Log.d("PlaybackService") { "ExoPlayerPool dropping errored player: ${player.currentMediaItem?.mediaId} (${error.errorCodeName})" }
                 PcmTapRegistry.unregisterPlayer(player)
                 player.release()
+                liveDecoders.decrementAndGet()
                 return@withLock
             }
 
@@ -214,7 +258,10 @@ class ExoPlayerPool(
     private fun demoteToCold(player: ExoPlayer) {
         if (player.isReleased) return
         player.pause()
+        // stop() tears the renderers down, which is what actually hands the MediaCodec instance
+        // back to the system — so this is the point where the player stops costing budget.
         player.stop()
+        liveDecoders.decrementAndGet()
         player.clearVideoSurface()
         player.clearMediaItems()
 
@@ -260,6 +307,7 @@ class ExoPlayerPool(
     }
 
     fun destroy() {
+        livePools.remove(this)
         scope
             .launch {
                 mutex.withLock {
@@ -272,6 +320,7 @@ class ExoPlayerPool(
                     warmSnapshot.forEach {
                         PcmTapRegistry.unregisterPlayer(it.player)
                         it.player.release()
+                        liveDecoders.decrementAndGet()
                     }
                     coldPool.forEach {
                         PcmTapRegistry.unregisterPlayer(it)
@@ -286,5 +335,17 @@ class ExoPlayerPool(
 
     companion object {
         private const val DEFAULT_WARM_SLOTS = 3
+
+        // MediaCodec instances are a per-process resource, but PlaybackService builds one pool for
+        // direct traffic and another for Tor-proxied traffic, so a per-pool budget would let the
+        // app hold twice the device's decoder ceiling. Both counters below are therefore global.
+
+        // Players currently holding a decoder: checked out, or warm (paused but still prepared).
+        // Cold players have been stop()'d and own none.
+        private val liveDecoders = AtomicInteger(0)
+
+        // Every pool that hasn't been destroy()'d, so a pool starved of headroom can reclaim a
+        // warm player from a sibling instead of overshooting the shared ceiling.
+        private val livePools = ConcurrentLinkedQueue<ExoPlayerPool>()
     }
 }

amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/MediaSessionPool.kt

@@ -64,6 +64,10 @@ class MediaSessionPool(
     val exoPlayerPool: ExoPlayerPool,
     val dataSourceFactory: DataSource.Factory,
     val appContext: Context,
+    // Ceiling on cached sessions. Each one holds a checked-out ExoPlayer, so on a device whose
+    // decoder ceiling is lower than [MAX_CACHED_SESSIONS] this is what keeps the session cache
+    // from pinning more MediaCodec instances than the hardware will grant.
+    maxSessions: Int = MAX_CACHED_SESSIONS,
     val reset: (MediaSession, Boolean) -> Unit,
 ) {
     private val exceptionHandler =
@@ -123,7 +127,7 @@ class MediaSessionPool(
     private val playingMap = mutableMapOf<String, SessionListener>()
 
     private val cache =
-        object : LruCache<String, SessionListener>(10) { // up to 10 videos in the screen at the same time
+        object : LruCache<String, SessionListener>(maxSessions.coerceIn(1, MAX_CACHED_SESSIONS)) {
             override fun entryRemoved(
                 evicted: Boolean,
                 key: String?,
@@ -296,6 +300,10 @@ class MediaSessionPool(
     companion object {
         private val CLEANUP_INTERVAL_NS = TimeUnit.MINUTES.toNanos(1)
 
+        // Roughly how many videos can share a screen at once. Acts as the upper bound only —
+        // a device that advertises fewer concurrent decoders than this caps lower.
+        const val MAX_CACHED_SESSIONS = 10
+
         // AOSP default for config_mediaMetadataBitmapMaxSize, used when the framework resource
         // can't be resolved by name on a given ROM.
         private const val DEFAULT_METADATA_BITMAP_DP = 320

amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/service/PlaybackService.kt

@@ -80,14 +80,20 @@ class PlaybackService : MediaSessionService() {
                 },
             )
 
+        // The device's concurrent-decoder ceiling bounds both how many players may be checked out
+        // at once (the session cache) and how many the pool may retain, since a session and a warm
+        // pool entry each pin one MediaCodec instance.
+        val decoderBudget = SimultaneousPlaybackCalculator.max(applicationContext)
+
         return MediaSessionPool(
             exoPlayerPool =
                 ExoPlayerPool(
                     ExoPlayerBuilder(videoCache, resolvingDataSourceFactory),
-                    poolSize = SimultaneousPlaybackCalculator.max(applicationContext),
+                    poolSize = decoderBudget,
                 ),
             dataSourceFactory = resolvingDataSourceFactory,
             appContext = applicationContext,
+            maxSessions = decoderBudget,
             reset = { session, keepPlaying ->
                 (session.player as ExoPlayer).apply {
                     repeatMode = if (keepPlaying) Player.REPEAT_MODE_ONE else Player.REPEAT_MODE_OFF

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt

@@ -136,6 +136,14 @@ import java.io.IOException
 // Allows time for receiving app to copy the file after user confirms share.
 private const val SHARED_VIDEO_CLEANUP_DELAY_MS = 120_000L
 
+// Assumed shape of a video whose dimensions nobody has reported yet — no imeta `dim` and nothing
+// cached, which is the norm for a NIP-53 live stream on its first play. Without a ratio the sizing
+// modifier leaves height unconstrained, so the player stretches to whatever ceiling encloses it
+// (300.dp on the live-stream screen) and letterboxes the real frame inside, leaving black bars top
+// and bottom. Guessing the overwhelmingly common video shape puts the first layout in the right
+// place; [MediaAspectRatioCache] then corrects anything unusual once the decoder reports its size.
+private const val DEFAULT_VIDEO_ASPECT_RATIO = 16f / 9f
+
 @Composable
 fun ZoomableContentView(
     content: BaseMediaContent,
@@ -195,7 +203,7 @@ fun ZoomableContentView(
         }
 
         is MediaUrlVideo -> {
-            val ratio = content.dim?.aspectRatio() ?: MediaAspectRatioCache.get(content.url)
+            val ratio = content.dim?.aspectRatio() ?: MediaAspectRatioCache.get(content.url) ?: DEFAULT_VIDEO_ASPECT_RATIO
             val bridgedUrl =
                 remember(content.url, useLocalBlossomBridge) {
                     content.toCoilModel(useLocalBlossomBridge)
@@ -209,7 +217,13 @@ fun ZoomableContentView(
                 backdrop = (content.thumbhash ?: content.blurhash)?.let { { BlurhashBackdrop(content.blurhash, content.description, content.thumbhash) } },
             ) {
                 Box(
-                    modifier = Modifier.fillMaxWidth().then(boundsTrackingModifier),
+                    // The sizing modifier is repeated here because ContentWarningGate only applies
+                    // the one it is handed when the content is actually sensitive — the common
+                    // non-sensitive path emits content() bare. Without a height constraint of its
+                    // own this box stretches to whatever ceiling encloses it and the player
+                    // letterboxes the frame inside, which is what put black bars above and below
+                    // live streams (their enclosure is StreamingHeaderModifier's 300.dp cap).
+                    modifier = mediaSizingModifier(ratio, contentScale).then(boundsTrackingModifier),
                     contentAlignment = Alignment.Center,
                 ) {
                     VideoView(