Skip to content
This repository has been archived by the owner on Aug 23, 2024. It is now read-only.

Basic WinRT API (System Media Transport Controls) usage implementation #171

Merged
merged 2 commits into from
Jan 13, 2022

Conversation

GermanDarknes
Copy link
Contributor

@GermanDarknes GermanDarknes commented Jan 4, 2022

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:
2022-01-05_00 49 48-953c03

I did not test it on Windows 10.

This implementation could be a start of #79

@jeffvli
Copy link
Owner

jeffvli commented Jan 5, 2022

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:

  1. Installed MS Build tools
  2. Installed Windows SDK for 14393
  3. Re-installed node-gyp
  4. Copied Windows.winmd and platform.winmd to C:\Program Files (x86)\Windows Kits\10\UnionMetadata

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.
build error.txt

@GermanDarknes
Copy link
Contributor Author

GermanDarknes commented Jan 5, 2022

I was able to reproduce your error, seems to the an issue with parallel building.

Could you try to change --parallel to --no-parallel in sonixd/.erb/scripts/ElectronRebuild.js?

@jeffvli
Copy link
Owner

jeffvli commented Jan 5, 2022

Changing to -no-parallel worked, thanks for that fix.

Is this feature limited to Windows 11? I don't have access to a Windows 11 machine at the moment, and I don't see anything appearing on my Win10 machine.

I'm assuming it's supposed to look something similar to this, but I don't see anything in either the Action Center or my volume controls.
image

src/main.dev.js Outdated Show resolved Hide resolved
@GermanDarknes
Copy link
Contributor Author

GermanDarknes commented Jan 5, 2022

Just tested the build on Windows 10, when pressing a media key, it shows this overlay:
image

The Spotify Software and FireFox with YouTube also show this overlay when pressing a media key. I don't know if Spotify also uses another api to show the player like in your image.

@jeffvli
Copy link
Owner

jeffvli commented Jan 5, 2022

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 current-song event)? I'm not sure if most people would prefer it like that, or if they would rather have it only appear when a media key is pressed (which is what Spotify does). In either case, a toggle in the config should probably be added to enable/disable the feature.

I'll see if I can test on another machine or a VM. What Windows 10 build are you on? I'm testing from 19043.1415.

@GermanDarknes
Copy link
Contributor Author

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:
https://share.germandarknes.net/2022-01-05_13.37.16-b5648e.mp4

The Windows 10 Build is 19044.1415

@jeffvli
Copy link
Owner

jeffvli commented Jan 5, 2022

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 yarn package and node v14.16.1.

src/main.dev.js Outdated Show resolved Hide resolved
@GermanDarknes
Copy link
Contributor Author

GermanDarknes commented Jan 5, 2022

https://github.com/GermanDarknes/sonixd/releases/tag/v0.11.1

Uploaded the build I made on Windows 10.

@jeffvli
Copy link
Owner

jeffvli commented Jan 5, 2022

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.

@jeffvli jeffvli added the help wanted Extra attention is needed label Jan 5, 2022
@kaiser-chris
Copy link

Hi, I have tested @GermanDarknes build on Windows 10 using my keyboards media keys (Play/Pause and Next).
The overlay opens as described (when pressing a media key) and I have recorded a small video showing how it behaves:
https://share.bahmut.de/2022-01-05_18.38.03-31e0d3.mp4

Do you maybe not have media keys @jeffvli?

@jeffvli
Copy link
Owner

jeffvli commented Jan 5, 2022

Thanks @kaiser-chris. After some further testing I was able to get it to display.
These were my methods of testing since I don't have dedicated media keys on my keyboard.

  1. Using the media keys on my Logitech mouse (does not work)
    image
  2. Using AutoHotkey script (does not work)
; !q is ALT + Q
!q::Send  {Media_Next}
!e::Send {Media_Prev}
!`::Send {Media_Play_Pause}
  1. Using alternative Autohotkey script (this works)
!q::Media_Next
!e::Media_Prev
!`::Media_Play_Pause

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

@SetCr4
Copy link

SetCr4 commented Jan 6, 2022

Hey there,
here is another test on Win10 of @GermanDarknes build using the fn keycombinations on my keyboard.
Seems to be working.
https://share.cr4.live/2022-01-06_01.29.24-a72562.mp4

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. (
image
)

@GermanDarknes
Copy link
Contributor Author

I think I got the problem now. The popup won't show up when Global Hotkeys are enabled.

const Controls = BackgroundMediaPlayer.current.systemMediaTransportControls; is responsible for the media keys, but gets replaced with the currently implemented media key capture.

@jeffvli
Copy link
Owner

jeffvli commented Jan 6, 2022

