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-10
Last edited by jdgordon - 2006-06-29

FS#5338 - Insert Random Folders

I propose that the context-menu for folders (specifically those containing other folders) be given a new context-menu option similar to “insert shuffled”, but in which only the folders are mixed up, and the tracks are left to play in-order (creating a “random CD” effect).

There are currently at least 3 other feature requests open for this general idea: one proposes that, in normal play, once a folder is completed, the “next track” be chosen from a randomly-selected folder. Another proposes a list of all playlists be kept, and one chosen at random. Yet another requests that the “next track” be chosen from a random “album” listing in TagCache.

What I ask for here is fundamentally different, and IMHO would be easier to implement (’insert recursive folders’ and ‘insert shuffled’ are already there, so there’s plenty of example code to work with), and easier to test (just view the playlist generated, see how random it is, make sure it contains all subfolders, etc). Plus if anyone is really familiar with playlist.c, it may even be possible to do without having to create whole new functions.

[working patch here: http://www.rockbox.org/tracker/?getfile=11854]

Closed by  jdgordon
2006-10-09 11:25
Reason for closing:  Rejected
Additional comments about closing:  

random folder advance is in so this wont be accepted

for what it’s worth, this is my preferred implementation of the feature. +1

hardeep and I have gone over preliminary implementation ideas in IRC.. Attaching a text snippet from the IRC log to share.

as of right now i’m officially offering a $10 bounty (10 USD paid by Paypal) for a complete, working patch for this (to be used, at least, on my iriver h140). I will possibly consider a higher bounty (plus project donation) if it can get included in the official release, at least after 3.0 has been finished. PM me on the forums for more info.

done!
it actually worked first go which is pretty amazing :D this is limited to 512 directories tho (32 on the archo’s) but its good enough.. also its not really all that random

hahaha, im such an idiot…. ignore that above patch.. its complet crap..

ok, well i fixed it.. its actually working..
except i dont like how the found directories are stored so im redoing that little bit which is taking longer than i expected.. i guess coz im asleep…

ill hopefully get it completly finished tomorow (too bad this wont get added (if at all) untill after 3.0..)

I’m trying out the patch now. It seems to be working, though I haven’t run it enough times to tell just how well it might randomize, etc. Also, it seems to take a rather long time to initialize, even compared with the normal “insert shuffled” command.

the randomisation shold be pretty good… its the same shuffle algorithm the rest of rb uses..
as for the init time.. ye i know, if you dont have dircache on then this may be wa to slow to be usable.. im sure there is plenty of room for optimization tho so ill keep playing..
if i can get it fast enough to be really useable then ill try using this for jumpt to a random dir at end of current dir thingy

i’ve figured out one reason it might _seem_ slow – the counter it displays as it adds folders stays at “0” the whole time (or counts to 10 or 12 or however many tracks an album has, and resets to “0” so fast that all we see is a continuous “0”). It may also be *actually* slow compared to the normal “insert shuffled”, but seeing the “0” there for a minute probably plays tricks on the mind.

as promised… new version with 1 minor bug somewhere…

ok, here we go.. i tihnk this works (a call to srand somehwere might be good, but not really needed..)
no more storing indicies of any sort.. and only a minor addition to the current add_directory_to_playlist function :D

drinks all round i tinhk.. (oh this version lets u add to an exsisting playlist… it _should_ only add after the current track, but thats not tested…)

I have a solution for the end-case bug i was talking about with you earlier. it hit me about a minute after you left IRC :-P

here’s the algo, approximately:

i = random() % num_of_tracks

if i >= (num_of_tracks / 2)
{

 count UP to find next folder

}
else [if i < (num_of_tracks / 2)]
{

 count DOWN to find next folder

}

easy!! and it starts working from even the 2nd folder added, the middle-cases should work fine, and it doesn’t bias the beginning or end of the playlist too much. let me know if it seems like it’ll work.

it has since been pointed out to me (thanks Hardeep) that a simple coin-flip operation would be enough to determine whether to count UP or DOWN (let’s say, for example, if the random number is odd, you count down, and if it’s even, count up…)

grrr… this is it.. all working and everyone is happy :D

crap.. change line 155 from “+ if (r<0) r=(c-1);” to “+ if (r<0) { r=0; break; }

take 7 :p

i can’t get this one to work either - it compiles, but when used it puts the albums in order, except it puts all songs except two from the second album at the end

I’ve just noticed the same problem Ego describes: almost all the albums are completely in order.

bugger… i thought that was fixed??? back to the drawing board then :’(

JdG:
this is a new, and as far as i know, different problem. I’m thinking it might be a bug in its detection of folder boundaries or something like that. I’d try to debug the code myself but i’m still just a bit too confused by it :-/

hmm… i havnt had a closer look yet, but it sounds very much like a bug i had before the coin flip code… i thought i fixed it but i might have brought it back.. its possible its doing it if the directinon is -1 (but like i said, i havent investigated yet, so im purely guessing)

right.. well i restarted and implemented it a tad differently..
i think this version is a bit simpler to follow.. but its randomness needs a bit of work..
lemme know what u tihnk…

this one seems to work right. thanks :)

http://pastebin.com/720095

i’ve implemented the cointoss, and already-playing case. i *think* i’ve sorted out the folder-split problem, though it could use some further testing.

issues:
– normal “insert shuffled” (the included rockbox function) seems to be broken. i don’t think it’s in the playlist.c function itself, so check elsewhere.
– something in what’s written to the .playlist_control file is messing things up – in the simulator at least – for instance, if you do “insert tree randomly” (and it works), then stop, and try it again, it will NOT work. that might be something unique to the simulator tho, as i haven’t had the problem (yet) on my h140. (i also haven’t tried the latest code on it yet. too tired.)

here we go again..

hmm.. you reintroduced one of the bugs i’d taken care of yesterday when you condensed my code ;-) specifically: i think it splits folders when the cointoss is going “-1”.. i’m trying stuff now…

