Skip to content

Conversation

@ENDlezZenith
Copy link
Contributor

MPV have its own integration of SMTC, and hence currently behavior of feishin is that the media session option would do nothing additive but disables global hotkeys for MPV.

In according to:

This was discussed in the Discord (/matrix bridge). The correct mechanism would be to just use MediaSession API for Electron (when not using MPV player).

This PR does the following:

  • Hides the media session option in MPV
  • Shows global hotkeys option in MPV
  • Redo registering global hotkeys when enabled like before in MPV

@vercel
Copy link

vercel bot commented Oct 22, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
feishin Ready Ready Preview Comment Oct 29, 2025 5:49am

Comment on lines 53 to 55
isHidden:
!isElectron() ||
(enableWindowsMediaSession && isWindows && playbackType === PlaybackType.WEB),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally not in favor of hiding it.

Users who are already using MPV may never know that this option is available if it is hidden. Disabling is enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then should we change the globalMediaHotkeys too, only disable but not hidden for Web users?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm slow but why would MPV users need to see/know about this option when they are already getting the same functionality anyway? They're not missing out on anything not seeing it 🤔
Or am I misunderstanding the PR description?

@jeffvli jeffvli merged commit 829c27a into jeffvli:development Nov 2, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants