addressing coderabbit comments

Signed-off-by: Lance Chant <13349722+lancechant@users.noreply.github.com>
This commit is contained in:
Lance Chant
2026-06-25 09:06:06 +02:00
parent 3b926e0061
commit b2eb7f1120
2 changed files with 58 additions and 0 deletions

View File

@@ -155,6 +155,10 @@ class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context
surfaceReady = true
if (rendererStarted) {
// 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)
} else {
@@ -268,6 +272,28 @@ class MpvPlayerView(context: Context, appContext: AppContext) : ExpoView(context
*/
fun destroy() {
renderer?.stop()
// Reset view-level state so a subsequent loadVideo() on the SAME view
// instance re-creates the mpv handle and re-attaches the still-live
// 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()
// against mpv == null, where every mpv?.command() (including the
// "stop" and load commands) silently no-ops, leaving a black frame.
//
// This path is hit by direct-player.tsx's goToNextItem()/stop(),
// 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.
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
}
fun seekTo(position: Double) {

View File

@@ -298,6 +298,38 @@ class MpvPlayerView: ExpoView {
*/
func destroy() {
renderer?.stop()
// Reset view state and re-create the mpv handle so a subsequent
// loadVideo() on the SAME view instance can actually load.
// Without this, stop() leaves renderer.mpv == nil, and the next
// loadVideo(config:) calls renderer.load() which early-returns
// at `guard let handle = self.mpv else { return }` but only
// after flipping isLoading = true and dispatching the loading
// delegate callback, so the JS layer is stuck in a perpetual
// "loading" state with no actual playback.
//
// This path is hit by direct-player.tsx's goToNextItem()/stop(),
// which call destroy() immediately before router.replace() to
// the same route Expo Router reuses the same MpvPlayerView
// instance, so the next `source` prop update arrives on this
// view without a remount. setupView() is otherwise the only
// place start() is called, so without re-starting here the
// renderer stays dead until the whole view is unmounted and
// recreated.
//
// start() is idempotent (`guard !isRunning else { return }`)
// and stop() has already nulled mpv synchronously before
// dispatching the async mpv_terminate_destroy, so creating a
// fresh handle here is safe even while the old handle's
// teardown is still in flight on a background queue (libmpv
// handles are independent).
currentURL = nil
intendedPlayState = false
do {
try renderer?.start()
} catch {
onError(["error": "Failed to restart renderer after destroy: \(error.localizedDescription)"])
}
}
func seekTo(position: Double) {