-
-
Notifications
You must be signed in to change notification settings - Fork 73
Basic WinRT API (System Media Transport Controls) usage implementation #171
Conversation
Thanks for the PR! I pulled your branch and I'm actually having trouble building/running this locally on my machine (Win 10). Do you know of the specific dependencies that need to be installed for NodeRT? What I've been trying:
I attached a snippet of my build error, but it's also quite annoying as the errors return an excessive amount of newlines in my shell which makes it basically unreadable. |
I was able to reproduce your error, seems to the an issue with parallel building. Could you try to change |
Weird.. I'm not seeing anything on my system. I initially assumed it was the media player bar from your screenshot, but when I didn't see anything I looked for alternative options. I tested Spotify's overlay and that works on my machine so I don't think it's necessarily a system setting that is disabled on my end. From your current implementation, the overlay should pop up when the song changes regardless of whether or not a global media key is pressed, correct (since you're using the I'll see if I can test on another machine or a VM. What Windows 10 build are you on? I'm testing from |
Hi, just tested some more on Windows 10. The popup will only appear when a media key is pressed, it will not appear a normal track change, just when the user changes the track with a media key. This can be seen in this video: The Windows 10 Build is 19044.1415 |
Can you package and upload a production build for me? Maybe I'm still missing a build dependency somewhere since it doesn't seem to be working for me on any system I've tried. You should be able to package it with |
https://github.com/GermanDarknes/sonixd/releases/tag/v0.11.1 Uploaded the build I made on Windows 10. |
Thanks for the build, but unfortunately it's still not working for me. Might need some additional people to test since I can't confirm it on my side. |
Hi, I have tested @GermanDarknes build on Windows 10 using my keyboards media keys (Play/Pause and Next). Do you maybe not have media keys @jeffvli? |
Thanks @kaiser-chris. After some further testing I was able to get it to display.
This leads me to believe that the current implementation doesn't gracefully handle all instances of media keys being pressed. Would you be able to do some additional research and testing to determine why the controls aren't displaying for certain media key presses? @GermanDarknes |
Hey there, Also which program is your volume control from, @jeffvli ? Since its' media control doesn't look like a normal Win10 Feature, I'd like to test the build with this one installed. ( |
I think I got the problem now. The popup won't show up when Global Hotkeys are enabled.
|
@GermanDarknes Just tested and confirmed on my side that it works without Global Media Hotkeys enabled, good find!
Are you comfortable with working on the frontend and settings aspect of this? If not I can take a look at it a bit later. I also noticed an issue in Windows 10 where the Media overlay in the lock screen incorrectly shows the previous song's image if the current song does not have an image set (due to not replacing the existing image when there's a placeholder. Seems like setting a null value for the thumbnail doesn't make it blank there. Not sure if it's fixable but may be worth taking a look at. To reproduce the issue:
I'll also need to create a custom build agent and update the CI/CD since all production releases are being built using macOS which won't work with the newly added native dependencies for Windows. @SetCr4 The screenshot above is just something I found on the web, I'm not sure if it's actually a usable feature. |
The problem with the thumbnail only appears on the lock screen itself, on desktop it will show a blank/black image. I will see if Would be nice if you could do the ui part, I think a config value for the OS needs to implemented for it. Maybe a select menu can be used for the hotkey setting, if the disabling of the global hot key checkbox is not working right or not intuitive enough. Update:
Even this doesn't seem to work on the lock screen, it always uses the last available cover for it. You can see it doesn't update when you lock the screen while the popup is there. It will show the popup on the top left and the lock screen thing at the bottom right, while the popup shows a blank picture, the lock screen widget still shows a cover. Seems to be a Windows issue for now. |
No worries, I can add it. Technically, a workaround can be added to just point to the placeholder image hosted on GitHub: https://raw.githubusercontent.com/jeffvli/sonixd/main/src/img/placeholder.png. Can you try setting the thumbnail to that url instead? |
Is there any other online resource besides the update check? Otherwise I think we should prefer offline available content. It would need a new dependency, but this uses the icon from the asset folder instead of the online resource as fallback:
|
Did you already test the solution? I'm having trouble installing I agree that using the offline image would probably be better, but if it creates additional barriers to setting up the development/build environment it may be better just to fall back to using a suboptimal solution. |
Got it working, I made a branch with the changes: This also enables parallel building for Linux/Mac again. I got the same problem as you, seems to be an issue with electron-rebuild and its cache, you have to delete %USERPROFILE%/.electron-gyp , after this the plain "yarn" command will work again and everything rebuilds just fine. Attached a complete build log from the branch. Update: Is it possible to lock the version of nan for the linux build only? Seems to be an issue with the latest yarn.lock |
Just tested to build and package my latest state in WSL Ubuntu 20.04 (node v14.18.2), everything was successful. Can't reproduce the error from the Check there, maybe you can find something. Update: |
I wasn't able to get the build to work by deleting I still think it would be better to revert either to setting the thumbnail to null or just pointing it to the hosted image url as I'm not reliably able to compile the new dependency on my local machine. The frontend code looks fine, though I think some refactoring of my existing code will be needed. if (settings.getSync('globalMediaHotkeys')) {
globalShortcut.register('MediaStop', () => {
stop();
});
globalShortcut.register('MediaPlayPause', () => {
playPause();
});
globalShortcut.register('MediaNextTrack', () => {
nextTrack();
});
globalShortcut.register('MediaPreviousTrack', () => {
previousTrack();
});
} else {
electronLocalshortcut.register(mainWindow, 'MediaStop', () => {
stop();
});
electronLocalshortcut.register(mainWindow, 'MediaPlayPause', () => {
playPause();
});
electronLocalshortcut.register(mainWindow, 'MediaNextTrack', () => {
nextTrack();
});
electronLocalshortcut.register(mainWindow, 'MediaPreviousTrack', () => {
previousTrack();
});
} ipcMain.on('disableGlobalHotkeys', () => {
globalShortcut.unregisterAll();
electronLocalshortcut.register(mainWindow, 'MediaStop', () => {
stop();
});
electronLocalshortcut.register(mainWindow, 'MediaPlayPause', () => {
playPause();
});
electronLocalshortcut.register(mainWindow, 'MediaNextTrack', () => {
nextTrack();
});
electronLocalshortcut.register(mainWindow, 'MediaPreviousTrack', () => {
previousTrack();
});
}); |
Removed the |
Looks good to me! Can you rebase and clean up some of the commits before I rebase it into main (I also need to update the ci/cd)? Edit: Also thanks for bearing with me on all the changes/updates! |
Cleaned it up a bit. Thanks for help with this. It's still missing the show on windows only check for the toggle, would be nice if you could do it after merge. |
Sorry for this, was an error on my side. Could you open this again or should I do a new request? |
Wanted to use the WinRT API (System Media Transport Controls) to get the currently played song, this is a basic implementation of the functionality.
Windows 11 shows it like this:
I did not test it on Windows 10.
This implementation could be a start of #79