Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Music playback
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Alexander Levin - 2006-10-08

FS#6145 - Pitch screen: up/down by half tone

This patch allows to change pitch not only by a certain amount of percents but also in halftone steps. Pitch changing mode is toggled by pressing MODE button while in the pitch screen.

Key mapping is only provided for H1xx since I don’t know which buttons are the best candidates for this on other players.

Also a small bug is corrected reg. nudging: if nudging would exceed the allowed range then it’s not executed. But on button release, the “denudging” was always executed – regardless whether the nudging was previously applied.

TODO:
- the active mode should somehow be displayed on the screen (an asterisk?)
- half tone pitching is not very accurate, the error is accumulated. But for simple pupose of transposing a piece it’s OK.

I’m very interested in your opinions about this patch.

Closed by  Linus Nielsen Feltzing
2006-11-06 10:12
Reason for closing:  Accepted
Alexander Levin commented on 2006-10-09 21:13

Updated the patch:

- on large screens, the mode is displayed (Pitch Up/Down or Semitone Up/Down). On very small screens, there is no mode indicator. The right word is ‘semitone’, not ‘halftone’.
- in semitone mode, repeat is switched off
- pitch mode is retained between ‘pitch sessions’

TODO: semitone pitching is still not very accurate. Pitching up from 100% and then down will not result back in 100%. But for my purpose (comfortable strumming while listening to music) this is enough.

It’s now committable IMHO.

Alexander Levin commented on 2006-10-10 11:40

PS. E.g. going one semitone up from 100.0% results in 105.9%. Then, going one semitone down results in 99.9% (and not 100.0%). This is due to the fact that pitch is held as an int.

Barry Wardell commented on 2006-10-10 18:27

Would it be possible to just use a hardcoded array of values for the percentage, rather than calculating each time and having to deal with round off errors?

Alexander Levin commented on 2006-10-10 20:45
Would it be possible to just use a hardcoded array of values for the percentage, rather than calculating each time and having to deal with round off errors?

No, I’m afraid no. Since you can change pitch in normal mode a bit, then switch to semitone mode, and then further fine adjust the pitch in normal mode.

Alexander Levin commented on 2006-10-10 20:46

Further updated the patch:

- solved the problem with inaccurate semitone pitch change. Now it’s really accurate.
- added key mapping for main device

TODO: key mapping for remote. I can’t and will not do that since I’ve never used the remote. Anyone?

Another note: on H120, reset to 100% on the main unit is mapped to PLAY. Why? Wouldn’t it be better to do it with NAVI? This would also allow to leave the pitch screen with PLAY.

Steve Bavin commented on 2006-10-11 06:13

Why not have a seperate pitch offset, then look that up from a table of 12 semitones and add it to the current pitch value? You could of course double/halve for octaves (if that much range is needed).

Alexander Levin commented on 2006-10-11 17:17

Committers? Anyone? Is the patch not good enough? I’m afraid if it doesn’t get committed fast something will become stale and will require much effort to keep up. (That probably happens to 90% of submitted patches.)

Admin
Linus Nielsen Feltzing commented on 2006-10-12 07:03

We’re not too happy about floating point operations in the core code. Use fixed point.

Alexander Levin commented on 2006-10-12 15:07
We’re not too happy about floating point operations in the core code.

Why not? Is it because not all devices (CPUs) support it? Because a performance issue it is definitely not.

Thom Johansen commented on 2006-10-12 16:15

None of our CPUs support it, so emulation code needs to be linked in, which makes both bigger and slower code than necessary. We use fixed point absolutely every other place where math like this is done, so we should do it here too.

Admin
Linus Nielsen Feltzing commented on 2006-10-12 16:59

None of the targets so far support floating point natively. I agree that it is hardly a performance issue in this case, however. I suspect that the binary size increases somewhat when you include floating point code.

Alexander Levin commented on 2006-10-12 19:33

Updated the patch: replaced floating point arithmetics with integers (well, actually long). I hope, unsigned long is at least 4 bytes long on all platforms.

Here are the pitches (as speed promille) when changing it in semitone steps (for quality assurance :-):

Up: 1059, 1122, 1189, 1260, 1335, 1414, 1498, 1587, 1681, 1781, 1887, 1999
Down: 944, 891, 841, 794, 749, 707, 667, 630, 595, 562, 530, 500

Alexander Levin commented on 2006-10-12 19:45

Sorry, forgot one thing: the constant should be used for initialising the pitch_mode

Alexander Levin commented on 2006-10-12 20:14

PS. The values above are computed when starting at 100.0%. If you change the pitch in normal mode and then switch to semitone mode, you’ll of course have other values.

Alexander Levin commented on 2006-10-12 20:21

One more small correction: use ‘uint32_t’ instead of ‘unsigned long.’

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing