From 356a93310f8fc04a163ea03e3312c040603cf593 Mon Sep 17 00:00:00 2001 From: Ray <154766448+damontecres@users.noreply.github.com> Date: Mon, 5 Jan 2026 15:38:36 -0500 Subject: [PATCH] MPV: Handle resolution switch (#635) ## Description When switching resolution (and possibly refresh rate less frequently), the initial surface is not always valid. This means, the MPV playback treated it as detaching the surface and would never update. This PR instead subscribes to the `SurfaceHolder`'s callbacks which will provide a new, valid surface once the switch is ready. Then the surface is attached and queued commands execute to start playback. Also temporarily disables content scale options for MPV since the compose implementation conflicts with MPV rendering. ### Related issues Fixes #626 --- .../wholphin/ui/playback/PlaybackDialog.kt | 5 +- .../wholphin/ui/playback/PlaybackPage.kt | 14 ++- .../wholphin/util/mpv/MpvPlayer.kt | 106 ++++++++++++++---- 3 files changed, 99 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackDialog.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackDialog.kt index f6a42f64..1f13b278 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackDialog.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackDialog.kt @@ -46,6 +46,7 @@ data class PlaybackSettings( @Composable fun PlaybackDialog( enableSubtitleDelay: Boolean, + enableVideoScale: Boolean, type: PlaybackDialogType, settings: PlaybackSettings, onDismissRequest: () -> Unit, @@ -117,7 +118,9 @@ fun PlaybackDialog( buildList { add(stringResource(R.string.audio)) add(stringResource(R.string.playback_speed)) - add(stringResource(R.string.video_scale)) + if (enableVideoScale) { + add(stringResource(R.string.video_scale)) + } if (enableSubtitleDelay) { add(stringResource(R.string.subtitle_delay)) } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackPage.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackPage.kt index 1bf8ec56..3dcf6584 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackPage.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackPage.kt @@ -40,6 +40,7 @@ import androidx.compose.ui.focus.focusRequester import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.RectangleShape import androidx.compose.ui.input.key.onKeyEvent +import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalConfiguration import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.text.intl.Locale @@ -152,7 +153,7 @@ fun PlaybackPage( var playbackDialog by remember { mutableStateOf(null) } OneTimeLaunchedEffect { if (prefs.playerBackend == PlayerBackend.MPV) { - scope.launch(Dispatchers.Main + ExceptionHandler()) { + scope.launch(Dispatchers.IO + ExceptionHandler()) { preferences.appPreferences.interfacePreferences.subtitlesPreferences.applyToMpv( configuration, density, @@ -161,7 +162,15 @@ fun PlaybackPage( } } AmbientPlayerListener(player) - var contentScale by remember { mutableStateOf(prefs.globalContentScale.scale) } + var contentScale by remember { + mutableStateOf( + if (prefs.playerBackend == PlayerBackend.MPV) { + ContentScale.FillBounds + } else { + prefs.globalContentScale.scale + }, + ) + } var playbackSpeed by remember { mutableFloatStateOf(1.0f) } LaunchedEffect(playbackSpeed) { player.setPlaybackSpeed(playbackSpeed) } @@ -572,6 +581,7 @@ fun PlaybackPage( onPlaybackActionClick = onPlaybackActionClick, onChangeSubtitleDelay = { viewModel.updateSubtitleDelay(it) }, enableSubtitleDelay = player is MpvPlayer, + enableVideoScale = player !is MpvPlayer, ) } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/util/mpv/MpvPlayer.kt b/app/src/main/java/com/github/damontecres/wholphin/util/mpv/MpvPlayer.kt index c2ff22a1..1be415e9 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/util/mpv/MpvPlayer.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/util/mpv/MpvPlayer.kt @@ -69,7 +69,8 @@ class MpvPlayer( ) : BasePlayer(), MPVLib.EventObserver, TrackSelector.InvalidationListener, - Handler.Callback { + Handler.Callback, + SurfaceHolder.Callback { companion object { private const val DEBUG = false } @@ -467,26 +468,55 @@ class MpvPlayer( override fun clearVideoSurfaceHolder(surfaceHolder: SurfaceHolder?): Unit = throw UnsupportedOperationException() + private var surfaceHolder: SurfaceHolder? = null + override fun setVideoSurfaceView(surfaceView: SurfaceView?) { if (DEBUG) Timber.v("setVideoSurfaceView") - val surface = surfaceView?.holder?.surface - if (surface != null && surface.isValid) { - Timber.v("Queued attach") - sendCommand(MpvCommand.ATTACH_SURFACE, surface) - } else { - clearVideoSurfaceView(null) + if (surfaceView != null) { + this.surfaceHolder?.removeCallback(this) + this.surfaceHolder = surfaceView.holder + if (surfaceView.holder != null) { + val surface = surfaceView.holder?.surface + surfaceView.holder.addCallback(this) + Timber.v("Got surface holder: isValid=${surface?.isValid}") + if (surface != null && surface.isValid) { + Timber.v("Queued attach") + sendCommand(MpvCommand.ATTACH_SURFACE, surface) + return + } + } } + clearVideoSurfaceView(null) } override fun clearVideoSurfaceView(surfaceView: SurfaceView?) { - if (surface == surfaceView?.holder?.surface) { + if (surface != null && surface == surfaceView?.holder?.surface) { Timber.d("clearVideoSurfaceView") sendCommand(MpvCommand.ATTACH_SURFACE, null) } else { - Timber.w("clearVideoSurfaceView called with different surface") + Timber.w("clearVideoSurfaceView called with different surface: %s", surfaceView) } } + override fun surfaceChanged( + holder: SurfaceHolder, + format: Int, + width: Int, + height: Int, + ) { + Timber.v("surfaceChanged: format=$format, width=$width, height=$height") + } + + override fun surfaceCreated(holder: SurfaceHolder) { + Timber.v("surfaceCreated") + sendCommand(MpvCommand.ATTACH_SURFACE, holder.surface) + } + + override fun surfaceDestroyed(holder: SurfaceHolder) { + Timber.v("surfaceDestroyed") + sendCommand(MpvCommand.ATTACH_SURFACE, null) + } + override fun setVideoTextureView(textureView: TextureView?): Unit = throw UnsupportedOperationException() override fun clearVideoTextureView(textureView: TextureView?): Unit = throw UnsupportedOperationException() @@ -660,6 +690,7 @@ class MpvPlayer( MPV_EVENT_VIDEO_RECONFIG -> { Timber.d("event: MPV_EVENT_VIDEO_RECONFIG") updateTracksAndNotify() + updateVideoSizeAndNotify() } MPV_EVENT_END_FILE -> { @@ -720,6 +751,19 @@ class MpvPlayer( notifyListeners(EVENT_TRACKS_CHANGED) { onTracksChanged(tracks) } } + private fun updateVideoSizeAndNotify() { + val width = MPVLib.getPropertyInt("width") + val height = MPVLib.getPropertyInt("height") + val videoSize = + if (width != null && height != null) { + VideoSize(width, height) + } else { + VideoSize.UNKNOWN + } + playbackState.update { it.copy(videoSize = videoSize) } + notifyListeners(EVENT_VIDEO_SIZE_CHANGED) { onVideoSizeChanged(videoSize) } + } + private fun loadFile(media: MediaAndPosition) { Timber.v("loadFile: media=$media") playbackState.update { @@ -836,24 +880,34 @@ class MpvPlayer( internalHandler.obtainMessage(cmd.ordinal, obj).sendToTarget() } + private val queuedCommands = mutableListOf>() + override fun handleMessage(msg: Message): Boolean { val cmd = MpvCommand.entries[msg.what] - Timber.d("handleMessage: cmd=$cmd") if (isReleased && cmd != MpvCommand.DESTROY) { Timber.w("Player is released, ignoring command %s", cmd) return true } if (surface == null && !cmd.isLifecycle) { - // If libmpv isn't ready, re-enqueue the messages + // If libmpv isn't ready, ueue the messages // Note: this means nothing will play until it is attached to a surface, // so MpvPlayer can't be used for background audio/music playback - Timber.v("MPV is not initialized/attached yet, requeue cmd %s", cmd) - internalHandler.sendMessageDelayed(Message.obtain(msg), 250) - return true + Timber.v("MPV is not initialized/attached yet, queue cmd %s", cmd) + queuedCommands.add(Pair(cmd, msg.obj)) + } else { + handleCommand(cmd, msg.obj) } + return true + } + + private fun handleCommand( + cmd: MpvCommand, + obj: Any?, + ) { + Timber.d("handleCommand: cmd=$cmd") when (cmd) { MpvCommand.PLAY_PAUSE -> { - val playWhenReady = msg.obj as Boolean + val playWhenReady = obj as Boolean MPVLib.setPropertyBoolean("pause", !playWhenReady) playbackState.update { it.copy(isPaused = !playWhenReady) @@ -867,13 +921,13 @@ class MpvPlayer( } MpvCommand.SET_TRACK_SELECTION -> { - val (propertyName, trackId) = msg.obj as TrackSelection + val (propertyName, trackId) = obj as TrackSelection MPVLib.setPropertyString(propertyName, trackId) updateTracksAndNotify() } MpvCommand.SEEK -> { - val positionMs = msg.obj as Long + val positionMs = obj as Long MPVLib.setPropertyDouble("time-pos", positionMs / 1000.0) playbackState.update { it.copy(positionMs = positionMs) @@ -881,7 +935,7 @@ class MpvPlayer( } MpvCommand.SET_SPEED -> { - val value = msg.obj as Float + val value = obj as Float MPVLib.setPropertyDouble("speed", value.toDouble()) playbackState.update { it.copy(speed = value) @@ -889,7 +943,7 @@ class MpvPlayer( } MpvCommand.SET_SUBTITLE_DELAY -> { - val value = msg.obj as Double + val value = obj as Double MPVLib.setPropertyDouble("sub-delay", value) playbackState.update { it.copy(subtitleDelay = value) @@ -897,11 +951,11 @@ class MpvPlayer( } MpvCommand.LOAD_FILE -> { - loadFile(msg.obj as MediaAndPosition) + loadFile(obj as MediaAndPosition) } MpvCommand.ATTACH_SURFACE -> { - val surface = msg.obj as Surface? + val surface = obj as Surface? if (surface == null || (this.surface != null && this.surface != surface)) { // If clearing or changing the surface MPVLib.detachSurface() @@ -914,6 +968,13 @@ class MpvPlayer( this.surface = surface MPVLib.setOptionString("force-window", "yes") Timber.d("Attached surface") + if (queuedCommands.isNotEmpty()) { + Timber.d("Processing queued commands") + while (queuedCommands.isNotEmpty()) { + val msg = queuedCommands.removeAt(0) + handleCommand(msg.first, msg.second) + } + } } } @@ -928,7 +989,6 @@ class MpvPlayer( Timber.d("MPVLib destroyed") } } - return true } } @@ -1024,7 +1084,7 @@ private data class PlaybackState( val EMPTY = PlaybackState( timestamp = C.TIME_UNSET, - isLoadingFile = false, + isLoadingFile = true, media = null, positionMs = C.TIME_UNSET, durationMs = C.TIME_UNSET,