- Status Closed
- Percent Complete
- 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
FS#5344 - Separate "Shuffle" and "Track Skip" settings for Crossfade
Proposal: Separate the options in “Crossfade Settings” for “track skip” and “shuffle”.
Currently you can select “shuffle”, “track skip only”, “always”, and “never”.
What if i want “shuffle” AND “track skip”? when i have Shuffle on, i want to have every song change crossfaded, but when i have regular or album play on, i don’t (necessarily) want crossfade unless i skip a track. Currently, switching back and forth between “shuffle” and listening to an album (or intentional playlist which i don’t want crossfaded) requires that i change the CF settings too. if there were a “shuffle and track skip” option, or both options were selectable separately, it would take some hassle out of it. Plus, the code is already there, so it should be easy to implement.
After this is implemented, a few easy modifications could be made to allow for separate crossfade settings for “track skip” versus regular crossfading (see http://www.rockbox.org/tracker/task/4953 ).
Loading...
Available keyboard shortcuts
- Alt + ⇧ Shift + l Login Dialog / Logout
- Alt + ⇧ Shift + a Add new task
- Alt + ⇧ Shift + m My searches
- Alt + ⇧ Shift + t focus taskid search
Tasklist
- o open selected task
- j move cursor down
- k move cursor up
Task Details
- n Next task
- p Previous task
- Alt + ⇧ Shift + e ↵ Enter Edit this task
- Alt + ⇧ Shift + w watch task
- Alt + ⇧ Shift + y Close Task
Task Editing
- Alt + ⇧ Shift + s save task
agreed i really would like to see a “shuffle and track skip” option for crossfade
Ok, so i’ve just gone ahead and made a patch to add “Shuffle AND Track Skip” to the “crossfade enable” menu. My preferred implementation (described above) would be much more involved to do, and may even necessitate rewriting crossfade significantly… for now, this does what *I* want, so i’m happy
everyone test, let me know what you think :)
Haven’t tested it yet, but I like the idea.
Didn’t you change anything in lang/english.lang as well?
(where is LANG_SHUFFLE_TRACKSKIP #defined?)
Whoops. I had done it, but forgot to copy the file into the source structure from which i took the diff.
Here’s an updated one with the change to english.lang included.
Whoops again: just found a big error in my code (actually just a big misunderstanding by me) regarding how automatic-track-changes are crossfaded (or not), such that it crossfaded even if “shuffle” was off (which goes against the whole point of my patch).
New version: should completely solve that; it now track-changes exactly how it does when crossfade is set to “shuffle” but “shuffle” is set to OFF. (except, of course, it will also crossfade when you manually track-skip no matter what the mode.)
Ok, this time I’ve just fished out one little bug that made it act weird. This *MAY* be the last update :)
One more tiny little bugfix (it would prevent a setting of “Always” in “Enable Crossfade” from being saved upon reboot)…
Why do you change the #define CONFIG_BLOCK_VERSION? It seems to me that you only *changed* an entry in the config struct, not added one!?
Any ideas?
Hmm. not too sure why I changed that.. IIRC I incremented it because I added a new option in the settings menu, and I thought it was supposed to be when that happened. Maybe a dev can clear this issue up for me…
AFAIU (and this is not a lot) you have to change it when you add an etnry only. But anyway, maybe a dev can comment.
Falco98 was correct in incrementing the CONFIG_BLOCK_VERSION - he changed the definition of the crossfade option from using 2 bits to using 3 bits, which means all the settings subsequent to that option are no longer in the same place in the config block.
The only time you can change the settings struct and not increment the CONFIG_BLOCK_VERSION is if you add a new item at the end. In that case, all the existing settings stay in the same place, so there is no problem.
thanks for clearing this up
would anyone be willing to clean this up (i.e. synch it to recent CVS) and push it for inclusion? I would except i’m on the road 6 days a week these days, so i’m a bit rusty on the rockbox thing.
thanks!
tried to sync to today’s cvs. am i correct in changing the CONFIG_BLOCK_VERSION to 55? (it is currently 54 in cvs). this seems to break crossfading all together- the setting for this patch exists but none of the crossfading options actually work (except for off -_-). hope somebody can take a look at it.
nevermind. it works. settings need clearing.
ok, well there doesnt seem to be any objection to adding this, so I have cleaned up the patch a bit and its attached to this.
One thing tho, is there a better way to set out the options?
<quote linuxstb on irc>Just seems to me that there are three logical on/off settings - crossfade during normal playback, crossfade during shuffled playback, and crossfade during manual skips.</quote>
which does sound reasonable to me, but neither of us use crossfade… so we could be wrong.
the other thing I wanted to check is if pcmbuf_is_crossfade_enabled() in pcmbuf.c needs changing also?
JdGordon:
you guys are right about there being 3 logical on/off settings; however, as it’s been written, it only allows them to be used mutually-exclusively. separating out the options (as this original feature request asked for) would be a better implementation, but it’s more code changed than I know how to do.
also: pcmbuf_is_crossfade_enabled() may have to be changed or maybe not; i’ll simply say that i’ve been running with this patch for 6 months now and it seems to work fine. what does that variable do, exactly? you might be right that it should be changed, i wouldn’t be sure one way or the other.