Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Settings
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by fml2 - 2008-01-27
Last edited by fml2 - 2011-01-18

FS#8521 - Permanent setting for the pitch

This patch makes the pitch setting permanent. You can set the pitch value that should be used after the DAP is switched on. This can also be changed in the pitch screen but the value set there is NOT saved. So if you set the pitch to 150% in the pitch screen but have it set to 100% in the sound settings, 100% will be used after the next power on.

Also, the initial mode for changing pitch (small steps/semitones) can be set.

Manual is also updated.

I changed the symbol HAVE_PITCHSCREEN to HAVE_ADJUSTABLE_PITCH since it better tells what we’re at IMHO.

   pitch_setting.patch (34.2 KiB)
Closed by  fml2
2011-01-18 10:07
Reason for closing:  Rejected
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

Since the pitch is stored in bookmarks since some time now, this feature is not needed much any more.

kugel. commented on 2008-01-27 16:43

Not nessecary. You can define a default pitch level in fixed.cfg (or was it fixed_config.cfg?).

fml2 commented on 2008-01-27 18:43

I think, there is no setting for the pitch yet. Hence you can’t define it via the config file (no matter which one). This patch makes it possible. Another question is whether we should allow the user to set the two settings via the GUI. I chose to do so. Otherwise they will be forgotten and will never be used.

Another note: it would be good if some indicator would display in the status bar if the pitch is not exactly 100%. Maybe different symbols for <100% and >100%. But this would be the subject for another patch once this is committed.

kugel. commented on 2008-01-27 18:58

Ok, a little test revealed that there’s no pitch/speed setting in the config.

However, it would be easier and cleaner to just introduce a persistent. Especially after reading the title of this, I think your patch does too much at once, and it doesn’t do it the right way.

But then again, I’m no dev. I’m just stating my opinion.

fml2 commented on 2008-01-27 19:50
I think your patch does too much at once, and it doesn’t do it the right way.

Care to elaborate on what would be not too much and what would be the right way to do it?

kugel. commented on 2008-01-27 19:56

Your change HAVE_PITCHSCREEN to HAVE_ADJUSTABLE_PITCH is definitely not need at all. HAVE_PITCHSCREEN does the job very well, since the pitchscreen is only activated for those target for which HAVE_PITCHSCREEN is defined.

Also, you are introducing several new settings. Are you sure they are needed, and worth the bin size grow and the menu cluttering? I don’t think so.

fml2 commented on 2008-01-27 21:00

I introduced the settings in the menu because, as I understand, there are no settings without GUI access in Rockbox. The symbol name might indeed be changed in a separate patch but I think the new name better describes the feature, i.e. the capability of the player to adjust the pitch. And if the player is capable of that, Rockbox will of course allow that. So HAVE_PITCHSCREEN is an ‘implication’ of HAVE_ADJUSTABLE_PITCH.

Anyway, (positive ;-) comments from other people are welcome.

alexwallis646 commented on 2008-03-09 13:58

Hi. Could this patch please be updated to work with the latest svn code?

brihall commented on 2010-08-03 15:06

Please commit some variation of this to mainline. I use my e200 mainly for listening to podcasts while commuting, and it is tiring having to adjust the playback speed to my preferred 183% on every boot.

brihall commented on 2010-12-13 18:48

For my use case (persistent speed setting for podcast playback), this patch works great:  FS#8521  - Permanent setting for the pitch

fml2 commented on 2011-01-18 10:06

I will close this task since we have pitch settings in bookmarks now, which mostly makes the feature unneeded.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing