Rockbox

Add approx Hz conversion and test plugin

To emulate the clicker sound produced by RetailOS:

Sound needs to have a period of 26 or 27, for a duration of 0.004 or 0.005 seconds. I don't know any way of making Rockbox sleep for that short a period (also the sleep function always seems to do a sleep + 1), without changing the definition of HZ. Setting HZ to 1000 produces some very funky button handling.

Whether we should be slavishly trying to emulate the sound of RetailOS is another matter... but I think a default beep and click would be good, just not sure what settings to use.

I have worked a little more on the patch, it sleeps with udelay for RetailOS like sound. The sleep frequency and length may be configured in the menu (when the length is 0 it is silent). (patch against cvs from 2006.7.24). (does not include the test_piezo.c plugin code).

New version of piezo.diff without the keyboard junk and another patch (to be aplied after tracker - the old piezo.diff) for sleep using timer instead of udelay which should be snappier.

Doesn't beep now during button repeats. Also remove what I think are some unused calls to button_beep.

It also beeps during volume change in the WPS - I haven't yet looked at trying to stop this.

Thanks Rob ;)

Infact obo, piezo.c and piezo.h seem to be missing in your latest patch. Have they been altered since you're old patch?

Oops - sorry about that - I was using yet another source tree and had forgotten to "cvsdo add" them. piezo.[c|h] are unchanged from freqmods last diff, but I'm attaching a new file for completeness :)
I've also wrapped all the piezo functions in button.c with #ifdef HAVE_PIEZO - makes it compile cleanly on other platforms. Could have just done a (void)tpe, but didn't see the point in calling something that did nothing.

Obo, freqmod's piezo.c/h still aren't working on target. I've gone back to using your old piezo.c for now.

We've had some more fun testing the piezo with Mikachu this afternoon.

Looks like the piezo commands can be decomposed as:
0x80000000 : sound on if set / off if 0
0x7f000000 : doesn't seem to have any effect
0x00ffe000 : seems to be some kind of wave form modifier (not sure about 0x0000e000 though)
0x00001fff : frequency

You can find a small test plugin here: http://people.videolan.org/~dionoea/ipodtest.c

This patch no longer compiles, I'm guessing it has something to do with the thread scheduler.

I have updated the piezo patch for the new scheduler. The priority of the piezo thread is set to PRIORITY_USER_INTERFACE (same as the rockbox UI) because the clicks need to be in sync with the actual scrolling/screen update.

Updated for the ipod target tree.

where r the settings for this?

General Settings -> Display Settings -> LCD Settings

Button click duration
Button click frequency

Has anyone had experience with this conflicting with other patches? I'm trying to figure out which one it is.

ok, it conflicts with patch http://www.rockbox.org/tracker/task/5744

I apologize but my last comment was incorrect. It does NOT conflict with the USB connection patch. What the issue is if you have the iPod LCD brightness patch (http://www.rockbox.org/tracker/task/5594) that setting must be all the way to 15 or the piezo will not work.

Add support for the "wave form modifier" - can be used to adjust volume.

Remove the microsecond timer.
Add check on piezo_button_beep to prevent the queue overflowing - any better ways to do this?

using this new patch, what values should the duration and frequency vars have so the click sounds like the original ipod?

Anyone object to applying this patch to trunk? (If not, I'll apply it)

resync

resync

I'd really like to have this stuff commited soon, but I have two gripes with it right now:
1. Beeps are too long. Yes, I know I told you not to use the user timer for shorter durations, and we still can't do that, thanks to backlight fading also using it. Any other ways of doing it?
2. I don't think the options are necessary. A retailos-ish beep should really be enough.

1 - is better now, but maybe not perfect.
2 - changed the options to a bool only. Retailos pitch and duration may need tweaking.

Lots better, the bool option we can't really get rid of, so that ok :) The beep sound is lots better now, but now it's perhaps a bit on the inaudible side? I also have some weird effects where the sounds changes a bit if you do successive beeps.

There seems to be an issue with the current patch version and the most recent svn checkout. Patching english.lang fails the hunk. After patching by hand, everything compiles, but there are no menu items relating to the piezo, and no beeps. Thoughts? Maybe just a sync would fix this.

does this patch conflict with slider progress bar?

Please re-syc the patch, repeared !

sync

Is there any way to change the way piezo sounds, on my Ipod it dosent sounds like the original apple OS, and the sound is very low.
Maybe a menu option or just to say what and where to edit the patch.

Please re-sync the patch?

When you scroll a little faster, it starts to sound little different, and not cool at all.

This is a shameless bump to the thread, as I have no coding skills of my own, but would really like to see this patch picked up again by someone (hopefully to the point where it could be merged with the main build). Is there any chance that any coders with an ipod find this feature as good an idea as I do?

I've updated the patch to r17730

Sweet, someone else at least interested enough to update for compatibility! I assume it's working the same as the old version did? I don't suppose you're interested in working on the patch more?

Either way, thanks so much for syncing it. Much appreciated!

Yes, very very much thanks for syncing it. When will this patch be in SVN, because in my opinion it's stable enough. I think this is also very useful for blind people (the current keyclick function in rockbox using the headphones doesn't work well on the ipods (at least on my nano)).

I dont tihnk it should be commit as is just yet.
1) it should probably be HAVE_HARDWARE_CLICK or something instead of piezo
2) the setting should be similar to the ipod OD where you can click in the piezo/headphones/both
3) it should be handled in the same way and places as the software click

I agree with jgordon, it's not ready for svn yet, but I would love to see someone passionate about the feature take a look into his suggestions. It's definitely important that it gets handled similarly to the current software click, and the two would probably get grouped together in the same location to toggle one or both on, as he said in 2.

Changing to hardware_click makes sense to me on more than on level, since before I actively searched for this topic I had no idea what piezo was... maybe that's not necessarily keping people from realizing this feature has been worked on, but it kept me in the dark for a while.

And of course, calling it hardware click makes sense with the other kind being a software click.

I wish that I could do more to help see this in svn, but I have trouble even applying patches, let alone writing them. I don't suppose there's anything non-code related I could do to help out?

I'm actually interested in this patch too, just for completeness. I think one big problem is that it doesn't sound 'nice', according to one of the comments above. I hope someone can step up and try to fix the issue.

Disclaimer: I haven't tried the patch yet, so I don't know how "different" it is from the OF. We certainly need confirmation on this one.

I've changed the define to be HAVE_HARDWARE_CLICK, moved the options in the Keyclick menu as a bool to "Enable Speaker", and placed the beep with the software keyclick in actions.c

It does sound different than the standard iPod click, but personally I don't mind it. I might play with it later.

That should be "action.c" above, and on closer listening between the two - the difference is barely noticeable (or I'm imagining it)

The most ironclad test would probably be to have an ipod and a rockbox'D ipod side by side, to hear if there's any pitch/duration difference (rather than trying it, rebooting it in the other firmware and trying it again).

And once again, thanks for taking the time to work on this. It's honestly the only thing I miss about the normal firmware.

Okay. I did it. I finally, finally figured out how to make my own build, and apply patches. Sweet. I've tested the latest patch, and while the click (when it works) sounds pretty okay, I'm seeing a few problems on my end.

1) Is there a possibility of hearing the click only through the speaker, or through the headphones? It would be nice to have the option to select none, either, or both (even though the OF doesn't allow this, to my knowledge, it would be kinda neat).
2) Is there a way to disable/adjust the click when adjusting the volume/doing other WPS things? The click can disrupt the sound a bit, and with crossfade turned on, having a click sounding on track change becomes semi-unnecessary (unless you definitely want to know that the track change was registered without looking at it). I know that for the OF, the clicks still occur when changing volume, but they seem to have made them less frequent when doing so.

Those two are more of a personal preference, but the last one is an actual problem. I'm still getting weird beeps when I scroll too fast, only now it's coming from the speaker as well as the headphones. It's not particularly loud, about the same volume as the clicks, but more piercing and annoying. I guess the cause of this is still unknown? Is there anything that can be done about it at this point?

To explain myself before any comments come in, the reason I think it would be nice to have the option of hearing the click from speaker or heaphone individually is that when I use my player in the car, the clicks through the headphone jack are more distracting than the softer clicks I could potentially hear from the ipod speaker (and yes, I do realize that I can turn the headphone click down). It would be useful to me to have the click in this fashion.

The beep is caused by the piezo thread yielding for too long. If your feeling more adventurous, in file "firmware\target\arm\ipod\piezo.c" change line 106 from :-
yield();
to :-
if (duration >= 5000) yield();

If not, I'll make a patch file up for you soon - I'm just playing with another patch and have the source code in bits. I can look at reducing the frequency of the clicks, but at moment its in line with the rockbox software keyclick code.

At the moment, you can set keyclick off but set enable speaker to only hear it through the piezo speaker - or disable the speaker but set the keyclick from off to enable software keyclick through headphones. The options do need cleaning up.

My bad on the last part... I should have remembered that keyclick was already there pre-patch. The options do not need cleaning up, as you said. Although I still do think that since both options produce a click, it might make sense to simply use the "keyclick" option, and upon selecting it you would see something like
-headphones
-speaker
-both
-off

It's perfectly fine as-is though, so unless anyone else feels that way/my wording of it just changed your feelings on it, I wouldn't worry about it.

As to feeling adventurous, I may look into what you were talking about... it sounds like whatever you're doing with the source now is bigger than that, though, and I may just wait to see what you've got up your sleeve. Thanks again for the hard work.

Here is a new patch - I've just changed the option wording a bit. See what you guys think of it now. I plan to borrow my friends iPod nano to check the click sound next to each other.

I'll check this patch later today and let you know how it goes. Also, I'd just like to apologize for mistyping jdgordon earlier, and also that I could've sworn you said the options did not need cleaning up, when obviously you did say they needed cleaning up. I'll try and be a more careful reader in the future, haha.

This new patch is a crazy awesome improvement, if not simply for the killing of the beeps/reworking of the menu wording. My thoughts/observations:

1) With keyclick repeat on, holding down any button will make the device start clicking rapidly. This should probably be altered so that a single button press, even a held one, still produces a single click. This wouldn't be applied to the scroll wheel, since it needs to continue to click when doing things like changing volume (albeit less frequently, as I suggested) and scrolling through lists. I know that right now using the repeat mode probably isn't so conditional, but to add those tweaks would probably be useful, if they're doable.

2) Since 'keyclick' is the name of the overall setting menu, 'headphone click' and 'speaker click' are probably simpler and less repetitive than saying 'keyclick' again in full. probably works for 'click repeats' as well. Besides that, I think the new wording and setup looks much better.

3) When scrolling through lists (again, with repeat enabled), I was thinking about another handy tweak that might be likeable, if it could be done... on my super-duper cool phone (the newer chocolate), when I'm scrolling through lists, it makes a beep noise instead of a click when wrapping from the bottom back to the top (or vice-versa). This is another thing that, if implemented, would assist in navigating without needing to look down at the screen, as you can tell when you've wrapped around and can do things like scroll immediately to the bottom of a list and then four up to the song you want, just by hearing the beep and four clicks (the tone wouldn't need to be a beep, of course). Does anyone else think that would be useful, and is it doable?

I'm very impressed with all of the work. You're a machine! I'll be interested to hear how the current click stacks up against the nano's side by side. Please let us know, and I hope my comments are more helpful than annoying...

1) no... it shold keep beeping... I like how in the OF the pitch changes (havnt tryed this patch so dunno if we are doing that also)
3) yes this is doable, sort of... possibly the best thing we can do is beep when you get to the top/bottom of the list because in most lists it wont let you wrap untill you release the button

1) I'm not exactly sure what you're referring to in the OF. I still don't think the clicks should repeat in that manner... it's especially weird when doing something like shutting off the player (where holding down a button is necessary) and the ipod sounds like a mini-rattlesnake. Fast-forwarding a rewinding by holding prev or next in the OF also only make a single click, though changing volume does make multiple clicks (as previously stated).

3) Currently if you continue to scroll after reaching the bottom, the ipod continues to click away as if you're still scrolling through something. Ideally, this is where something should just tell the click to stop, rather than make a beep that could be lost in the current series of clicks. Once it stops clicking here, you would know to stop scrolling for the moment so one could wrap the list, and once the wheel is pressed again, when the code sees you can't go any further and loops back to the top, I think that a beep should be made (and scrolling up instead would just return to clicking). There has to be code in there telling the list to loop, so it sounds like it should be doable to make that loop produce the beep.

I'm not sure about the pitch change in the OF on the iPod nano - I notice the duration of the click changes on some actions and there was provision for this in an earlier patch. I'll have a look into the top/bottom list beep but would this also want applying to the software keyclick?

I think it would make sense to apply it to the software click as well, yes. I think that for all intents and purposes, it makes the most sense to keep the same features for both the speaker and software click (and hopefully take less work to maintain?). That will mean that people will choose the type of click based solely on where they want to hear it, and not because one has features the other does now. Not to mention it would be pretty weird to have both on and hear two different behaviors at once.

... "and not because one has features the other does NOT" is what I meant to say.

I think the beep when scrolling up/down is moving away from this task, however here is a patch with it roughly implemented for the hardware click only. "Keyclick Repeat" can now be turned off and the clickwheel will only repeat clicks as suggested by DancemasterGlenn for 1).

