Rockbox

Tasklist

FS#7307 - Keyclick feature

Attached to Project: Rockbox
Opened by Steve Bavin (pondlife) - Thursday, 14 June 2007, 09:03 GMT
Last edited by Steve Bavin (pondlife) - Monday, 21 January 2008, 09:59 GMT
Task Type Patches
Category Applications
Status Closed
Assigned To No-one
Operating System SW-codec
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

This patch adds a keyclick option. IMHO, this makes unsighted operation easier, and helps with the Gigabeat's touchpad.

Improvements which could be made (as of keyclick_3.patch, below):

1) Speed up response by triggering the beep when the button is pressed, rather than when the button event is dequeued. I couldn't see a way to do this without calling /apps code from /firmware code and whilst this works, it's bad and the response improvement is small.

2) Play a different pitch for each button. To do this would require each device has a button code to button number/pitch map. I did attempt to map action codes to pitches, but this makes tha actual pitch context-sensitive, and I suspect that would be more confusing than useful.

3) The click doesn't occur when paused (e.g. when unpausing). This needs fixing by a more general revamp of pause mode (needed to get voice support working better anyway) and is not in scope for this patch.

4) The click is affected by the main volume and fades. The former is probably a good thing, the latter not so, but I can't see a way to resolve this.

5) Make it work on HWCODEC? I don't think this is possible.
This task depends upon

Closed by  Steve Bavin (pondlife)
Monday, 21 January 2008, 09:59 GMT
Reason for closing:  Accepted
Additional comments about closing:  Committed.
Comment by Steve Bavin (pondlife) - Thursday, 14 June 2007, 09:04 GMT
One other thing - I have no scrollwheel device. This definitely needs testing on such a beastie. I've no idea what will happen at the moment (probably lots of unwanted beeping).
Comment by Jonathan Gordon (jdgordon) - Saturday, 16 June 2007, 12:49 GMT
Just tried this on my sansa... it only does one click whent he wheel is turned, and i tihnk it actually feels more wierd than if it did click again every time the wheel clicked.

also, the click sound is a bit odd...
Comment by Stephane Doyon (sdoyon) - Friday, 02 November 2007, 13:43 GMT
The choice of having it click on keypresses (not releases and not
repeats) is interesting. It does make sense in that it better simulates a
kind of hardware / direct physical response. However as I tried it out
(only for a few minutes admitedly) it seemed to me that I might prefer
having it click on actual action events. That way clicks more closely
correspond to recognized events and it less often clicks without a direct
accompanying action. That's pretty subjective at this point
though. Having it click on events gives blind users an idea of the repeat
rate, might tell you more precisely when a LONG press has been held long
enough, and gives much better feedback on the e200 scroll wheel, although
it's a bit noisy. In fact, I don't find it all that useful on most
buttons but I like it on the scroll wheel in particular.

I feel the beep is still not very discreet, so I've changed the
parameters to make it more like a click. I've also reduced the volume: it
seemed pretty loud to me with beeps set to weak. BTW the beeps/keyclick
options should probably be moved out of the playback menu, and moved into
the system menu perhaps.

Now for the problems:
-The keyclicks are often not played, most likely because of the voice
shutups, in particular when moving up/down a list just fast enough so
items are not fully spoken. The inconsistency of not always hearing the
keyclick is a bit annoying. Perhaps the pcm buffer can be flushed to
prevent this somehow.
-Starting and stopping playback also often interferes with the
keyclick. It's usually not heard on stop, and it's paused in the middle
of the click when changing tracks which doesn't sound too nice. Again
flushing / waiting for the pcm buffer might help?
-I had some weird lockup and/or pcm buffer or voice UI disfunction when
going through values quickly in some sound setting menus or when skipping
several tracks quickly. This may be triggering some race condition...
needs more investigation.

In general, I find it a bit noisy, but I do like the extra feedback on
the scroll wheel, in particular on repeat events: let's you count skipped
items when you move quickly enough that the voice is suspended.

Here's my updated patch. Now this is modified: triggers on actions
instead of presses and has a more discreet beep sound. So anyone wishing
to check this out should also try the original.
Comment by Steve Bavin (pondlife) - Friday, 02 November 2007, 16:31 GMT
Hi St├ęphane,

I've made a hybrid of the two patches, basically yours but triggering on the initial press only and earlier in the code, so it's triggered immediately on the keypress, rather than once the action has been determined; that can take a while while it determines between a short and long keypress.

This works much better for me on an H340, but I have no wheel device to try it on; that may make a big difference to the feel.

The setting still needs moving to System and maybe also should update the heard keyclick volume as the user scrolls through the volumes (it will always be one keypress behind, but better than nothing).
Comment by Steve Bavin (pondlife) - Wednesday, 14 November 2007, 19:43 GMT
Here's an update that puts the configuration into the System menu, and leaves the old beep setting alone (that might be removable later, of course). There's an option as to whether repeats should click or not too.
Comment by Steve Bavin (pondlife) - Thursday, 15 November 2007, 08:40 GMT
I'm stupid. There's no need to have a separate enable option - the volume control will do nicely.
Comment by Steve Bavin (pondlife) - Thursday, 15 November 2007, 10:05 GMT
JdGordon reports that this makes a "brain-frying noise" when used on Sansa. Investigation continues as to why, but it's been tested fine on Gigabeat F20 and Iriver H340.

You have been warned.

iPod testers are welcome!
Comment by Steve Bavin (pondlife) - Thursday, 15 November 2007, 11:41 GMT
The nasty noise seems to happen on iPod targets, but seems to be caused by calling pcm_play_data(NULL, non-NULL, 0). :/

This version works around this as I don't have a PP target to debug this on. It should be fixed properly, of course.
Comment by Steve Bavin (pondlife) - Monday, 19 November 2007, 10:09 GMT
Resync (of v3).
Comment by Jonathan Gordon (jdgordon) - Wednesday, 21 November 2007, 05:50 GMT
I cant remember if I told you on IRC or not....
just tryed _5 and the squeel is still there.
Comment by JerryLange (psycho_maniac) - Sunday, 25 November 2007, 08:27 GMT
this would be a very nice patch. i could help you test also now as i have a ipod video 80gig and a gigabeat F40.
Comment by Steve Bavin (pondlife) - Tuesday, 27 November 2007, 07:36 GMT
Resync to r15832.
Comment by Steve Bavin (pondlife) - Tuesday, 27 November 2007, 08:43 GMT
Hi Jerry, yes - testing and feedback would be very welcome.

It should be fine on the Gigabeat, but may cause nasty noises on the iPod Video, like it does on the Sansa. These aren't problems with the keyclick per se, more that the underlying code doesn't cope well with short and/or overlapping beeps. Or so I believe...

I'd like to commit this regardless, in the hope that it will inspire someone to fix up the PortalPlayer pcmbuf_beep code.
Comment by JerryLange (psycho_maniac) - Wednesday, 28 November 2007, 02:57 GMT
I think this has to be re synced again because i cannot get it to compile for the gigabeat at this moment.
Comment by Steve Bavin (pondlife) - Wednesday, 28 November 2007, 10:55 GMT
Hmm, seems fine here, both patching and building for Gigabeat F. What errors are you getting?
Comment by JerryLange (psycho_maniac) - Thursday, 29 November 2007, 05:36 GMT
could you maybe upload the build here ;) jk for some reason now i got it to work. strange :S
Comment by JerryLange (psycho_maniac) - Thursday, 29 November 2007, 06:18 GMT
tried it on the ipod. i get the long beeps as well. also my music stops playing whenever i scroll for a long time. maybe like 2 pages.
Comment by Steve Bavin (pondlife) - Friday, 21 December 2007, 13:40 GMT
Resync. I'd REALLY like to commit this, and then hope that the beep-of-doom will inspire someone who knows pcmbuf to fix the PP beep code.
Comment by Jonathan Gordon (jdgordon) - Friday, 21 December 2007, 18:20 GMT
go for it, just put in a nice warning message in the commit message? (or disable it by default)
Comment by Steve Bavin (pondlife) - Monday, 14 January 2008, 11:07 GMT
Resync.

Loading...