ok fixed it, i think.. it was only one line of code:
line 687 of playlist.c needs to be i++ regardless of direction because of how indexing works.

whoops, not 687, 684… maybe i’m wrong :-P we’ll see…

well, nevermind my last comments.. there is still a folder splitting error.. at this point i have changed your code back to how it was, and disabled the coin flip (meaning it only ever seeks “up”), and it’s still splitting the first folder wrong somehow. seeing if i can fix that now…

ah.. i THINK i have it now.
when it finds the folder boundaries, you have it do “i += direction; break;” instead, it needs to do “if (direction = -1) i++; break”

it seems to work after several tests. i’ll let ya know.

bilko commented on 2006-05-16 16:17

It seems a great idea to be able to play entire albums from a shuffled album list

Bilko: thanks! have you tried out the patch? Hopefully we’ll have a 100% working one up soon for ya, but in the mean time bear with us :-P

Jd: I’ve tested as much as I can in the sim, and will test for a bit in my 140, but as far as i can tell my fix worked (only one line of code too, heh). I also re-hardcoded the coinflip to only go “up” when already_playing, but i guess your way works too [though it seems to me that that way may overly increase the chances of things getting added “last”…]
Pastebin is down at the moment so i’ve put it here instead… careful, it expires after a day.
http://rafb.net/paste/results/KJlECJ99.html

Falco: i owe u a little apology..
you were correct the whole time about needing to do i++ if direction == -1.. so thats in the patch now for good…
as for hardcoding direction to 1 if playlist is started is not needed… just because it adds to the end if u get to the begginging doesnt mean it adds to the liklyhood that it will get added to the end.. in fact if u have it with the ability to goto -1 it decreases it.. even with that constraint… because the whole point of direction is for when it looks for a place to add the folder.. so you could be at the end of the playlist, but direction == -1 so it wont get added at the end which is perfectly acceptable if the playlist has started.

Jd: agreed, i finally admit you have a good point on that (though my way was better for bug testing at least, hehe)

Here is an updated version of the previously-posted patch, in which the only thing changed is the “if( direction = -1 ) i++” thing. I included my line of code about direction-fixing, but it’s commented out.

everyone test! let us know how it is. and if it works perfectly, enjoy!

Just found a bug that would occasionally cause folders to be inserted before the currently-playing song, instead of being sent to a proper place (in this case, the end of the p/l). I believe my small edit has fixed this. I’d appreciate it if anyone can test and try to break the code, so we can further debug it O:-)

ok, there is a break; statement thats been missing since the begining which ive added… also removed some of your comments.. and changed the english… this 1 is ready for as soon as the freeze finishes..

hey, just tried the most recent, this one doesn’t patch cleanly. the english.lang hunk fails

ok, more info. it’d been maybe 5 days to a week since i updated my CVS, and i had been using mike falco’s latest version of the patch so i updated that too. compiling it, i got a fatal error on english.lang, so i tried again from a clean CVS. going through all the patches, none of them can add to the .lang file. i took a look at it to try and correct it myself, but limited as i am i can’t see what the problem is

hell, some things in playlist.c come up as undeclared, too

to be honest the last build I tried it with was 5-19; i’m trying it in 5-22 now since i already had the source sitting around…
ah. damn. I guess when they fixed up some stuff in playlist, they broke the patch.
I know for a fact that it works perfectly with the source from 5-19, so try that out. I’m figuring JdGordon’s latest one should work just fine with it. Meanwhile I’ll try to figure out what got broken…

even with changes to the playlist.c in the cvs it shouldnt matter.. but oh well… the earliest it can be accepted is like tuesday ro something (after the freeze closes) so i was gonna get it merged with the sources then so it works ok.. but whaterver…

bloody patch puts the stuff for playlist.c in the wrong place… all fixed..

would you guys mind syncing this once more before 3.0 comes out? hopefully you won’t have to again :)

big thanks, seriously.

synced to cvs and cleaned up the fs page

synced to cvs (again….)

anyone wanna push this for inclusion in CVS now that the codefreeze is lifted? I’m on the road 6 days a week nowadays, so i’m not really able to get all into it these days until this project is over with.
thanks!

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing