MarshallOfSound / Google-Play-Music-Desktop-Player-UNOFFICIAL-

A beautiful cross platform Desktop Player for Google Play Music
https://www.googleplaymusicdesktopplayer.com
MIT License
8.27k stars 767 forks source link

Media keys are sent twice, double-skipping or pausing and playing immediately. #2253

Closed RadGH closed 7 years ago

RadGH commented 7 years ago

OS: Windows 10 x64

Issue Descriptions: Pressing media hotkeys, such as play/pause, triggers the event twice.

Steps to Reproduce: Install GPDMP, play any station or playlist, and press any media key.

Additional Information:

I use GPDMP at home and at work. I only have the issue on my work computer. I have the same keyboard at both locations (Logitech G710+). I have uninstalled the play music extension for chrome (before using GPDMP in fact, because it was useless anyway).

I do not use other media players. I have tried closing and uninstalling my keyboard software (Logitech Gaming Software) which had no effect.

When tabbed out of GPDMP, pressing Play/Pause pauses and starts playing again within about 1/10 of a second. When tabbed in to GPDMP it sometimes pauses or plays properly, but sometimes it still sends it twice.

My guess, which is completely speculative, is that there is a native media control being sent quickly, and some emulated event also being triggered at a very minor delay. When tabbed in, the delay is less prominent and so sometimes they line up and pause properly.

I have reviewed these, but they do not help. I have the current version from the homepage, 4.1.1.

  1. https://github.com/MarshallOfSound/Google-Play-Music-Desktop-Player-UNOFFICIAL-/issues/2107

  2. https://github.com/MarshallOfSound/Google-Play-Music-Desktop-Player-UNOFFICIAL-/issues/2217

Thanks

debug_info.zip

jostrander commented 7 years ago

https://github.com/MarshallOfSound/Google-Play-Music-Desktop-Player-UNOFFICIAL-/issues/2107#issuecomment-274723989

jostrander commented 7 years ago

In other words, this has been fixed, as stated by that comment. It will be out in 4.1.2, as soon as that's finished.

RadGH commented 7 years ago

@jostrander Terribly sorry, I didn't realize the update wasn't released yet. That explains everything!

Meegooo commented 7 years ago

That issue is still present in 4.2.0 (with windows 10 media service thing enabled)

MarshallOfSound commented 7 years ago

That issue is still present in 4.2.0 (with windows 10 media service thing enabled)

I mean, of course it will... The whole point of adding that option is so that people experiencing this issue can disable it... Only a portion of users have this issue on windows, so those that have the issue should disable the feature, though I think I disabled it by default in the latest version to get it working for everyone

MattJeanes commented 7 years ago

That's not really a fix though?

I like having the Windows media integration but the double keys are very annoying. Perhaps you could just either have an option to disable GPMDP's response to media keys or automatically do it if integration is enabled?

All of my hotkeys are not set and there's no way to stop it responding to media controls.

Kytech commented 7 years ago

@MarshallOfSound I've been looking into this issue and messing around with the code a little bit. Windows 10's System Media Transport Controls are initialized with the use of a BackgroundMediaPlayer instance for lock screen integragion, which is the method GPMDP uses. I believe BackgroundMediaPlayer automatically registers low-level hooks to bind the Controls.on('buttonpressed', (sender, eventArgs)) event handler with the hardware media keys in addition to the overlay buttons, causing the GPM.playback.forward() function to be called when a physical media key is pressed. Digging through the GPMDP changelogs, I found that media key support was possibly integrated using your Low Level Key Hooks module when releasing v3.1.0, however, I can't find where this is used in the current version.

Where do you implement media key bindings for other windows versions in the latest version of GPMDP? I have a theory on why the media keys are being triggered twice, but to test it, I need to find the code that binds media keys on Windows (Pre Win10 media overlay integration/original media key binding method). My theory works as follows:

  1. Media Key is Pressed
  2. Controls.on(buttonpressed, (sender, eventArgs)) is called by Win10 media service/integration on media key press, calling GPM.playback.forward()
  3. Low Level Key Hooks module (or equivalent used for latest version) which allows for media key integration on windows versions other than Win10 catches the key press and fires an event that calls, GPM.playback.forward(), resulting in the observed repeated effect when using the physical media keys, since clicking the buttons on the overlay doesn't cause the skipping effect.

I might be able to track down the issue if I can find where the media key bindings were first implimented for Windows. A little help finding the code for that would be great!

MarshallOfSound commented 7 years ago

@Kytech You are probably correct that was my conclusion as well, they hard part is fixing it. Knowing the difference between duplicate events and two distinct events.

This is the code that you are looking for I think 👍

https://github.com/MarshallOfSound/Google-Play-Music-Desktop-Player-UNOFFICIAL-/blob/master/src/main/features/core/keyboardShortcuts.js#L14

timmevandermeer commented 7 years ago

@MarshallOfSound for me only registering the media keys in keyboardShortcuts.js if win10 mediaservice is disabled solves the problem. Is there a reason to not do this?

https://github.com/timmevandermeer/Google-Play-Music-Desktop-Player-UNOFFICIAL-/commit/e11b54639c493590b7455b593536dfc717ed2585#diff-1a98018c7484c813f86c59d6bf122bc5R15

RadGH commented 7 years ago

Since I'm the one who created this topic, I'd just like to point out to @jostrander that the issue for me seems to be resolved with version 4.2.0.

But there is some talk of other development changes in here. This topic is closed though. Consider re-opening this topic or taking the conversation somewhere more relevant.

I'll be unsubscribing from the thread because I'm not a (software) developer and I don't really know what's going on here anymore :)

But thanks for your input, everybody!

MarshallOfSound commented 7 years ago

@RadGH The topic is closed however still open for discussion, unsubscribe if you don't want to be notified anymore 👍

@timmevandermeer Yes, it doesn't cover all scenarios, I have seen in testing that their are some instances where certain keyboards or emulators like AHK or Razer's configurator don't dispatch windows media events but do dispatch key events.

I will do more testing to see how widespread that scenario is and go from there

MattJeanes commented 7 years ago

My keyboard is the Microsoft Sidewinder X4, if it helps.

Kytech commented 7 years ago

@MarshallOfSound, if the keyboard isn't dispatching windows media events, I'd expect that to be an issue with the keyboard driver. Using @timmevandermeer 's method, I no longer experience the repeat issue. If someone is using one of the AHK or Razer configurators that don't dispatch windows media events, a setting could be added that forces detection of key events. That might explain why some users never experienced the problem in the first place.

MarshallOfSound commented 7 years ago

That might explain why some users never experienced the problem in the first place.

This is one of the main reasons I'm hesitant to make this change. I know for a fact that <100% of users had the double hit problem (me included). This means that one of the techniques works and the other doesn't. Will look into this at the weekend

navossoc commented 7 years ago

Same here, double key pressing... I didn't have this issue with the previous/next track keys.

Not sure when started, but I think in this last release 4.2.0.

@timmevandermeer Is there any build available to test?

Kytech commented 7 years ago

@navossoc I have a build with the fix @timmevandermeer used that is working on my fork. I can't seem to get the installer to build, but the main program is building just fine. You can download a zip of my build from from the releases page here.

navossoc commented 7 years ago

@Kytech I made a few tests here, seems to solve the problem (at least for me).

Kytech commented 7 years ago

@MarshallOfSound, I just did some testing with the latest release version of AHK, and it is dispatching Windows media events to the system. I used the following .ahk script that uses the Ctrl+Alt+p combination to dispatch the Windows Media Play/Pause event.

^!p::
    send, {Media_Play_Pause}
Return

I've found that AHK is dispatching the media events consistently in full screen UWP apps and games, and full screen desktop apps, including DirectX and OpenGL based games when I use this script. The hardware media keys are also dispatching the Windows media events in all of the scenarios I've tested. Could you provide more details on scenarios where the media events aren't being dispatched?

dankimmel commented 7 years ago

Is anyone working on this? I'm still seeing the same behavior when I use the play/pause buttons on my headphones (version 4.4.0 of the player, version 10.12.6 of macOS), and am confused why this issue is closed.

Kytech commented 7 years ago

@dankimmel, I've looked into it here and there, but my work and schooling have taken a lot of time off my hands. There is an electron module in development that will eventually resolve the issue, though that will take some time to be released and implimented. Work has been done periodically to try and work out a patch for this program, though I can't make any guarantees about when.

jostrander commented 6 years ago

What about using the _.debounce function from lodash to ignore the second key press? I know this issue doesn't apply to everyone including me, so it's hard for me to test.

PhaedrusAD commented 6 years ago

I know this is an old thread and it has been closed as well as all of the duplicate threads created about the issue, but I am experiencing this in 4.4.1. Windows 10 x64 Pro, OS up to date. Logitech Orion G810 keyboard. I downloaded the Version 4.2.0.1 (Bug Fix) and that fixed the issue. Not sure what changed, was just wondering if/hoping it might be fixed again the in next patch.

myfrom commented 6 years ago

I can say that this bug hasn't yet been fixed on 4.5.0 either. I use Windows 10 and Microsoft Surface Bluetooth keyboard .

I have an idea of possible solution that would be universal:
How about introducing a timer in function that handles media key presses. Like a 10ms timer triggering on every press. If a press is detected and timer is already running, it gets ignored. If I understand the problem correctly, it would prevent duplicate events as they come immediately after each other and shouldn't block human interaction because 10ms is very small amount of time.

KensonPlays commented 6 years ago

This same thing is happening. I have tried both the keyboard Fn + Play/Pause, and my Elgato StreamDeck with a play/pause button assigned, they both double press, I want to pause music, it pause/plays, I want to play music, it play/pauses.

VERY irritating.

rockgenius85 commented 5 years ago

In case anyone finds this, I was able to fix it by disabling the chrome addon.

Gelunox commented 5 years ago

I'm also having this issue, running version: 4.6.1, I don't have, and never had, the chrome addon. Play/Pause sometimes works, but next/previous is always registered twice.

agc93 commented 5 years ago

Is this still being worked on at all?

I'm running latest version and using media keys still sends double events from keyboard media keys (Surface Bluetooth in my case).

@Kytech's patch seems to work, but is now several minor versions out of date, ~and there doesn't seem to be any sign of more upstream work on this bug? (or at least I couldn't find another issue tracking this)~

EDIT: I stand corrected, looks like this is now being tracked over in #2364 but looks like its still WIP :(

Kytech commented 4 years ago

That might explain why some users never experienced the problem in the first place.

This is one of the main reasons I'm hesitant to make this change. I know for a fact that <100% of users had the double hit problem (me included). This means that one of the techniques works and the other doesn't. Will look into this at the weekend ... their are some instances where certain keyboards or emulators like AHK or Razer's configurator don't dispatch windows media events but do dispatch key events.

@MarshallOfSound I think this might be caused due to the current Media Transport Control implementation using the old MediaControl API which Microsoft deprecated instead of the new SystemMediaTransportControl which Microsoft intends to replace the MediaControl API. This new API might have addressed this issue under the hood.