-
Notifications
You must be signed in to change notification settings - Fork 173
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
Added extended headset controls (headset button click handling) #1218
Open
sandreas
wants to merge
11
commits into
advplyr:master
Choose a base branch
from
sandreas:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
7ba1ebf
Clickhandler works partially, I need support on the rest
sandreas bfb571a
Clickhandler works in all tested scenarios
sandreas 595ac55
Merge branch 'advplyr:master' into master
sandreas b0c5588
removed useless currentPlayerDispatcher property
sandreas 25c1cec
Implemented fastForward, adjusted timings
sandreas 56cd667
reverted unneccessary formatting changes, added experimental headset …
sandreas dc8d0cf
added enableExperimentalHeadsetControl as a setting
sandreas c24db68
Renamed "experimental" to "extended", fixed issues with extended head…
sandreas 1ee23ad
Fixed translations, reduced seekBufferTime for fastForward and rewind
sandreas d1cd7bc
Moved headset settings in an extra category
sandreas 4f653a7
Update android/app/src/main/java/com/audiobookshelf/app/player/MediaS…
sandreas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not understanding the logic here. How do we know that KEYCODE_MEDIA_NEXT/PREVIOUS are clicks and not a separate next/prev button press?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@advplyr
Android is handling specific shortcuts not emitting exact events, but overriding them by combined events:
This cannot be prevented, because it is a system / kernel behaviour.
So double-clicking does NOT emit 2 raw click events, but 1 single (automatically rewritten) event
KeyEvent.KEYCODE_MEDIA_NEXT
.I handle
KeyEvent.KEYCODE_MEDIA_NEXT
as if 2 single clicks had happened and increase the click count by 2. Same for the triple-click, which happens to be emitted asKeyEvent.KEYCODE_MEDIA_PREVIOUS
by Android instead of 3 single keydown/keyup event-combos.This results in a more flexible API, since you can configure behaviour based on click count now and are not required to work around pre-defined behaviour. You can also see this in the
how does it work
comment, the mentioned problems there are kind of fixed / unfixable - I tuned the timings as good as possible, but I need feedback from other device owners.Best way would be to try it out (by using a headset) monitoring the logs. If you don't own the required hardware to test it yourself, let me know, maybe we can arrange something.
KEY_DOWN
// wait 1248ms
KEY_UP
on Android 7.0 devices is
// nothing
// wait 1248ms
KEY_DOWN
,KEY_UP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that these events are emitted by the bluetooth device and so depending on the bluetooth device it might emit a KEYCODE_MEDIA_NEXT on 2 clicks if it is setup for that.
Or you might have a bluetooth device that has separate buttons next to play/pause that is for KEYCODE_MEDIA_NEXT.
Did you find any documentation on these KeyEvents that say when/how they are triggered?
I think this needs to be tested on a bluetooth device that has NEXT/PREV media buttons. I'll see if I have one. I think many bluetooth in vehicles have this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@advplyr
Ah, now I understand. You mean a device like this - a car bluetooth receiver with multiple buttons (e.g. for
play/pause
,next
andprev
).Well, first things first: The PR is NON-DESTRUCTIVE, so that there are no behaviour changes, as long as the according setting (
extended headset controls
) is disabled, which it is by default. So no fear of breaking anything, that works fine at the moment.I own such a car bluetooth recevier with buttons for
play/pause
,next
andprevious
and tested it. It shows the following behaviour (which I would have expected):play/pause
next
prev
So I think it works like expected. Even better, on bluetooth devices with A SINGLE button, you can now use it for
next
andprev
.The only unexpected behaviour might be that multiple fast
next
clicks won't do anything, as long as the timer is not hit - you have to click slower, but since this feature is disabled by default, I don't see this as a huge problem.