As for 2) - I quite like the word Keyclick, I don't think Click "looks" right. My personal opinion.

I like this patch, but there's something really annoying me namely when at the end or beginning of a list it beeps very loud with a high pitch. It's getting me a headache. Also when piezo keyclick is turned off, the high beep still remains to beep. I'd like to see an option to turn that beep off. I've attached the current version of this patch including Dutch Translations (I hope this is correct, this is the first time I'm working with patches).

Oops attached wrong, this version should be fine.

Haha, that was an amazingly fast response to turn off something that was just added to the latest patch... having tested it, I would agree that a final implementation would definitely need a less high-pitched sound than that current beep. Also, the way it was implemented was more like jdgordon's idea of having the beep upon reaching the bottom or top of a list, rather than during the action of moving from bottom straight to the the top, or vice versa. I would at this point agree with you (Craig) that the whole deal with the beep is probably best left for another task, which I'll probably open if/when the speaker patch does get committed. Thank you for working with it for my sake, though.

Back to the patch, the change causing a button hold to not click like crazy is REALLY GREAT. Is it doable to make the device stop clicking when you've scrolled to the bottom of a list, as well? That will probably be more useful than the beep idea anyway. If the device isn't clicking anymore, I would know I've reached the top or bottom of the list anyway, and could go from there (the same idea could also be applied to changing volume, so one knows they can't turn the volume up/down anymore?).

Finally, your opinion on liking 'keyclick' over 'click' is totally justifiable, and it's definitely not something I feel the need to defend any further. 'Keyclick' sounds fine to me.

Lol, didn't saw the beep was just implemented...
Anyways, yes I agree with Glenn it's really great it doesn't click like earlier when holding a button.
I think it isn't too difficult to let it stop clicking when at the bottom of a list or menu. Later this day I'm gonna try to remove that beep by myself (I have some basic C skills, still learning more and more).

Well, Craig knows exactly what code he used to put the beep in, and assuming he wasn't really all for pursuing it in this particular patch (which seemed implied, especially since it was only implemented in the speaker click and not the headphone click), I'm sure it'll be really easy for him to remove the beep himself. I appreciate your enthusiasm a lot, though... I wish I knew anything about coding in C so that I could help out too.

It's really great to see people this interested in getting this up and running so well! I'm simply overjoyed.

Hehe, yeah the beep staying on when the keyclick option is off is my fault - I forgot to include the option itself! To remove the beep code - its in "\apps\gui\list.c", remove everything that's "#ifdef HAVE_HARDWARE_CLICK". You can also make it beep when it reaches the bottom (not when tries to wrap) in the same function - but I think its definitely for another task.

To make it stop clicking when at bottom of menu seems a bit more tricky, the keyclick is produced on response to the buttons in "\apps\action.c" without knowing where it is in the GUI. I think some information/opinion from jdgordon on this is needed, as (again) its going from more than an iPod thing and could be an option to change behaviour of software keyclick too.

Here the patch with no beep and the dutch translation for people to use.

This patch seems to be fine (no beep in it). When at the bottom or beginning in a menu / list it just clicks instead of beeping.

Offtopic: Is it normal that crosscompiling in Cygwin takes almost an half our? I'm running on a Celeron M 1,5 Ghz.
I also noticed after crosscompiling my memory usage is increased from approx. 300 mb to almost 1000 mb, and it just stays at that height. I have to reboot to get it back to normal. I'm thinking to install Debian or so for Rockbox compiling on my laptop.

More offtopic: WOW, installed linux/debian on my machine and the needed crosscompilers, it compiles full rockbox in just 3-5 minutes!

I made a little comparison between the OF piezo and the rockbox piezo. The clicks after the first beep are from the OF, after the second beep are from Rockbox. As you can hear the rockbox clicks are a little bit higher, but it doesn't much differ from OF.

Here it is:

If at all possible, I'd definitely like to see the Rockbox click match the OF click more closely. Finally hearing the two by side, I would definitely make the argument that the OF click is a much "nicer" sound, and definitely more subdued. It's definitely a noise that will not attract quite as much attention when navigating in public, if you need a real world reason to make the attempt. How is the current Rockbox pitch determined, and is it something that can be adjusted to mimic the OF in this way?

Also, is there any code in the list source that returns a list length? That would possibly be helpful in stopping clicks at the end of the lines.

list->nb_items

Try this one - the keyclick is made with the action handling code if not in a list context, otherwise its made by the list handling code - so hopefully no clicks at top or bottom of list.

I've also reduced the pitch of the clicks to try and make it match better. I've got my friends 1GB iPod nano, and his OF click sounds quite a bit different to my 4GB nano OF click - both are same firmware versions, maybe damage to one of the piezo or just natural variation.

This patch is great! The way you coded it stops the clicks perfectly. I was worried an initial implementation wouldn't remember to start clicking again after reaching the beginning/end and then looping, but you totally took care of it. Good job! The click also sounds better... Though the OF has this very distinct sound that I can't put my finger on. In Stijn's test, it almost sounds like someone hitting something very tiny and hollow. Anyway, no biggie if no one wants to work much further on it. I would say that the click could actually be a little quieter? But that's pure opinion, and is neither here nor there. The sound is a massive improvement, either way.

Some observations/other thoughts:

1) Some testing revealed that keyclick now repeats no matter what. Personally, my question is: who needs this option? To me, turning off repeat was only really important to me when the thing was still beeping when used too quickly, and I never really wanted the click just to know I've made a general action. If people think this is still useful, it will need to be fixed, but I'd appreciate it if someone would tell me its purpose. In addition, now that it's working very nicely, would anyone object to 'keyclick repeats' defaulting to 'on' (this is not asking for the keyclick to be defaulted to on, just the repeat function)? Once again, I think this was defaulted to 'off' because of the beeping. Just thought I'd throw that out there.

Actually, I just found out what turning on 'keyclick repeats' does: it seems to give the exact same behavior, but it brings back the crazy clicking when holding down a button. So right now, setting this feature to 'off' is actually giving me the behavior I was looking for. Weird. So actually, if you want to get technical, the 'off' feature is correct: going through a list is not holding down a button, so that's not a click repeat. Same with changing volume. So actually... I guess that as-is, I'm not seeing any need for the 'on' function. Interesting. If this is what everyone else wants, do we need this option anymore?

2) Some more testing (actually plugging in headphones) revealed that the headphone click is now tied to the speaker click. When I turn on the speaker click, it enables the headphone click even when the headphone click is still off. This isn't as big a deal at the moment, since the click noise is much easier to deal with in its lower pitch, but should still probably be cleaned up. Also, turning on headphone click and disabling speaker click gives me no clicks at all. As opposed to the repeating options, I do think that it is a good idea to be able to toggle the headphone and speaker clicks independently. I don't necessarily understand needing to adjust the volume of the headphone click, but that should probably be left up to people who use it.

3) As I've said before, I still think that it would be very simple to have this in the 'Keyclick' menu:
-headphone keyklick
-speaker keyclick
-both
If the volume of the headphone click was sufficient for everyone (and the repeat thing was deemed to be properly implemented), then we've knocked out a whole set of further settings to navigate through, in favor of three simple options (basically, "where do you want your clicks coming from?"). I'd like to hear people's thoughts on this, as well.

4) Finally, having seen that we can use action handling code to make the clicks work in specific ways in lists, can the same thing be done with the volume handling? For example, is there a piece of code that can retrieve the current volume (I'm assuming there is, so that the gui can be updated), and if so, can that be used like the list code was used to stop clicks at the minimum and maximum volume limits?

To take this a step further, in an attempt to try and understand how the OF slows its clicks when changing volume... perhaps a click could only be given every five decibels, or something like that? I don't know if the code could be made to work this way, but it's a thought.

Once again, sorry to unload a ton of information all at once. I just don't want to forget anything, and I'm all giddy about all the work that's been done lately. Keep up the amazing work.

Sorry, in number three I of course forgot an 'off' option. So four options. It would still be much simpler, though, if people were into it.

I can't reproduce your problems in 2) - have you cleaned your rockbox code and started with a fresh rockbox install? There is an active bug with the keyclick on PP-based targets FS#8836, the speaker click disappears and works mainly when music is playing.

I can look at reducing the frequency of clicks whilst using clickwheel in CONTEXT_WPS to give the desired clicks for volume changing. At the moment, I don't know about stopping it clicking on min and max volume though.

I believe the version I compiled this morning was 17806. An absolutely fresh rockbox (the way I learned to patch, I'm not sure if there's an easier way, but it sounds like an easier way would be wrong). I'm using an ipod video 5.5 80 gig, though I'm under the impression that shouldn't make a difference. I can try compiling again if you want, but I don't think I'll get different results. Shisken, JDGordon, did you experience any of the problems in 2?

Also, a penny for your thoughts regarding the other stuff I blabbered on about?

As for min and max volume clicks, something like "if rockbox.volume() > 6 then keyclick==null" (haha, yay for fake code). I think 6 is the max, it is on my ipod. If there is currently a way to retrieve decibel values, I feel like it should owrk similarly to however you got it to stop clicking in lists.

I think there are several issues here and they're getting muddled - here is how I understand it, anyone please correct and help make progress :-)

The 'Headphone click', as my patch now calls it, is Rockbox default software keyclick - which on iPods doesn't work correctly, and on my iPod nano only really works when I have music playing or before I press play after a restart - related to bug FS#8836 .

On the iPod, the clickwheel motion of scrolling until you stop behaves as a single keypress, ie. holding down the button - so default behaviour is no repeated clicking when keyclick repeat off - the patch overrides the "Keyclick repeats" option for the clickwheel only (coming from the speaker - it doesn't alter the behaviour of the software click). The "Keyclick repeats" is there for other keys and other players. The crazy clicking on button press is the keyclick repeat working correctly. Perhaps a new task (or this one??) should be started to change the keyclick for the ipod in general.

The volume options for the software click come from the Rockbox normal menus - when the software click does work, they do make a difference and I can see people having different preferences.

Its the headphone click tied to the speaker click that I'm interested in - I don't know if there is there a way to make the piezo drive to the headphones?? The software and hardware options are different - I've double checked, but I did change the parameters to the piezo to try and improve the click. On my nano it behaves as it should do, I've made some more small changes to the patch in preparation to try looking at the volume magic later.

I believe I understand what you're saying... that essentially, this task should be kept more specifically to the piezo driver work, and that work on the headphone click is probably more appropriately kept to a separate task. In that case, it would make sense to work out the remaining quirks in the speaker click before subsequently tackling a headphone click rework in a separate task. And in the meantime, until a point at which they're essentially the same and possibly a particular headphone volume is decided upon, a point could possibly be made to streamline the options. In the meantime, work must continue on this task.

Hopefully that's what you meant.

I would make the point that whether they use the same code or not, the two clicks should probably eventually end up doing the same thing (or at least have the same sound). Like I said a while ago, it would be weird to have them both on and doing different things. We'll see where it goes when we come to it. I'll admit I'm constantly forgetting that this needs to work in some other targets besides ipods, so I'll need to be more conscious of that when it comes to brainstorming in the future.

I'll say one last thing in this task in regards to the headphone click: it's nowhere near as sophisticated as you've gotten the speaker click. I'm probably never going to use it, but I hope when this task is done you'll think about cleaning up the headphone click for all ports to enjoy.

Okay, I'm definitely getting headphone clicking with the speaker click on. I'm not crazy!!! It even sounds like the speaker click... the headphone click doesn't sound anything like the speaker click anymore. It's not just bleedthrough, because my headphones are noise cancelling and if I hold the ipod way out of reach with my fan on, I can't hear the speaker click with the headphones off. And I can hear it with them on. You're really not hearing this? It's only audible with no music playing... When music starts, the louder the music, the more the click fades into the background. Which I know sounds like I'm hearing bleedthrough that's being drowned out, but I'm serious. This has to be coming from the headphones.

To reproduce:
1) Apply patch
2) Start Rockbox
3) Enable speaker click
4) Cover your ipod with something, I guess. or drown it out some other way. You've got to be able to hear this. Just scroll up and down a list.
5) Play a music file. Scroll the volume up and down. When it gets low, the click can be clearly heard. As you up the volume, the song fades in, but the click fades out (or maybe stays the same and is drowned out, I can't tell).

Please, someone confirm this. If it's really not just me, then we have a headphone click already for some reason... it's just not audible when the volume is up for some reason. Tested on a fresh source.

Thats pretty much what I was getting at, and yeah I think the software click should end up doing the same thing, but its really up to jdgordon if this is getting worthy of committing, and integrating the keyclick mods.

All I can suggest is to try this patch - I've put the click back to how it was but still at a lower pitch. Remember that we do have different hardware, maybe I've stumbled on the hardware headphone click for the ipod video (like the OF on headphone setting)??? - but for my nano I get nothing. Also, the volume click in the WPS is now every 6 dB.

All right. Two things.

First of all, this patch still gives me the magic headphone click on my firmware. If you're trying to reproduce, use the steps in my last post, but for 5: "Play a music file. scroll the volume up and down, THEN GO BACK INTO A MENU AND SCROLL UP AND DOWN A LIST AGAIN (before you could test it by just continuing to turn the volume down, but now that it doesn't click after reaching the bottom, this isn't the easiest way to test). When volume is low, the click can be heard. When the volume is up, it sort of fades away." Apparently this doesn't work on the nano, so if anyone at all has a non-nano ipod, please test this for me and tell me if I'm nuts or not.

Second of all (and most importantly), this patch pretty much does everything that the OF click does, at this point. The click stops at the end/beginning of a line, and stops at the minimum and maximum volumes. Clicking every 6 dB sounds pretty good (I might go even higher, maybe seven or eight? Hard to tell without hearing it), and the fact that you accomplished all of this at all is just great. The click is sounding good, too (especially comparing it to the original rockbox click). Is there anything else that should be set as a goal for this specific type of keyclick? JDGordon, what are your thoughts on what this patch still needs before it would be considered for committing?

The only reason I picked 6 dB is that 4 clicks with my earphones is the difference between loud and quiet - I made this patch click every 12 dB, see what you think.

This patch also includes the test plugin from dionoea near the start of this task. Glenn, if you could first of all test the previous patch that didn't give the magic headphone clicks, to make sure it still doesn't, then apply this patch and start up the piezotest plug-in under applications. The piezo sounds if enabled is greater than 0x80. I changed the waveform from 0x80 to 0xF0 before when you first said the problem, and reduced the freq from 0x32 to 0x5B (lower freq is an increase in the value). I guess its a case of see if the buzzing comes through your headphones.

Well, to be perfectly honest, I tested most of the initial patches once I started actually using them with no headphones on. I'll try a couple of the patches out, but it'll be slow going. Am I patching wrong? I always download a new source, run ./rockboxdev.sh, patch, configure and make. Every time I want to try a new patch, I'm deleting all of that and starting completely fresh. That's right, right? Or can I un-apply a patch on a source so I can re-use it again slightly more quickly?

Either way, I'll go back and test right after we removed the beep when looping. I'm still not quite sure what you're asking me to do with the plugin, but hopefully I'll know when I actually see it. And if not I'll ask.

Myquestions to everyone else still stand! We need more than just my feedback...

Glenn, A) rockboxdev.sh install the dev environment and you basically never have to re-run it. B) you _can_ unapply a patch by simply doing the exact same command line but adding -R

Dan, thank you so much! That speeds this up greatly.

Craig, I was rolling backwards through your patches, when I realized I should start from your initial commits and move forward. Your initial sync doesn't compile for me, and the one after that (when you moved all the settings to where they are now)... gives me magic headphone click. So I have no idea. We really need someone else to confirm this. It's definitely there... I'm literally holding my ipod under a pillow, and I can only hear it clicking with my headphones on. I have no explanation as to why it only seems to be happening on my video, though.

And don't get me started on how weird it sounds when I enable both (this is on more recent patches)... without repeat on, I hear the different-pitch headphone click with every first click, followed by little subsequent (different pitch) clicks (the magic ones). With repeat ON, it gets kinda buggy. The headphone click beeps sometimes, but the magic clicks don't. Curiouser and curiouser.

What would happen if we commented out all the headphone click code? Could you make a patch that does that? I'm wondering if disabling all headphone click junk may disable something that's been magically tied to the speaker click on my build. If it does, then I would know that something in that code is the culprit.

In the meantime, I'm going to test out the newest patch, with the plugin. I'll let you know my results.

Yes, the buzzing does come through my headphones with this plugin.

Having spent the day with the patch, I'd say clicking every 12 is probably higher than it needs to be (especially with what you said about it only taking 4 clicks for it to get very loud for you). I'm always clicking all the way up for when I'm in the car, since I like to get it up to 0 dB and I'm runnning it through the headphones, so it's good to hear a few more clicks. I'd go with every 8 or 10, probably. 10 would be a nice round number (hah), but 8 gives people who don't turn the player all the way up all the time (you and probably most other people) more clicks. And clicks are good. So 8 would probably be good.

You know what? I just realized the OF doesn't even mess with the clicking in volume. I mean, it stops at the min/max values, but it just kinda chugs away when changing volume. Maybe it's clicking less than in a list, but it's a bit hard to tell... the volume ramps up a bit faster in the OF than in rockbox. I'm a bit embarassed, I'm not sure why it sounded so different before. So I guess whatever you want to do is up to you... personally I like it not clicking so much (I'd rather be hearing the song than a bunch of clicks), but if you want to change it back to the way it was before we started messing with it I'd understand.

Testing against r17865, the newest patch now fails (something related to the plugin), but the patch before it works fine.

I've changed the volume click to every 8 dB so only have to count 3 clicks now. From what you say about the piezo coming through your headphones with the plug-in suggests something else - the test plug-in drives the piezo (as known at the moment) directly, have you played around with it to see if any command stops the headphone buzz? Without hardware to play with I don't think I can help anymore, could do with help from an PP/iPod guru. Another thought I've had, is what happens when you load the ipod firmware, change its clicker setting and then reboot to rockbox?

The only way I could get it to stop in the headphones was to play in a low volume (like 1) and a lower frequency (higher pitch, if I remember correctly). Not really optimal for a click noise, and it probably faded out because the headphones simply couldn't handle the tone anymore. I can give you more concrete values if it would help.

Really, I would say this is an issue similar to the beep experienced in the software click... it's a weird little quirk, but it shouldn't stop the task from being committed. Maybe adding this to the main build will inspire someone with more knowledge to work out what it is that's happening (which is what happened when software click was sent out into the wild, I think). I'd be perfectly fine seeing this committed, even with my phantom clicks. That's just my opinion, though.

So, two final things before I turn in for the evening... turning off the click in OF and rebooting didn't change anything (unfortunately... it was a good idea, though). Secondly, I'm wondering if you might be able to make me a special patch... one that lets me choose the frequency of the volume click. I keep testing these builds and thinking I'll know the right number of dB to click to, but I really can't figure this one out. if it's possible to make it an option for a patch (one I would NOT want to see in a final commit, just one so it can be tweaked to find the right value for this one), that would be awesome. Maybe it's just that this volume thing is the last real issue bugging me about this task, but I'd like to get this one just right before I'm 100% behind it being committed.

If a patch for that can't be easily made, I would say stick with your original click every 6 dB. maybe it's just me having finally realized that the OF doesn't do this, but the more I'm using these high number intervals, the less they sit right with me. Might go even lower than 6 at this point. Sorry for being so wishy-washy.

I figured I might as well extend the question again, as to whether anyone has goals this patch must meet before it is truly considered for a commit. For me, the biggest remaining issue is to get a volume click we can all agreee on (I'm once again okay with lower values, 6 was probably fine). I think some of the problem for me is that sometimes it seems like the volume change lags, and volume doesn't go up/down as fast as usual. I think I sometimes see these as the norm, and the difference between these spaced out clicks and the normal rapid clicks are large. This is probably a gui boost issue. It's all very complicated, and frankly, if we just set it back to click every 6, or or maybe even lower, I'd be fine with it.

I think that looking back at JDGordon's initial thoughts on what the patch needed, the only thing still really missing is his number three: "it should be handled in the same way and places as the software click." Because the piezo click has now (in my opinion) surpassed the headphone click in functionality, it is doing some things differently than the piezo click does. Are these things this patch should work to reconcile? If so, it would make sense to work on it and get that obstacle out of the way. If it is something that should not be addressed by this patch, like I said before, is there anything else that would block this from being committed?

Any chance this could be submitted for Rockbox V3?

Rockbox is currently in feature freeze before the release of 3.0 - this means no new features (including this patch) will be added, only bug fixes.

I'll start the post by apologizing, because I feel like my extensive comments and wishy-washy behavior have contributed to this patch not seeing any activity in some time. I'm not surprised in the slightest that this isn't making it into 3.0, because as we've gone over, it's not quite done yet (done currently meaning "meets jdgordon's original 3 suggested criteria"). There's also the additional question of how many people will use this once implemented, but I think that once it is offered that number can only grow. It's definitely a nifty feature.

So I guess what I'm wondering, once again, is if any higher-up people would like to revise the "to do" for this patch, to get it up to the level where it can be considered for submission (post-3.0). Anything besides click frequency in changing volume (which should definitely go back down to every 4 db, or something around that, after having lived with the higher one for this time), and possibly getting the features of this patch matching the current headphone click functionality (in this task or a new one)?

Either way, Craig, don't give up on the patch. You've done really solid work bringing this patch to the level it's at, and I know I appreciate it very much. I hope you'll be back at some point to show this task some more love. Thanks again.

Yeah please don't give up on this patch.. I would definitely use this but for 1 thing, on my Ipod Nano 1st gen 2 gb in rockbox it breaks video playback... It says "Cannot create buffer thread". I do not know if this affects any1 else since noone else mentioned it, but this message is persistent.. I dont really know wat this means? If this could be fixed it would be an awesome patch.. Thanxs in advance for making a working piezo patch.. :)

Confirm the "Cannot create buffering thread!" error at video playback on iPod Color :-(

Yeah, I also got the buffering thread error on my Nano 2gb while using this patch.