- Status Closed
- Percent Complete
- 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 Alexander Levin - 2008-01-27
Last edited by Alexander Levin - 2011-01-18
Opened by Alexander Levin - 2008-01-27
Last edited by Alexander Levin - 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 Alexander Levin
2011-01-18 10:07
Reason for closing: Rejected
Additional comments about closing:
2011-01-18 10:07
Reason for closing: Rejected
Additional comments about closing:
Since the pitch is stored in bookmarks
since some time now, this feature is not
needed much any more.
Not nessecary. You can define a default pitch level in fixed.cfg (or was it fixed_config.cfg?).
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.
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.
Care to elaborate on what would be not too much and what would be the right way to do it?
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.
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.
Hi. Could this patch please be updated to work with the latest svn code?
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.
For my use case (persistent speed setting for podcast playback), this patch works great:
FS#8521- Permanent setting for the pitchI will close this task since we have pitch settings in bookmarks now, which mostly makes the feature unneeded.