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 Falco98 - 2006-05-11
Last edited by midkay - 2006-05-18

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 ).

Closed by  jdgordon
2006-12-19 01:26
Reason for closing:  Accepted
ep0ch commented on 2006-05-16 10:16

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?

(application/octet-stream)    changes (5.4 KiB)

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.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing