Rockbox

Tasklist

FS#7738 - Scrollwheel acceleration for iPOD (ported from Sansa)

Attached to Project: Rockbox
Opened by Andree Buschmann (Buschel) - Sunday, 09 September 2007, 14:58 GMT
Last edited by Bryan Childs (GodEater) - Monday, 19 November 2007, 11:05 GMT
Task Type Patches
Category Applications
Status Closed
Assigned To No-one
Operating System iPod 5G
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 is a merge/adaption of the Sansa scrollwheel acceleration. I've also added a small configuration -- nevertheless the configuration does not work well yet (iPOD acceleration only works with 10 and 20).

Please feel free to start the discussion :o)
This task depends upon

Closed by  Bryan Childs (GodEater)
Monday, 19 November 2007, 11:05 GMT
Reason for closing:  Accepted
Comment by Andree Buschmann (Buschel) - Sunday, 09 September 2007, 17:35 GMT
After some discussion and review here is the correction of the internal calculation of clicks/s. Now the iPOD clickwheeldriver seems to deliver more reliable data for the rotation speed.

Changes:
- correct calculation of rotation speed
- change default configuration for acceleration start to 100 clicks/s
- change wording for the setting
- add german translation

How to:
The setting "Wheelspeed for Acceleration" describes above whoch rotation speed the acceleration starts, the default is set to 100 clicks/s -- which is about 1 full rotation/s. To get more agressive acceleration this value has to be lowered, to get less agressive acceleration it has to be enlarged.

Note:
Only added for iPOD Video's yet. To enable and test this functionality on other iPOD's just merge the changes in config-ipodvideo.h to the appropriate config-file.
Comment by Nils Wallménius (nls) - Sunday, 09 September 2007, 19:01 GMT
The added string for the configuration option should only be included for scrollwheel targets so it should read like this instead:

<phrase>
id: LANG_WHEEL_ACCEL_START
desc: wheel acceleration start
user:
<source>
*: none
scrollwheel: "Wheelspeed for Acceleration"
</source>
<dest>
*: none
scrollwheel: "Wheelspeed for Acceleration"
</dest>
<voice>
*: none
scrollwheel: "Wheelspeed for Acceleration"
</voice>
</phrase>

New lang strings should also be added tho the end of the english.lang file to maintain compatibility also if this gets comitted it should be noted in the commit message that this breaks lng/voice files for whichever ipods now define HAVE_SCROLLWHEEL
Comment by Jonathan Gordon (jdgordon) - Monday, 10 September 2007, 08:00 GMT
just tested on the nano, works great.
Comment by Michael Sevakis (MikeS) - Monday, 10 September 2007, 11:44 GMT
Ok, you wanted to discuss, so here goes. I was just testing on e200 and the factor range is way to wide and may overflow the alloted bits. The acceleration factor really should be a fine-tuning aspect and not a principle setting that requires a long long parameter. The "lowest" setting is just incredibly oversped.

How does the setting relate to wheel speed if it doesn't actually use the wheel speed data anyway? Its name implies it does but it really turns y = s*c*v^2 into y = s*c^2*v^2 with c being basically a scaling constant that is chosen. I think having the driver cheat on velocity reporting would make more sense than to change the amplification. Maybe I'll look into fixed-point fractional exponent routines which would be ideal to vary the curve from perfectly straight to very ramped which would actually vary the amount of acceleration. Sometimes no floating-point just stinks so v^2 was a minor kludge. :)

I'd suggest keeping the fixed-point notation instead since that help track bit requirements and expressions such as 0xffffff*7/5 allow the possibility of settings with non-integer factors (0xffffff*x/y). It also is clearer that it means the fraction "7/5" or "1.4" in 0.24 fixed-point. If you hadn't thought about it (from config), 0xffffff*16.000000894069724921567733381255... = 0xfffffff - implying you have four bits of headroom - dont' exceed it! :)

Some more info on the e200 wheel:
The acceleration/skip ok bit on the sansa driver is controlled by the driver, not the list code or button code. Acceleration is allowed to start at 50 clicks/S and turns off again when speed drops below 16.67 clicks/S. The hysteresis here was really important to hiding the mode change from preception and also so that it doesn't jump alot between modes while moving from minor velocity changes. The velocity filtering (simple moving-average IIR filter) also helps this so that short jumps in movement speed don't trigger fast mode and short drops don't cancel it prematurely if velocity is near one of th e levels. It's rather impossible to not have jitter in your control input.

The slow mode is a mode that runs the wheel at half speed for finer control so two clicks are required to get another event. A direction reversal - even after no or one click requires a full two clicks to happen in the same direction again to get an event. This makes list selection of a specific item easier and less prone to a missed selection because your fingner happens to move a little bit while hovering on something. It's also easy to bump the wheel a bit while pushing a button.

Fast mode simply sends an event every click and is cancelled if the wheel changes direction. To get back, one must move fast enough, long enough in one direction.

The flag was designed to have the "ok" given by code that has the most information about the wheel states. The configuation really should be set in the wheel driver via a function which allows greater flexibility and gives the driver access to the setting. If this were configurable in that way for e200 for instance, the WHEEL_FAST_ON_INTERVAL and WHEEL_FAST_OFF_INTERVAL would be variables chosen higher up and this wouldn't disrupt the change points relative to other things. The problem was I never actually found other values that seemed to hide the transitions as well. A variable factor could even be calculated within the driver and used elsewhere allowing different individual formulations for each player's hardware.

I think the skip count bit is obvious, but for completeness: It's 7 bits allowing a record of up to 127 skips. Direction reversal/slow mode cancels this and resets it to 1.
Comment by Andree Buschmann (Buschel) - Monday, 10 September 2007, 12:33 GMT
So, if the range for allowed configuration is too wide -> this can easily be corrected (from a ergonomics and overflow point of view). We may even define the ranges different for different targets via the existing config-...h's.

Just to explain my thoughts for the calculation:

- wheel-driver does: v = 0xffffff * (clicks / usec)
- button/list does : accel = factor * v * v / normalization and
y = (accel>1) ? step*accel : step (whereas step is the non-accelerated value)

So, one can interprete accel>1 as active acceleration and accel<=1 as non-active acceleration. For this reason you may calculate a factor which will result in active acceleration if clicks/usec reaches a given size.

accel = factor * 0xffffff * 0xffffff * clicks * clicks / (10^6 * 10^6 * sec * sec) / normalization = factor * clicks/sec * clicks/sec * 10^-12 (as 0xffffff^2 / normalization = 1)

We search for accel >= 1:

1 <= factor * clicks/sec * clicks/sec * 10^-12
factor >= 10^12 / (clicks/sec * clicks/sec)

If you now use a configuration for the limit in clicks/sec from which on the acceleration may start (accel > 1), you can therefor calculate the factor out of it as done in my patch. The factor will lead to accel < 1 and therefor to non-accelerated scrolling as long as the clicks/sec do not exceed the configured limit.

Imho the configuration is an elegant way to adapt the acceleration to
a) personal needs (more/less agressive)
b) other clickwheels with other number of clicks per full rotation

My opinion to your other points:
- IIR-filtering -> absolutely needed and included in my patch
- switch off acceleration on directional change -> absolutely needed and included in my patch (via velocity = 0 in driver)
- switch off acceleration after timeout -> -> that's absolutely needed and included in my patch (via velocity = 0 in driver)
- hysteresis -> I am really not sure about this as I did not experience any ergonmic flaws yet. Imho the hysteresis makes the code less straight forward -- but that's only personal smell.

Regarding the "fast mode"-flag: In contrast to my solution (which will always have smooth transition from non-accelerated to accelerated scrolling), your solution may have jumpy transition in theory. As far as I saw you tweaked the driver to avoid this. Imho we could just drop this flag and only use velocity. You may set velocity = 0 instead of setting the "fast mode"-flag to off.
Comment by Andree Buschmann (Buschel) - Monday, 10 September 2007, 12:48 GMT
Just another thin which came up my mind when having a cigarette...

You said you enable fast mode at >50 click/s and disable it at >17 clicks/s. The factor you are using will lead to v > 1 for >90 clicks/s. So, if I understand correctly you enable fast mode without any effect on the scrolling between 50...90 clicks/s as speed-up is v<1 (in this case the standard steps are used).
Comment by Andree Buschmann (Buschel) - Tuesday, 11 September 2007, 10:07 GMT
Another update with the following changes:

- changed the .lang-files like suggested by jdgordon
- enabled scroll acceleration for iPOD Nano as jdgordon confirmed proper functionality
- reduced configuration range to 50...200 clicks/sec
- changed calculation and hand-over of velocity as well as calculation of acceleration factor (explanation follows)
-> no need to use <unsigned long long> anymore
-> velocity now equals the physical clicks/sec
-> also changed for sansa-driver, please review and test :o)

In driver:

velocity = 1000000 * clicks / usec; -> clicks/sec

In list/button:

v = velocity;
v = (v<0x3ff) ? v : 0x3ff; -> limit v to 10 bit to prevent overflow
factor = 2^20 / (clicks*clicks); -> clicks = configured value for starting acceleration
accel = (factor * v*v)>>20;

So, in total the results are the same as with my former patch-version. But now there is no need to use <unsigned long long> anymore.

Comment by Jonathan Gordon (jdgordon) - Tuesday, 11 September 2007, 10:47 GMT
it was nls that suggested the lang fix :)
and you should probbaly enable this for all ipods which use button-clickwheel, seen they should all work.

When I first tested it i didnt realise you changed the sansa driver also, Ill test it later, if i remember :)
Comment by Andree Buschmann (Buschel) - Tuesday, 11 September 2007, 10:59 GMT
About the fix-suggest you are right :) mea culpa :P

Are there any iPODs which do not use buttonwheelclick?

The sansa-change was commited with the last version (v3) of the patch. Just to simplify some calculations...
Comment by Michael Sevakis (MikeS) - Tuesday, 11 September 2007, 12:30 GMT
Andree, things popping into your head when you're having a smoke and not looking at the code? Hehe...practically every idea occurs to me at that time.

This part is important:
The hint flag should be there but for iPod just make a synthetic one and don't have a slow mode. The thing you're missing here is the fact that even when moving slowly, regardless of whether v would be greater than 1, is that a queue_post could be bypassed because a button message is already in the queue so therefore it also hints to ignore the skip count. The wheel does not need to have a driver fast mode as sansa does to have this flag give valuable info. In other words, it's not only for acceleration purposes.

It's really imporatant on e200 to have the slow/fast behavior even if iPod is comfortable without it. It's only one bit and besides I probably worked out the numbers in detail and just don't remember all the steps I went though to match up the numbers. Once I'm done, most of what I thought about gets deleted so I forgot about the other point regarding the flag. :P Think about it, selecting precisely may not be imporant at low speed in a game, but for the lists it's paramount. How difficult is it to just provide the flag at the correct velocity? Not at all. I wanted an interface with features that can be relied on to exist uniformly.

The fact button_apply_acceration doesn't ignore the skip count with the hint flag cleared seems to be a stupid, idiotic bug on my part and explains why it sometimes jumps when moving slowly at low CPU frequency. Oops. Feel free to fix that one too.

To wit, "aggressiveness" is really a function of the power v is raised to and not the scaling constant. The constant just scales linearly even if the constant is squared but the power of v has more "power" as it were since square growth always overtakes linear.

Simplifying the interface overall is a good thing though so I won't argue there I had to quickly invent something because time-based acceleration as used on joystick devices (which is _great_ there) was just wrong for a wheel based device.

Comment by Michael Sevakis (MikeS) - Tuesday, 11 September 2007, 12:33 GMT
Never mind that...blah I cheated and kept delta at 1 in slow mode. I really made it all backwards from what I intended. Argh. :P Lol. Good thing your making me look at the code again.
Comment by Michael Sevakis (MikeS) - Tuesday, 11 September 2007, 13:58 GMT
Ok, I'm going to straighten it out in SVN which I guess will need a minor resync. Quite frankly I'd rather provide wheel velocity in some fixed-point abstracted form like angular velocity simply because the notion of clicks/S is very device specific and makes the same setting behave quite differently depending on target. Just an idea.
Comment by Andree Buschmann (Buschel) - Tuesday, 11 September 2007, 14:34 GMT
Ok, here's the next one with following changes:

- corrected german umlaut (displaying "ü") in deutsch.lang
- enabled the acceleration for iPOD 4th gen, iPOD Color, iPOD Mini 2nd gen, iPOD Nano, iPOD Video
- inserted some #ifdef's in the iPOD-wheeldriver to have either acceleration or old behaviour (iPOD 1st-3rd gen, iPOD Mini 1st gen) -- depends on config.h's for the iPOD-version

Mike: If I understood your code you should simply set click/s to about 50. This way the acceleration will be around =1 when switching from normal to fast mode -- the scrolling speed will of course increase then :o)
The other solution would be to #<define WHEEL_FAST_ON_INTERVAL 10000>. This way the fast mode flag should be set in time with acceleration growing larger than 1.

You think of using something like rotation/sec? This is quite an easy understandable representation.
Comment by Michael Sevakis (MikeS) - Tuesday, 11 September 2007, 14:43 GMT
Ok, you wanted to discuss things and discuss like hell I will because the though process of the logic is coming back. Another reason for forcing things is to prevent delta "leakage" from one context to another while saving a global state which risks especially (not even in a minor way) a long delta being leaked into an input context where it is no longer valid -- say moving the wheel quickly and racking up a large skip count and stopping suddenly. Now you have this data stored but perhaps a button was pushed which left a menu or a plugin. Next wheel movement can contain that stored > 1 delta which would not be right in the new context. Any proposal on preventing this potentially annoying situation while always being truthful about states? There's plenty of devils in this.
Comment by Michael Sevakis (MikeS) - Tuesday, 11 September 2007, 16:53 GMT
The transistional speeds were very carefully chosen and any change just wrecks the feel - even millisecond ones - I can't explain why. It just does. Making accel kick-in a tiny bit earlier might actually be an improvement since 90clicks/sec is well over one rotation/second on a wheel with on the order of 2^6 clicks per rotation. I guess the setting will allow that to happen.
Comment by Bryan Childs (GodEater) - Wednesday, 12 September 2007, 14:15 GMT
I'm trying to test this patch myself, but the V4 gives this error :

patch: **** malformed patch at line 236: Index: firmware/export/config-ipodcolor.h

Which is a bit of a pain.
Comment by Andree Buschmann (Buschel) - Wednesday, 12 September 2007, 14:32 GMT
patch was broken -- my fault...
Comment by Michael Sevakis (MikeS) - Wednesday, 12 September 2007, 15:52 GMT
1) I'm not going to change anything for now because being completely truthful would introduce that context "leakage" I mentioned earlier. My real issue is the setting name and the setting referring to the value of a particular parameter. Really, a more "touchy-feely" sort of name and values would be better imo. Imprecision is good here since a particular setting is rather player specific in terms of feel.

2) Why'd you make the default 100 for e200? Even the original 90 is a bit on the high side there since it has a lower number of clicks/turn than iPods. I guess that makes my point about the setting having precise values. There's also not much point having settings lower than the fast mode speed if smooth transitions are to be important. The transition itself is an initial acceleration.

Anyone may disagree on 1) and I have no problem with committing this. As far as 2) goes, the range and defaults should be selected more carefully on Sansa. If that's addressed. Go for it.
Comment by Andree Buschmann (Buschel) - Wednesday, 12 September 2007, 16:21 GMT
1)
a) As far as I can see the leakage won't practically occur as any context change will let the timeout (500ms) reset the acceleration.
b) Your suggestion?

2)
I recalculated your former define for the factor to the "new" factor. Your define equaled around 95 clicks/s (hopefully no error in my calculation).
Does the default not show the same behaviour?
Comment by Michael Sevakis (MikeS) - Thursday, 13 September 2007, 01:02 GMT
My OP.:
1)
a) I guess not since looking deeper reveals most of the scrollwheel acceleration interface isn't implemented which is fine since I don't own a relevant target. Each driver will be markedly different it what information it provides so I guess code outside the core being able to rely on a consistent interface is out. Be mindful accumulated_wheel_delta will be squared since you scale v with it instead of just providing an honest, straight-up v measurement and the skip count (safety-based fibbing aside); the e200 driver isn't so reliant upon acceleration because of the count. Using 1000000 instead of 0xffffff makes velocity reporting less than 1/16 as accurate as it was since we lost about 4.07 bits of fraction. The slowest detectable click velocity is now 1 click/s instead of .059 clicks/s. It was never meant to be all about what makes v > 1 for the lists only which seems to be the approach here.
b) Suggestion implied already.

2)
I'm really biased toward lowering the level a bit more than SVN has it but I was a bit conservative. I suppose I can tweak defaults and ranges at any point so no biggie there. The range is still too wild. I see the low end got toned down but the highest is unreachable by a mere mortal (around 3.5 revs/s when making 2 is quite a chore!) on sansa.

OP. i no nat,
Ed
;)
Comment by Andree Buschmann (Buschel) - Thursday, 13 September 2007, 09:32 GMT
Small change:
- also reset accumulated_wheel_delta when nullifying acceleration on either direction change or timeout

1)
a) I don't think the loss of precision is relevant here as the minimum detected clicks/s is still very low (1 clicks/s) and far below any acceleration. So, imho we don't miss anything here...

2)
My old fingers are still so fast I can even use 200 clicks/s and get an acceleration ;o)
Comment by Michael Sevakis (MikeS) - Thursday, 13 September 2007, 11:22 GMT
1) I think it's quite relevant since 1 click/s isn't very slow. In fact, that's about a normal navigation speed when selecting items.
2) You have an iPod with more clicks/rev than a sansa so I'm sure it's no problem there since that only takes about 2 revs/sec.

I don't understand your aversion to fixedpoint. It's used when floating point isn't available and maximally uses the available bits and is used in rockbox code (though mostly in DSP code). I also don't understand your stubborn refusal to just implement the interface completely since it's so utterly trivial to do so and is of benefit to code elsewhere besides the lists to pass all the info along.
Comment by Michael Sevakis (MikeS) - Thursday, 13 September 2007, 11:30 GMT
One thing I should have defined as well is a clicks/turn constant for the target and then a truthful velocity could be converted to revs/sec or some other more objective measure to provide the ability for games or whatever to use it for adjusting the speed of an element to be the same for all targets.
Comment by Andree Buschmann (Buschel) - Thursday, 13 September 2007, 12:25 GMT
It's no aversion to fixpoint -- I just wanted keep the multiplications (factor*v*v) in 32bit-range...
The coming days I won't have too much time to further implement on the interface. If you'd like you may change the interfacing and post the patch here?
Comment by Andree Buschmann (Buschel) - Thursday, 13 September 2007, 18:40 GMT
Next patch-version with following changes:

- changed interface: configuration now sets "°/s" (360°/s = 1 full rotation/s)
- driver calculates velocity in degree/s
- added new a define to sansa- and iPod-driver (#define WHEELCLICKS_PER_ROTATION, = 96 for iPods, = 48 for Sansa)
- reduced timeout for reset of acceleration on iPod to 250ms (before: 500ms)
- adapted internal calculatons in button_apply_acceleration() to new interface

@Mike: Can you just test the patch and try to find the best matching configuration for Sansa?
Comment by Andree Buschmann (Buschel) - Thursday, 13 September 2007, 19:54 GMT
Oops, last patch was broken for Sansa.

Changed:
- convert to °/s in queue-post only to avoid changes in internal calculation
- changed settings range to 270...910 °/s

ToDo:
- finalize settings range and default for sansa
Comment by Michael Sevakis (MikeS) - Thursday, 13 September 2007, 20:17 GMT
V8: That's what I tested already and it worked just fine.
Comment by Michael Sevakis (MikeS) - Friday, 14 September 2007, 02:54 GMT
Recap (I know I said some stuff already :):
One goal was to be forward looking to what is likely desireable to know about when using clickwheel data not only now for the lists, but in the future for anything else and to have the data be consistently available on all targets that have such a device. Providing a rate range that extends beyond practical limits high and low for current devices ensures that a) an accurate reading is always available b) some as-yet-to-be-ported device has slop room. 12.4 fixed would give a ludicrous upper range, not sacrifice sub-HZ speed, and give a whole byte back for other possible data. I wasn't too concerned about 64-bit intermediate results since the processors most likely to be used for this do multiword arithmetic quite efficiently. Even if they don't it's not called very often so speed isn't that important here anyway. The compiler support routines are already required by other code.

As far as altering iPod code, I can't really test it myself to tune it so I'm not really comfortable with modifying it but I do know 1) whatever the sansa wheel driver provides for data, the iPod one can as well with little trouble. I can see that clearly in the implementation of the driver and can't figure a situation that it couldn't be derived - even H10 might be able to use this interface and it has no wheel (I may very well attempt it). 2) sansa's wheel is different since it's a small physical wheel with relatively few clicks/rev so demands different settings ranges and defaults.

EDIT: That was odd. I submitted this one hours ago and it was just sitting there doing nothing. When I clicked "back" in my browser, it then posted it. :\
Comment by Travis Tooke (tdtooke) - Saturday, 15 September 2007, 19:03 GMT
This is working well for me, my only suggestion would be to add a few faster settings. I generally use how easily I can browse through my database by track as a gauge for how well my acceleration is working for me(I actually do that in real use from time to time), and the fastest setting still feels a bit slow to me. Otherwise great patch, glad to see someone looking into a solution for iPod scrolling that actually has a decent chance of being committed.
Comment by Andree Buschmann (Buschel) - Sunday, 16 September 2007, 14:12 GMT
Next version with rework of iPod scrollwheel driver.

Rework of iPod scrollwheel driver:
a)
Old driver lost lots of wheelclicks as a fwd/rwd was only accepted on wheelpositions with abs(diff)>N without keeping the position of the last accepted fwd/wrd. Now the wheeldriver keeps the correct last wheelposition.
b)
Old driver was of bad responsiveness with fwd/rwd accepted on wheel position diffs>4. I reduced this to >=2 which gives 48 fwd/rwd's per full rotation. This works fine on iPod Videos, should be tested on other iPods -- like Nano -- with smaller wheel. Maybe we should use different values for different iPods?
c)
Wheel positions, accelerations and accumulations were immediately resetted, if the finger slipped from the scrollwheel. Nevertheless this happens very often and very shortly while fast rotating. Through this wheelclicks were lost as well as the acceleration did not work properly.
Now the driver will only reset the settings, if an appropriate timeout was reached after "untouching" the wheel.
d)
Corrected BUTTON_REPEAT-handling (done via timeout now, if the scroll direction does not change).

Other changes were done on the acceleration stuff:
As the acceleration on iPods was too slow to handle large lists (> several hundreds) I added a second config which lets the user set the acceleration from v^2, v^3 to v^4 (v = velocity in °/s). For iPods v^4 works fine in combination with acceleration start = 270°/s.
- added new config (acceleration)
- adapted acceleration calculation
Comment by Andree Buschmann (Buschel) - Sunday, 16 September 2007, 14:54 GMT
Sorry, last patch was broken (missing #define in config-ipodvideo.h).
Corrected with v10.
Comment by Andree Buschmann (Buschel) - Sunday, 16 September 2007, 15:41 GMT
A small debug panel which helped a lot to get some stuff figured out. Maybe also interesting to add with the patch?
Attention: Please only use with the acceleration patch itself.
Comment by Dominik Riebeling (bluebrother) - Thursday, 27 September 2007, 10:43 GMT
I found the scrollwheel debug panel to be too wide to fit on my minis screen. I adjusted the patch by shortening some labels (and fixed a typo in the menu). No other changes.

For the scrollwheel patch itself, I tried various settings and it's still too sensitive for my taste -- scrolling exactly one line becomes quite hard. Or did I miss a setting? I'd also rather put the scrollwheel setting to the system menu than the display menu. But the acceleration itself is nice, will try it for some days ;-)
Comment by Andree Buschmann (Buschel) - Thursday, 27 September 2007, 17:03 GMT
For the issue with too sensitive please see my comment above. I wasn't sure if the sensitivity is too high for smalle scrollwheels. Try to change these lines:

if (wheel_delta >= 2)
...
else if (wheel_delta <= -2)

and exchange "2" with a larger number N (3 or 4). This will only generate a single line-scroll if the wheel was moved by at least N sensors (of 96 / 360°).
Comment by Thom Johansen (preglow) - Friday, 28 September 2007, 12:46 GMT
Tested on nano now, and I love the acceleration itself, but the cursor is way too nervous around small changes, even when exchanging wheel_delta in the above comment with something bigger.
Comment by Andree Buschmann (Buschel) - Friday, 28 September 2007, 14:45 GMT
We should do the following now: Test this patch on several iPods with different values than "2" in "if (wheel_delta >= 2)" and "else if (wheel_delta <= -2)". Btw, the trunk sets this value to "5" (>=4 and <=-4). If you've found a suitable value, please post it here and name the type of iPod you've used as well as the acceleration settings.

Hopefully we will get a broader view of the needed configuration then...
Comment by Thom Johansen (preglow) - Friday, 28 September 2007, 14:51 GMT
A synced patch would also be nice.
Comment by Dominik Riebeling (bluebrother) - Friday, 28 September 2007, 15:11 GMT
I just tried a value of 5 (i.e. <= -4 / >= 4) and like that pretty much.
Player is Ipod mini, settings are 360°/s / (°/s)^4.
Comment by Andree Buschmann (Buschel) - Saturday, 29 September 2007, 14:34 GMT
Sync'ed to SVN #14902, done the following changes:

- replace "magic number" with #define's
- #define WHEEL_SENSITIVITY is used the set the constant for sensitivity (see discussion above)
- WHEEL_SENSITIVITY is set to 4 by default (a little more sensitive than trunk, but twice less sensitive than former patch)

If we encounter large differences in haptic behaviour between the different iPod models, we can move this define to the config-files of the models.
Comment by Andree Buschmann (Buschel) - Saturday, 29 September 2007, 17:56 GMT
Added in some feedback from IRC:
- use WHEEL_SENSITIVITY = 6 for iPod nano
- use WHEEL_SENSITIVITY = 4 for all other iPods
Comment by Dieter (dip) - Monday, 01 October 2007, 14:43 GMT
This patch really works great. It seems to be very smooth (muss less choppy than  FS#5594 ). In my opinion it should be comitted and replace the list scan acceleration which is still not functional on the iPod and which does only a time based acceleration (as far as I understood).
Comment by JerryLange (psycho_maniac) - Wednesday, 03 October 2007, 05:13 GMT
i dont think this will happen soon but hopefully the lag when your scrolling when playing a mp3 song is fixed. i think this is not a patch bug but a bug in rockbox on the ipod that has to do with the cpu?
Comment by Bryan Childs (GodEater) - Wednesday, 03 October 2007, 06:43 GMT
Jerry,

You're correct - the "lag" when scrolling is nothing to do with anything this patch tries to address - it's to do with CPU boosting in Rockbox.

With that in mind - let's try to keep the comments here on topic.

Comment by Andree Buschmann (Buschel) - Sunday, 07 October 2007, 11:04 GMT
Sync'ed to SVN #15011, done tiny change:

- also do reset accumulated_wheeldata when !defined(HAVE_SCROLLWHEEL)
- accumulated_wheeldata must be divided by WHEEL_SENSITIVITY before sending the button-event when !defined(HAVE_SCROLLWHEEL)
Comment by Bryan Childs (GodEater) - Wednesday, 07 November 2007, 11:32 GMT
Sync'd against 15513
   7738.diff (28.6 KiB)
Comment by Dave Chapman (linuxstb) - Thursday, 08 November 2007, 11:50 GMT
I've just tried the latest patch (7738.diff) on my ipod Color.

Firstly, there's a warning:

target/arm/ipod/button-clickwheel.c: In function 'ipod_4g_button_read':
target/arm/ipod/button-clickwheel.c:211: warning: comparison between signed and unsigned

Apart from that, it seems fine to me with the default settings.
Comment by Andree Buschmann (Buschel) - Friday, 09 November 2007, 18:53 GMT
As I do not have this warning and as there seem to be differences between the last 7738.diff to v13 I just also re'synced to SVN 15550.
I added the additional debug screen to the patch now.
Comment by Bryan Childs (GodEater) - Friday, 16 November 2007, 10:38 GMT
This patch is sync'd against 15634, and I've attempted to remove all the menu settings associated with the patch (which is pretty much all the people want gone from it before it's committed.)
Comment by Bryan Childs (GodEater) - Friday, 16 November 2007, 11:11 GMT
Reworked it a bit more to tidy up the functions now that everything is defined as constants.
Comment by Linus Nielsen Feltzing (linusnielsen) - Friday, 16 November 2007, 14:12 GMT
This looks suspicious:

switch (WHEEL_ACCELERATION)
{
default:
v = (v*v + (1<< 7))>> 8;
break;
case 2:
v = (v*v*v + (1<<11))>>12;
break;
case 3:
v = (v*v*v*v + (1<<15))>>16;
break;
}

Isn't WHEEL_ACCELERATION constant?
Comment by Linus Nielsen Feltzing (linusnielsen) - Friday, 16 November 2007, 14:28 GMT
Here is a better version, using the preprocessor instead.
Comment by Andree Buschmann (Buschel) - Saturday, 17 November 2007, 15:13 GMT
Good to see that the default settings seem to be good enough to remove the whole settings menus stuff. If we do so, we can also remove all changes in the .lang-files.
Comment by Bryan Childs (GodEater) - Sunday, 18 November 2007, 09:10 GMT
Clearly I went a bit brain dead when I started removing the settings menu stuff - I could have sworn I removed the .lang file stuff!

Loading...