Merge branch 'develop' into fix/subtitles-ordering-fix

Fixed an issue where external and subrip subtitles were not ordered
correctly

Signed-off-by: Lance Chant <13349722+lancechant@users.noreply.github.com>

modules/mpv-player/android/src/main/java/expo/modules/mpvplayer/MpvPlayerView.kt

@@ -2,12 +2,14 @@ package expo.modules.mpvplayer
 
 import android.content.Context
 import android.graphics.Color
+import android.graphics.Rect
+import android.graphics.SurfaceTexture
 import android.os.Handler
 import android.os.Looper
 import android.util.Log
 import android.view.Surface
-import android.view.SurfaceHolder
-import android.view.SurfaceView
+import android.view.TextureView
+import android.view.View
 import android.view.ViewGroup
 import expo.modules.kotlin.AppContext
 import expo.modules.kotlin.viewevent.EventDispatcher
@@ -28,26 +30,15 @@ data class VideoLoadConfig(
     val cacheEnabled: String? = null,
     val cacheSeconds: Int? = null,
     val demuxerMaxBytes: Int? = null,
-    val demuxerMaxBackBytes: Int? = null,
+    val demuxerMaxBackBytes: Int? = null
 )
 
 /**
  * MpvPlayerView - ExpoView that hosts the MPV player.
- *
- * Uses SurfaceView (not TextureView) so the surface routes directly to
- * SurfaceFlinger (the OS compositor) rather than compositing into the
- * app's window surface. This matches mpv-android's architecture and
- * gives mpv a standalone surface.
- *
- * PiP black-screen mitigation: SurfaceView's surface is destroyed and
- * recreated on PiP entry/exit, and the new surface's initial dimensions
- * can be stale until the next layout pass. We push dimension updates to
- * mpv via both SurfaceHolder.Callback.surfaceChanged AND an
- * OnLayoutChangeListener, so the PiP transition (which fires layout
- * passes on the view itself) reaches mpv promptly.
+ * Uses TextureView for reliable Picture-in-Picture support.
  */
 class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context, appContext),
