Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for next/previous/pause/resume global hotkeys/actions #2239

Merged
merged 1 commit into from
May 23, 2022

Conversation

absidue
Copy link
Member

@absidue absidue commented May 12, 2022


Add support for next/previous/pause/resume global hotkeys/actions

Pull Request Type

  • Feature Implementation

Related issue

Related to #2138
Closes #1931

Description

This pull request adds support for the global next and previous track hotkeys/actions inside of playlists. As I'm using the MediaSession API this also works with the next and previous track actions on my Bluetooth headphones.

As there have been issues with the play/pause hotkeys on non Windows operating systems in the past, I've also added explicit handlers for those. I also made it explicitly report the playback state (playing, paused, none) to the MediaSession API. I can remove these handlers and explicit reporting of the playback state if we are sure that those issues have been fully resolved.

If play/pause issues persist then we can be certain it's either Electron or more likely, an issue with something outside of FreeTube on the Linux side.

Testing (for code that is not small enough to be easily understandable)

I tested this with the next and previous keyboard buttons as well as the actions on my Bluetooth headphones. By adding log statements to the event handlers i was also able to confirm that they are removed properly when they are no longer required.

Desktop (please complete the following information):

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 2809d81

@PrestonN PrestonN enabled auto-merge (squash) May 12, 2022 16:18
@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added the PR: waiting for review For PRs that are complete, tested, and ready for review label May 12, 2022
@efb4f5ff-1298-471a-8973-3d47447115dc

closes #1931 right?

@absidue
Copy link
Member Author

absidue commented May 12, 2022

It might do but I don't use linux, so I don't know if this fixes it.

@absidue
Copy link
Member Author

absidue commented May 13, 2022

@x-N0 Could you please test if this pull request fixes your issue?

@absidue
Copy link
Member Author

absidue commented May 22, 2022

As we haven't heard from @x-N0 yet I think you can probably review it without their response and if their issue is still not fixed that can be handled separately at a later point in time.

If their issue is still not fixed by this pull request then it's either an issue with Electron or their setup.

@efb4f5ff-1298-471a-8973-3d47447115dc

Is it possible for u to create a build for this so i can download it

@absidue
Copy link
Member Author

absidue commented May 22, 2022

Here is a build for this pull request: https://github.com/absidue/FreeTube/actions/runs/2367039517

Copy link
Member

@PrestonN PrestonN left a comment

Choose a reason for hiding this comment

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

Looks good to me. Also wanted to confirm that this does work on Linux (Using Fedora + Gnome). I'll let others get a chance to look over this as well but I'll probably go ahead and merge this in later today.

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

efb4f5ff-1298-471a-8973-3d47447115dc commented May 22, 2022

I can confirm that this does close #1931.
Used fedore36 to test this
I could pause the video with my Bluetooth headphones but not unpause it. Previous and next video in playlist also works on fedora with and without the bluetooth headphones. Im not sure if the unpausing of the videos is a compatibility issue or coding issue.

@absidue
Copy link
Member Author

absidue commented May 22, 2022

I'm going to assuming it's a compatibility issue as the code uses the standard Media Session play and pause events. At some point in the future I can create a Fedora VM and try it but no promises, as that will take a while as I've never used Fedora before and I have no idea how well Bluetooth headphones work together with VMware and Fedora.

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

efb4f5ff-1298-471a-8973-3d47447115dc commented May 22, 2022

Warning!!!
DONT CREATE A VM TO TEST THIS PR

u cant pass those keys through the vm because the devs of vmware, vbox and all the other vm makers think its not a priority

Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Tested play/pause/next track/previous track with WH-1000XM4 on MacOS 12.1

@PrestonN PrestonN merged commit 0146a63 into FreeTubeApp:development May 23, 2022
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label May 23, 2022
@absidue absidue deleted the media-keys branch May 23, 2022 08:03
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.

Add Bluetooth headphones commands support.
4 participants