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 an option to Ignore hardware media button events #9746

Merged
merged 2 commits into from
Feb 26, 2023
Merged

Add an option to Ignore hardware media button events #9746

merged 2 commits into from
Feb 26, 2023

Conversation

NyanCatTW1
Copy link
Contributor

@NyanCatTW1 NyanCatTW1 commented Feb 4, 2023

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Adds an option called "Ignore hardware media button events", which disables any pause/play and the prev/next track buttons, including ones on a headset. It helps prevent accidental touches and makes the app more usable on a headset with broken buttons.

Before/After Screenshots/Screen Record

The option is inside the Video and audio category.

  • Before:
    image

  • After:
    image

Fixes the following issue(s)

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Thanks! I don't think it's a good idea to add the BLUETOOTH_CONNECT permission. Unfortunately unless there is a different way to tell media button events apart, I don't think this will be merged.

@NyanCatTW1
Copy link
Contributor Author

Actually, it seems like only hardware media events go through the custom handler. Therefore, it should be safe to just not distinguish them.
I will make the changes in two or three days

@Stypox
Copy link
Member

Stypox commented Feb 7, 2023

Ok, thanks. So the setting description should be updated to explain that all hardware actions will be blocked (including bluetooth ones). Then it will be up to users to choose if they want to enable this setting or not.

@NyanCatTW1 NyanCatTW1 changed the title Add an option to Ignore headset media button presses Add an option to Ignore hardware media button presses Feb 9, 2023
@NyanCatTW1 NyanCatTW1 changed the title Add an option to Ignore hardware media button presses Add an option to Ignore hardware media button events Feb 9, 2023
@NyanCatTW1
Copy link
Contributor Author

I made a mess with the git history on the fork, but it should be ready for preview now
It now ignores all events that go through the MediaButtonEventHandler, which should hopefully be exactly what's needed to achieve the desired behavior

@SameenAhnaf SameenAhnaf added feature request Issue is related to a feature in the app player Issues related to any player (main, popup and background) labels Feb 12, 2023
@Stypox
Copy link
Member

Stypox commented Feb 26, 2023

I pushed some improvements to make code shorter and a bit more performant. I tested with a bluetooth speaker and the hardware buttons get correctly blocked when the setting is on. Are the changes I pushed ok, in your opinion? Thanks!

@sonarcloud
Copy link

sonarcloud bot commented Feb 26, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@NyanCatTW1
Copy link
Contributor Author

Looks nice! So I suppose it's ready for merge now?

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Yes, thanks for confirming :-)

@Stypox Stypox merged commit 3cb76e4 into TeamNewPipe:dev Feb 26, 2023
@Stypox Stypox mentioned this pull request Mar 1, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issue is related to a feature in the app player Issues related to any player (main, popup and background)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an option to disable prev/next track buttons on headsets
3 participants