-    MPVLayerRenderer.Delegate, SurfaceHolder.Callback {
+    MPVLayerRenderer.Delegate, TextureView.SurfaceTextureListener {
 
     companion object {
         private const val TAG = "MpvPlayerView"
@@ -61,7 +52,7 @@ class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context
     val onTracksReady by EventDispatcher()
     val onPictureInPictureChange by EventDispatcher()
 
-    private var surfaceView: SurfaceView
+    private var textureView: TextureView
     private var renderer: MPVLayerRenderer? = null
     private var pipController: PiPController? = null
 
@@ -72,45 +63,31 @@ class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context
     private var surfaceReady: Boolean = false
     private var pendingConfig: VideoLoadConfig? = null
     private var rendererStarted: Boolean = false
+    private var pendingSurface: Surface? = null
     private var activeSurface: Surface? = null
+    private var surfaceTexture: SurfaceTexture? = null
 
     // PiP state tracking
+    private var isWaitingForPiPTransition: Boolean = false
+    private var isPiPSurfaceForced: Boolean = false
     private val pipHandler = Handler(Looper.getMainLooper())
 
     init {
         setBackgroundColor(Color.BLACK)
 
-        // SurfaceView for video rendering. Routes the surface directly to
-        // SurfaceFlinger (the OS compositor), giving mpv a standalone
-        // surface. TextureView composites into the app's window surface
-        // which is less efficient and breaks PiP transitions.
-        surfaceView = SurfaceView(context).apply {
+        // Create TextureView for video rendering (composites into app window for PiP support)
+        textureView = TextureView(context).apply {
             layoutParams = ViewGroup.LayoutParams(
                 ViewGroup.LayoutParams.MATCH_PARENT,
                 ViewGroup.LayoutParams.MATCH_PARENT
             )
+            surfaceTextureListener = this@MpvPlayerView
         }
-        surfaceView.holder.addCallback(this@MpvPlayerView)
-        addView(surfaceView)
-
-        // Push dimension updates to mpv on every view bounds change. This
-        // is the primary PiP black-screen fix: entering PiP fires a layout
-        // pass on the SurfaceView itself, and we proactively tell mpv the
-        // new size so it resizes its EGL swapchain before rendering.
-        surfaceView.addOnLayoutChangeListener { _, left, top, right, bottom,
-                                                  oldLeft, oldTop, oldRight, oldBottom ->
-            val w = right - left
-            val h = bottom - top
-            val oldW = oldRight - oldLeft
-            val oldH = oldBottom - oldTop
-            if (w > 0 && h > 0 && (w != oldW || h != oldH)) {
-                renderer?.updateSurfaceSize(w, h)
-            }
-        }
+        addView(textureView)
 
         // Initialize PiP controller with Expo's AppContext for proper activity access
         pipController = PiPController(context, appContext)
-        pipController?.setPlayerView(surfaceView)
+        pipController?.setPlayerView(textureView)
         pipController?.delegate = object : PiPController.Delegate {
             override fun onPlay() {
                 play()
@@ -126,17 +103,17 @@ class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context
 
             override fun onPictureInPictureModeChanged(isInPiP: Boolean) {
                 if (isInPiP) {
-                    // Post size syncs after the PiP layout settles. Two passes
-                    // catch both the immediate surface re-attach and the
-                    // post-animation layout pass. Replaces the old TextureView
-                    // measure/layout polling hack (forcePiPBufferSize).
-                    pipHandler.removeCallbacksAndMessages(null)
-                    pipHandler.postDelayed({ syncSurfaceSizeToView() }, 100)
-                    pipHandler.postDelayed({ syncSurfaceSizeToView() }, 500)
+                    if (!isWaitingForPiPTransition) {
+                        isWaitingForPiPTransition = true
+                        pipHandler.removeCallbacksAndMessages(null)
+                        for (delay in longArrayOf(500, 1000, 1500, 2000)) {
+                            pipHandler.postDelayed({ forcePiPBufferSize() }, delay)
+                        }
+                    }
                 } else {
-                    // Restore from PiP: surface resized back to fullscreen.
+                    isWaitingForPiPTransition = false
                     pipHandler.removeCallbacksAndMessages(null)
-                    pipHandler.postDelayed({ syncSurfaceSizeToView() }, 100)
+                    restoreFromPiP()
                 }
                 onPictureInPictureChange(mapOf("isActive" to isInPiP))
             }
@@ -149,7 +126,7 @@ class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context
 
     /**
      * Start the renderer with the given VO driver.
-     * Called lazily on first loadVideo so user settings are available.
+     * Called lazily on first loadVideo so the voDriver setting is available.
      */
     private fun ensureRendererStarted(voDriver: String?) {
         if (rendererStarted) return
@@ -158,14 +135,10 @@ class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context
             renderer?.start(voDriver ?: "gpu-next")
             rendererStarted = true
 
-            // If the surface is already alive (surfaceCreated fired before
-            // loadVideo), attach it now. With SurfaceView the surface is
-            // owned by the holder, so we read it from there directly rather
-            // than stashing it on the side.
-            surfaceView.holder.surface?.takeIf { it.isValid }?.let { surface ->
+            pendingSurface?.let { surface ->
                 activeSurface = surface
                 renderer?.attachSurface(surface)
-                syncSurfaceSizeToView()
+                pendingSurface = null
             }
         } catch (e: Exception) {
             Log.e(TAG, "Failed to start renderer: ${e.message}")
@@ -173,20 +146,23 @@ class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context
         }
     }
 
-    // MARK: - SurfaceHolder.Callback
+    // MARK: - TextureView.SurfaceTextureListener
 
-    override fun surfaceCreated(holder: SurfaceHolder) {
-        val surface = holder.surface
+    override fun onSurfaceTextureAvailable(surfaceTexture: SurfaceTexture, width: Int, height: Int) {
+        this.surfaceTexture = surfaceTexture
+        val surface = Surface(surfaceTexture)
+        surfaceTexture.setDefaultBufferSize(width, height)
         surfaceReady = true
 
         if (rendererStarted) {
-            // The previous Surface reference is holder-owned; do NOT release
-            // it (SurfaceView manages its lifecycle). Just track the new one.
+            // Release the previous wrapper Surface before losing the only
+            // reference to it. cleanup() only runs on detach, so without this
+            // repeated PiP/background/resize cycles leak native surface objects.
+            activeSurface?.release()
             activeSurface = surface
             renderer?.attachSurface(surface)
-            // Push the actual view dimensions immediately so mpv doesn't
-            // render against stale full-screen geometry during PiP transitions.
-            syncSurfaceSizeToView()
+        } else {
+            pendingSurface = surface
         }
 
         // If we have a pending load, execute it now
@@ -197,36 +173,20 @@ class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context
         }
     }
 
-    override fun surfaceChanged(holder: SurfaceHolder, format: Int, width: Int, height: Int) {
-        if (width > 0 && height > 0) {
-            renderer?.updateSurfaceSize(width, height)
-        }
+    override fun onSurfaceTextureSizeChanged(surfaceTexture: SurfaceTexture, width: Int, height: Int) {
+        surfaceTexture.setDefaultBufferSize(width, height)
+        renderer?.updateSurfaceSize(width, height)
     }
 
-    override fun surfaceDestroyed(holder: SurfaceHolder) {
+    override fun onSurfaceTextureDestroyed(surfaceTexture: SurfaceTexture): Boolean {
+        this.surfaceTexture = null
         surfaceReady = false
         renderer?.detachSurface()
-        // Do NOT issue mpv "stop" here. Playback continues against the
-        // demuxer; when surfaceCreated fires again (PiP entry/exit, app
-        // background/foreground), we re-attach and frames resume. This
-        // matches the keep-open=always setting in MPVLayerRenderer.
-        //
-        // Do NOT release activeSurface — SurfaceView owns it via the holder.
-        activeSurface = null
+        return false // mpv manages the SurfaceTexture
     }
 
-    /**
-     * Read the actual SurfaceView width/height and push them to mpv.
-     * The PiP transition can fire surfaceCreated before the view's layout
-     * has settled to PiP dimensions, so we re-sync after layout passes.
-     */
-    private fun syncSurfaceSizeToView() {
-        if (!surfaceReady) return
-        val w = surfaceView.width
-        val h = surfaceView.height
-        if (w > 0 && h > 0) {
-            renderer?.updateSurfaceSize(w, h)
-        }
+    override fun onSurfaceTextureUpdated(surfaceTexture: SurfaceTexture) {
+        // Called every frame — no action needed, mpv drives rendering directly
     }
 
     // MARK: - Video Loading
@@ -315,7 +275,7 @@ class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context
 
         // Reset view-level state so a subsequent loadVideo() on the SAME view
         // instance re-creates the mpv handle and re-attaches the still-live
-        // SurfaceView surface. Without this, rendererStarted stays true and
+        // TextureView surface. Without this, rendererStarted stays true and
         // ensureRendererStarted() early-returns, so renderer.start() is never
         // called again — but stop() already nulled the renderer's mpv handle.
         // The next loadVideo() then runs loadVideoInternal() -> renderer.load()
@@ -326,12 +286,13 @@ class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context
         // which call destroy() immediately before router.replace() to the
         // same route — Expo Router reuses the same MpvPlayerView instance,
         // so the next source load happens on this view without a remount.
-        //
-        // SurfaceView note: the surface is owned by the holder and survives
-        // across destroy()/loadVideo() on the same view instance. The next
-        // ensureRendererStarted() reads it from surfaceView.holder.surface.
         rendererStarted = false
         currentUrl = null
+        // Move the active surface back to pending so ensureRendererStarted()
+        // re-attaches it to the freshly created mpv instance on next load.
+        // The Surface itself is still valid — onSurfaceTextureDestroyed has
+        // not fired because the TextureView is not being unmounted.
+        activeSurface?.let { pendingSurface = it }
         activeSurface = null
     }
 
@@ -366,10 +327,59 @@ class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context
     // MARK: - Picture in Picture
 
     fun startPictureInPicture() {
+        isWaitingForPiPTransition = true
         pipController?.startPictureInPicture()
+
+        // Resize buffer to match PiP window after animation settles
+        pipHandler.removeCallbacksAndMessages(null)
+        for (delay in longArrayOf(500, 1000, 1500, 2000)) {
+            pipHandler.postDelayed({ forcePiPBufferSize() }, delay)
+        }
+    }
+
+    /**
+     * Resize the SurfaceTexture buffer AND TextureView layout to match the PiP
+     * visible rect so mpv renders at the PiP window's actual dimensions.
+     */
+    private fun forcePiPBufferSize() {
+        if (!isWaitingForPiPTransition || !surfaceReady) return
+
+        val rect = Rect()
+        textureView.getGlobalVisibleRect(rect)
+        val visW = rect.width()
+        val visH = rect.height()
+        val vw = textureView.width
+        val vh = textureView.height
+
+        if (visW <= 0 || visH <= 0 || (vw == visW && vh == visH)) return
+
+        surfaceTexture?.setDefaultBufferSize(visW, visH)
+        renderer?.updateSurfaceSize(visW, visH)
+
+        // Force TextureView layout to match PiP visible area.
+        // layoutParams alone doesn't work during PiP because the parent
+        // never re-lays out its children.
+        textureView.measure(
+            View.MeasureSpec.makeMeasureSpec(visW, View.MeasureSpec.EXACTLY),
+            View.MeasureSpec.makeMeasureSpec(visH, View.MeasureSpec.EXACTLY)
+        )
+        textureView.layout(0, 0, visW, visH)
+        isPiPSurfaceForced = true
+    }
+
+    private fun restoreFromPiP() {
+        if (!isPiPSurfaceForced) return
+        isPiPSurfaceForced = false
+
+        val lp = textureView.layoutParams
+        lp.width = ViewGroup.LayoutParams.MATCH_PARENT
+        lp.height = ViewGroup.LayoutParams.MATCH_PARENT
+        textureView.layoutParams = lp
+        textureView.requestLayout()
     }
 
     fun stopPictureInPicture() {
+        isWaitingForPiPTransition = false
         pipHandler.removeCallbacksAndMessages(null)
         pipController?.stopPictureInPicture()
     }
@@ -537,12 +547,20 @@ class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context
      * off the JS path.
      */
     fun cleanup() {
+        isWaitingForPiPTransition = false
         pipHandler.removeCallbacksAndMessages(null)
         pipController?.stopPictureInPicture()
         renderer?.stop()
         renderer?.delegate = null
 
-        // SurfaceView owns the Surface via its holder — do NOT release it.
+        // Release the Surface that wraps the SurfaceTexture. These Surface
+        // objects are created in onSurfaceTextureAvailable and were never
+        // released; each playback session previously leaked one. The
+        // SurfaceTexture itself is owned by TextureView and released by it
+        // via onSurfaceTextureDestroyed, so we leave it alone.
+        pendingSurface?.release()
+        pendingSurface = null
+        activeSurface?.release()
         activeSurface = null
         surfaceReady = false
         currentUrl = null

modules/mpv-player/android/src/main/java/expo/modules/mpvplayer/PiPController.kt

@@ -44,11 +44,6 @@ class PiPController(private val context: Context, private val appContext: AppCon
     private var currentPosition: Double = 0.0
     private var currentDuration: Double = 0.0
     private var playbackRate: Double = 1.0
-    // Independently tracks whether the system should auto-enter PiP on home
-    // press. Decoupled from playbackRate so that disabling auto-enter
-    // (e.g. when the player unmounts) doesn't corrupt the play/pause icon
-    // state that buildPiPActions() derives from playbackRate.
-    private var autoEnterEnabled: Boolean = false
 
     private var videoWidth: Int = 0
     private var videoHeight: Int = 0
@@ -111,37 +106,15 @@ class PiPController(private val context: Context, private val appContext: AppCon
     }
 
     fun stopPictureInPicture() {
-        // Disable auto-enter eligibility without touching playbackRate.
-        // playbackRate drives the play/pause icon in buildPiPActions();
-        // mutating it here would cause a stale icon if PiP is re-entered
-        // before the next playback state callback corrects it.
-        autoEnterEnabled = false
         isInPiPMode = false
         pipEntryNotified = false
         unregisterLifecycleCallbacks()
-
-        val activity = getActivity() ?: return
-
-        // Push minimal params with just auto-enter disabled. Do NOT call
-        // buildPiPParams() — it calls ensurePiPReceiverRegistered() and
-        // setActions(), which would re-register the broadcast receiver
-        // (just unregistered above) and attach play/pause/skip actions to
-        // params being torn down. That leaves a live receiver + stale
-        // actions after the player has unmounted.
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
-            try {
-                activity.setPictureInPictureParams(
-                    PictureInPictureParams.Builder()
-                        .setAutoEnterEnabled(false)
-                        .build()
-                )
-            } catch (e: Exception) {
-                Log.e(TAG, "Failed to clear PiP auto-enter params: ${e.message}")
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+            val activity = getActivity()
+            if (activity?.isInPictureInPictureMode == true) {
+                activity.moveTaskToBack(false)
             }
         }
-        if (activity.isInPictureInPictureMode) {
-            activity.moveTaskToBack(false)
-        }
     }
 
     fun isCurrentlyInPiP(): Boolean = isInPiPMode
@@ -153,7 +126,6 @@ class PiPController(private val context: Context, private val appContext: AppCon
 
     fun setPlaybackRate(rate: Double) {
         playbackRate = rate
-        autoEnterEnabled = rate > 0
 
         if (rate > 0) {
             registerLifecycleCallbacks()
@@ -236,7 +208,7 @@ class PiPController(private val context: Context, private val appContext: AppCon
         builder.setActions(buildPiPActions())
 
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
-            builder.setAutoEnterEnabled(forEntering || autoEnterEnabled)
+            builder.setAutoEnterEnabled(forEntering || playbackRate > 0)
         }
 
         return builder.build()

utils/jellyfin/subtitleUtils.ts

@@ -44,9 +44,22 @@ export const isSubtitleInMpv = (
 /**
  * Calculate the MPV track ID for a given Jellyfin subtitle index.
  *
- * MPV track IDs are 1-based and only count subtitles that are actually in MPV.
- * We iterate through all subtitles, counting only those in MPV, until we find
- * the one matching the Jellyfin index.
+ * MPV track IDs are 1-based, but MPV's track list is NOT in MediaStreams order:
+ *   1. Embedded/HLS subs are enumerated from the container (or HLS playlist)
+ *      first, in MediaStreams order.
+ *   2. External subs are appended via `sub-add` AFTER the file loads, in the
+ *      order they are passed to MPV (here, also MediaStreams order — see
+ *      direct-player.tsx where the externalSubtitles array is built by
+ *      filtering MediaStreams).
+ *
+ * Iterating in pure MediaStreams order produces the wrong MPV ID whenever an
+ * External sub is listed before an Embed sub in MediaStreams (common when
+ * Jellyfin prepends a converted SRT/VTT ahead of an original PGS/ASS track),
+ * causing e.g. English to select Spanish. We therefore count in two phases
+ * that mirror MPV's actual ordering.
+ *
+ * Image-based subs (PGS/VOBSUB) during transcoding are burned into the video
+ * and absent from MPV's track list; they are skipped in both phases.
  *
  * @param mediaSource - The media source containing subtitle streams
  * @param jellyfinSubtitleIndex - The Jellyfin server-side subtitle index (-1 = disabled)
@@ -74,14 +87,30 @@ export const getMpvSubtitleId = (
     return undefined;
   }
 
-  // Count MPV track position (1-based)
+  const isExternal = (sub: MediaStream) =>
+    sub.DeliveryMethod === SubtitleDeliveryMethod.External;
+
   let mpvIndex = 0;
+
+  // Phase 1: embedded / HLS subs — these occupy MPV track IDs first because
+  // they come from the container or HLS playlist.
   for (const sub of allSubs) {
-    if (isSubtitleInMpv(sub, isTranscoding)) {
-      mpvIndex++;
-      if (sub.Index === jellyfinSubtitleIndex) {
-        return mpvIndex;
-      }
+    if (isExternal(sub)) continue;
+    if (!isSubtitleInMpv(sub, isTranscoding)) continue;
+    mpvIndex++;
+    if (sub.Index === jellyfinSubtitleIndex) {
+      return mpvIndex;
+    }
+  }
+
+  // Phase 2: external subs — appended via `sub-add` after the file loads,
+  // so they come last in MPV's track list.
+  for (const sub of allSubs) {
+    if (!isExternal(sub)) continue;
+    if (!isSubtitleInMpv(sub, isTranscoding)) continue;
+    mpvIndex++;
+    if (sub.Index === jellyfinSubtitleIndex) {
+      return mpvIndex;
     }
   }