Fix idle->playing state transition #90
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @pritambaral, thanks for the PR! Your explanation makes sense but sadly it seems to have broken the tests. Are you able to take a look at what's gone wrong? |
When mpv is started with the `--idle=yes` CLI flag, MPV_EVENT_IDLE fires almost immediately at process start. This causes mpv-mpris to mark `PlaybackStatus` as `STOPPED`. When a file/URL is subsequently loaded into mpv and starts playing, mpv-mpris continues to believe that playback is stopped. Turns out, MPV_EVENT_IDLE was designed only for a state transition into idle state, and is itself deprecated, with the "idle-active" property suggested as a proper substitute. This property changes correctly both when entering as well as exiting idle state. Curiously, the "pause" property starts out as false in `--idle=yes` mode, thus playback start never triggers a property change for it, and thus mpv-mpris remains in a stopped state. This temporary incorrectness in state is automatically corrected, however, when playback is subsequently paused.
pritambaral
force-pushed
the
fix/deprecated-idle-event
branch
from
703a9df
to
31e4237
Compare
Fixed, @hoyon. I pulled idle state tracking out of paused state tracking, since mpv tracks them as separate states anyway. The two states are merged into a single "PlaybackStatus" when reporting to MPRIS; except in an |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Bug
When mpv is started with the
--idle=yesCLI flag, MPV_EVENT_IDLE fires almost immediately at process start. This causes mpv-mpris to markPlaybackStatusasSTOPPED.When a file/URL is subsequently loaded into mpv and starts playing, mpv-mpris continues to believe that playback is stopped.
Steps to Repro
--input-ipc-server=/tmp/mpv.sock) or in Pseudo GUI mode (--player-operation-mode=pseudo-gui).loadfileIPC command if in IPC mode or via drag-and-drop if in Pseudo GUI mode — and wait until mpv starts playing itExpected: The mpv instance has the selected media playing, the seek status is being continuously updated, and MPRIS controls to pause and play work.
Actual: The mpv instance has the selected media playing, but the seek status does not update, and MPRIS controls to pause and play do not work.
The Fix
Turns out, MPV_EVENT_IDLE was designed only for a state transition into idle state, and is itself deprecated, with the "idle-active" property suggested as a proper substitute. This property changes correctly both when entering as well as exiting idle state.
The Workaround
Curiously, the "pause" property starts out as false in
--idle=yesmode, thus playback start never triggers a property change for it, and thus mpv-mpris remains in a stopped state. This temporary incorrectness in state is automatically corrected, however, when playback is subsequently paused.