Beluki / GaGa

A minimal radio player for the Windows Tray.
35 stars 13 forks source link

Looking for Feedback: Hotkey and Mediakey handling. #2

Closed joshimoo closed 9 years ago

joshimoo commented 9 years ago

Hello Carlos,

Please have a look below for my rudimentary implementation of global hot/media keys. As discusses I implemented the Hooking functionality in a separate library. Please send me pull requests for the library: https://github.com/joshimoo/GlobalHotkeys

The code is not ready for release yet, but I wanted to get your feedback on the current implementation? I accidentally hit Format Document and then committed the changed format, my code starts at the bottom.

Future Features:

Please have a look at the TODO Comments in the code for some improvements for GaGa. Please let me know if I should work on any of these?

My TODO List:

For the user defined Hotkeys

The Media keys need some more work.

Best regards Joshua

Beluki commented 9 years ago

Some thoughts:

I think its best to focus on the hotkey library itself being solid, rather than on how it should be integrated in GaGa. That said, in my honest opinion, I believe the best way to go about it is by not making the hotkeys configurable at all. No JSON, config files, etc... Just mimic the mouse interactions with the play, stop and mute multimedia keys. Play would be analogous to left click, stop to right click, mute to middle click.

Explicitly:

This allows to do all the useful operations in GaGa, including opening the menu with the hotkeys to select a different radio and navigating to the volume submenu to change it, while being way less complex than the alternatives.

(note: the additional settings file I want to add its just to remember things like the last volume/balance levels and URI played. It will be a binary file)

On the library itself:

I really like the interface to it. Just .RegisterHotkey() and handle an event. That's pretty much perfect. I took a look at the library source and I didn't like the fact that it creates an internal window. Is this really necessary? Perhaps we could use the SetWindowsHookEx API instead of RegisterHotkey.

Beluki commented 9 years ago

I think I'll try to implement it using the SetWindowsHookEx API.

Beluki commented 9 years ago

I've been experimenting with this. I got the multimedia keys working but:

Unfortunately, there's no reliable way to show the context menu from a global hotkey at the correct position. There are workarounds, such as Shell_NotifyIconGetRect, but further problems (e.g. switching focus back to the application that had it before the key was pressed) are harder to tackle and start requiring ugly hacks I'm not quite comfortable with.

So currently there's two options: make the hotkeys configurable as you proposed, or just have them play/stop/mute/change volume, without showing the menu. It's quite a lot of work... but I'm thinking about this (INI syntax):

Control + F4 = VolumeUp
Modifier + Key = Command
Key = Command
etc...

With the possible addition of:

Key = http://radio-url

So that we can bind keys to specific radio streams.

joshimoo commented 9 years ago

Hello Carlos,

I taught a bit about this and there are two issues.

The one issue are the media keys, which you seem to found an acceptable solution too. Are you hooking the WM_AppCommands then processing them inside of the app, or which messages are you hooking? Since they are not required to be user defined, we can just implement the functionality.

The other issue are user defined hot keys, which we could use the ini for. I would personally use Command (Identifier) = Value instead of Value = Command (Identifier) This would allow us to move your binary settings also inside of the ini files.

DefaultVolume = 0.1
DefaultBalance = 0.5
IncreaseVolume = Modifier + Key
NextTrack = Modifier + Key
TogglePlay = Modifier + Key
MuteVolume = Modifier + Key

# Could ether be in the ini or have a tag inside of the streams file
# I would prefer a tag in the stream file, but the ini is fine too
FavoriteStation1 = http://radio-url 
PlayFavoriteStation1 = Modifier + Key

edit: accidentally closed edit2: if you push a feature branch with your code, I can comment on specifics there if you want?

Beluki commented 9 years ago

Hi!

Despite the fact that I created a MediaHook repository, I'm actually working on configurable global hotkeys. Depending on how things progress, I'll choose either option. Instead of an INI file, my current approach involves a custom submenu, like the balance/volume one.

Here is a preliminary screenshot:

About the favorites... I'm currently thinking on having a hotkey to cycle between them. The idea is: right click on a stream adds it to a favorites submenu. On the favorites submenu, left click plays it, right click deletes from the submenu. On the hotkeys submenu, there's a hotkey to play the next favorite (or the first if the current stream is not a favorite).

To be honest, I don't know yet what's the best approach. It's hard to find a balance between "too minimal to be useful" and "too complex/convoluted".

On the binary file moving to INI... I think that's a route that we should not take. Consider adding say... statistics and other information that isn't really meant for the user to modify. If I end up choosing an ini file for the hotkeys though, your syntax is definitely better than the other way around.

About what I'm currently hooking: WM_KEYUP and WM_KEYDOWN from SetWindowsHookEx(WH_KEYBOARD_LL ...) + GetAsyncKeyState() to check modifiers. I'll push that soon. It will be in GaGa independently of using multimedia keys, configurable global hotkeys, a submenu or an INI.

Beluki commented 9 years ago

I've created a repository for the hooker. I'll be working on making it as solid as possible before using it in GaGa.

Any further thoughts on how to integrate it with GaGa when finished (submenu, additional INI, whatever) would be appreciated.

I would prefer a tag in the stream file

How would the tag work? Something like a specific [Favorites] section?

Beluki commented 9 years ago

Hi.

It's been some months and as you can see, I've decided to release a GaGa version with multimedia keys support (non-configurable). This will probably be the last GaGa, mINI and LowKey versions from me, unless there are bugs.

I understand that you put a lot of work into this pull request (and into the global hotkeys-discussion). I'm sorry for this but I want to move on and work on newer projects. I hope this decision doesn't stop you from contributing to other open source software in the future (or to fork GaGa if you want to).

Thanks for your work.

joshimoo commented 9 years ago

Hello Carlos,

Thanks for letting me know, I wish you all the best for your future endeavors.

Best regards Joshua