Skip to content

Add threshold config options for prev_or_restart action#3319

Closed
tanguycano wants to merge 1 commit intoDeaDBeeF-Player:masterfrom
tanguycano:configurable-prev-restart-threshold
Closed

Add threshold config options for prev_or_restart action#3319
tanguycano wants to merge 1 commit intoDeaDBeeF-Player:masterfrom
tanguycano:configurable-prev-restart-threshold

Conversation

@tanguycano
Copy link
Copy Markdown

The restart-vs-go-to-previous threshold in action_prev_or_restart_cb was hardcoded to 3 seconds. This makes it configurable via playback.prev_restart_threshold (float, in seconds), defaulting to 3 to preserve existing behavior.

Increasing this value is useful when triggering the action via "smart shortcuts" or "touch controls" on Bluetooth headsets, as some of these input schemes require multiple or long key presses that take longer to complete.

@Oleksiy-Yakovenko
Copy link
Copy Markdown
Member

Hi, Can you modify the PR to just set it to larger value?
Which value is preferred?
I think 3 seconds was quite arbitrary, and I understand it's not optimal.
But adding a configuration option for that is an overkill.

@tanguycano
Copy link
Copy Markdown
Author

I have it set to 10s on my machine, but that's also pretty arbitrary.

Some tracks also start slow, silent or generic enough that you can't actually tell whether you've rewound or skipped to the previous track until many seconds into the playback. It really depends on the content you are listening to.

Therefore, I'd still push for a configuration option, but it obviously is your call. Else, I will just rewrite the constant to a bigger value, such as 10s.

@Oleksiy-Yakovenko
Copy link
Copy Markdown
Member

I decided not to wait, and committed the change to 10 sec.
Sorry for not taking your PR as is - I'm just not a fan of secret config options without UI configuration.
(Also, as I stated, this behavior doesn't really need to be configurable)

@tanguycano
Copy link
Copy Markdown
Author

No worries, I should have resubmitted it with a 10s hard-coded value anyway.
Thanks for taking the time to make the change.

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.

2 participants