@GermanDarknes Just tested and confirmed on my side that it works without Global Media Hotkeys enabled, good find!
So the next steps in order to get this ready for merge:

  • Add a (Windows only) config toggle called something like Use Windows Media Controls to turn on/off the Windows desktop overlay when using media keys
  • If the new toggle is enabled, unregister all global/local electron shortcuts for media keys
  • If the new toggle is enabled, automatically turn off and disable the Global Media Hotkeys toggle

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:

  1. Have 2 songs in the following order: 1. HAS IMAGE 2. NO IMAGE
  2. Skip from SONG 1 to SONG 2 in the lock screen
  3. SONG 2 will show the previous song's image

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.

@jeffvli jeffvli removed the help wanted Extra attention is needed label Jan 6, 2022
@jeffvli jeffvli linked an issue Jan 6, 2022 that may be closed by this pull request
@GermanDarknes
Copy link
Contributor Author

GermanDarknes commented Jan 6, 2022

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 Controls.displayUpdater.clearAll(); works for this.

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:

    if (!arg.image.includes('placeholder')) {
      Controls.displayUpdater.thumbnail = RandomAccessStreamReference.createFromUri(
        new Uri(arg.image)
      );
    } else {
      Controls.displayUpdater.clearAll();
      Controls.displayUpdater.type = MediaPlaybackType.music;
    }

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.

@jeffvli
Copy link
Owner

jeffvli commented Jan 6, 2022

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.

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?

@GermanDarknes
Copy link
Contributor Author

GermanDarknes commented Jan 6, 2022

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:

import { StorageFile } from '@nodert-win10-au/windows.storage';

if (arg.image.includes('placeholder')) {
  StorageFile.getFileFromPathAsync(getAssetPath('icon.png'), (error, storageFile) => {
	if (error) {
	  Controls.displayUpdater.thumbnail = null;
	} else {
	  Controls.displayUpdater.thumbnail = RandomAccessStreamReference.createFromFile(
		storageFile
	  );
	}
  });
} else {
  Controls.displayUpdater.thumbnail = RandomAccessStreamReference.createFromUri(
	new Uri(arg.image)
  );
}

@jeffvli
Copy link
Owner

jeffvli commented Jan 6, 2022

Did you already test the solution? I'm having trouble installing @nodert-win10-au/windows.storage locally, with this error C:\code\sonixd\src\node_modules\@nodert-win10-au\windows.storage\_nodert_generated.cpp : fatal error C1128: number of sections exceeded object file format limit: compile with /bigobj [C:\code\sonixd\src\node_modules\@nodert-win10-au\windows.storage\build\binding.vcxproj]. I'm not quite sure how to add the /bigobj param onto the build.
err.txt

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.

@GermanDarknes
Copy link
Contributor Author

GermanDarknes commented Jan 6, 2022

Got it working, I made a branch with the changes:
https://github.com/GermanDarknes/sonixd/tree/feature-79-win-smtc

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.
build-log.txt
build-package.txt

Update:
Just merged the branch. Adjusted the code to use a setting toggle, only 1 toggle of global media keys or windows smtc will work. When a toggle gets activated, the other will be automatically disabled. It's still missing the windows 10 only check.

Is it possible to lock the version of nan for the linux build only? Seems to be an issue with the latest yarn.lock

@GermanDarknes
Copy link
Contributor Author

GermanDarknes commented Jan 7, 2022

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:
Found the issue, it's parallel building again. Could be a bug because of node v15.x in the test environment.

@jeffvli
Copy link
Owner

jeffvli commented Jan 7, 2022

I wasn't able to get the build to work by deleting %USERPROFILE%/.electron-gyp. I eventually found this manual solution to add the dependencies, but it may not be working correctly since I still don't see the placeholder image appearing on either the lock screen or the main desktop overlay. I also saw this as an unresolved issue that others were having so not sure if there's a definitive fix available for it.

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. electronLocalshortcut is still being registered for the media keys when the globalMediaHotkeys setting is disabled, which causes the track to skip 2 times when the Sonixd is focused and the new media transport controls is enabled. You can see this in the following code blocks in main.dev.js.

  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();
    });
  });

@GermanDarknes
Copy link
Contributor Author

Removed the @nodert-win10-au/windows.storage and replaced it with the URL from github. The skipping of songs is also fixed.

@jeffvli
Copy link
Owner

jeffvli commented Jan 7, 2022

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)?
For example removing the re-enabling and then re-disabling parallel builds for mac/linux, and also squash the package.json/yarn lock dependencies into a single commit.

Edit: Also thanks for bearing with me on all the changes/updates!

@GermanDarknes
Copy link
Contributor Author

GermanDarknes commented Jan 7, 2022

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.

@GermanDarknes
Copy link
Contributor Author

GermanDarknes commented Jan 8, 2022

Sorry for this, was an error on my side. Could you open this again or should I do a new request?

@jeffvli jeffvli reopened this Jan 8, 2022
src/main.dev.js Outdated Show resolved Hide resolved
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Media Keys Control
4 participants