Rockbox

Tasklist

FS#5338 - Insert Random Folders

Attached to Project: Rockbox
Opened by Mike Schmitt (Falco98) - Wednesday, 10 May 2006, 16:50 GMT
Last edited by Jonathan Gordon (jdgordon) - Thursday, 29 June 2006, 03:58 GMT
Task Type Patches
Category Music playback
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

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]
This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Monday, 09 October 2006, 11:25 GMT
Reason for closing:  Rejected
Additional comments about closing:  random folder advance is in so this wont be accepted
Comment by Alexander Oster (egotrippen) - Wednesday, 10 May 2006, 18:44 GMT
for what it's worth, this is my preferred implementation of the feature. +1
Comment by Mike Schmitt (Falco98) - Wednesday, 10 May 2006, 21:15 GMT
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.
Comment by Jonathan Gordon (jdgordon) - Thursday, 11 May 2006, 09:20 GMT
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
Comment by Jonathan Gordon (jdgordon) - Thursday, 11 May 2006, 11:08 GMT
hahaha, im such an idiot.... ignore that above patch.. its complet crap..
Comment by Jonathan Gordon (jdgordon) - Thursday, 11 May 2006, 14:26 GMT
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..)
Comment by Mike Schmitt (Falco98) - Thursday, 11 May 2006, 21:08 GMT
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.
Comment by Jonathan Gordon (jdgordon) - Friday, 12 May 2006, 00:33 GMT
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
Comment by Mike Schmitt (Falco98) - Friday, 12 May 2006, 01:39 GMT
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.
Comment by Jonathan Gordon (jdgordon) - Friday, 12 May 2006, 04:17 GMT
as promised... new version with 1 minor bug somewhere...
Comment by Jonathan Gordon (jdgordon) - Saturday, 13 May 2006, 09:47 GMT
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...)
Comment by Mike Schmitt (Falco98) - Saturday, 13 May 2006, 16:24 GMT
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.
Comment by Mike Schmitt (Falco98) - Saturday, 13 May 2006, 18:05 GMT
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...)
Comment by Jonathan Gordon (jdgordon) - Sunday, 14 May 2006, 02:24 GMT
grrr... this is it.. all working and everyone is happy :D
Comment by Jonathan Gordon (jdgordon) - Sunday, 14 May 2006, 02:27 GMT
crap.. change line 155 from "+ if (r<0) r=(c-1);" to "+ if (r<0) { r=0; break; }
Comment by Jonathan Gordon (jdgordon) - Sunday, 14 May 2006, 03:08 GMT
take 7 :p
Comment by Alexander Oster (egotrippen) - Sunday, 14 May 2006, 15:26 GMT
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
Comment by Mike Schmitt (Falco98) - Sunday, 14 May 2006, 16:02 GMT
I've just noticed the same problem Ego describes: almost all the albums are completely in order.
Comment by Jonathan Gordon (jdgordon) - Sunday, 14 May 2006, 23:10 GMT
bugger... i thought that was fixed??? back to the drawing board then :'(
Comment by Mike Schmitt (Falco98) - Monday, 15 May 2006, 00:09 GMT
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 :-/
Comment by Jonathan Gordon (jdgordon) - Monday, 15 May 2006, 01:42 GMT
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)
Comment by Jonathan Gordon (jdgordon) - Monday, 15 May 2006, 14:51 GMT
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...
Comment by Alexander Oster (egotrippen) - Monday, 15 May 2006, 16:18 GMT
this one seems to work right. thanks :)
Comment by Mike Schmitt (Falco98) - Tuesday, 16 May 2006, 06:30 GMT
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.)
Comment by Jonathan Gordon (jdgordon) - Tuesday, 16 May 2006, 07:30 GMT
here we go again..
Comment by Mike Schmitt (Falco98) - Tuesday, 16 May 2006, 14:49 GMT
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...
Comment by Mike Schmitt (Falco98) - Tuesday, 16 May 2006, 14:51 GMT
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.
Comment by Mike Schmitt (Falco98) - Tuesday, 16 May 2006, 14:55 GMT
whoops, not 687, 684... maybe i'm wrong :-P we'll see...
Comment by Mike Schmitt (Falco98) - Tuesday, 16 May 2006, 15:47 GMT
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...
Comment by Mike Schmitt (Falco98) - Tuesday, 16 May 2006, 15:53 GMT
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.
Comment by Steve Blincoe (bilko) - Tuesday, 16 May 2006, 16:17 GMT
It seems a great idea to be able to play entire albums from a shuffled album list
Comment by Mike Schmitt (Falco98) - Tuesday, 16 May 2006, 21:23 GMT
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
Comment by Jonathan Gordon (jdgordon) - Wednesday, 17 May 2006, 00:10 GMT
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.
Comment by Mike Schmitt (Falco98) - Wednesday, 17 May 2006, 02:57 GMT
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!
Comment by Mike Schmitt (Falco98) - Saturday, 20 May 2006, 04:08 GMT
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:-)
Comment by Jonathan Gordon (jdgordon) - Tuesday, 23 May 2006, 12:43 GMT
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..
Comment by Alexander Oster (egotrippen) - Thursday, 25 May 2006, 15:30 GMT
hey, just tried the most recent, this one doesn't patch cleanly. the english.lang hunk fails
Comment by Alexander Oster (egotrippen) - Thursday, 25 May 2006, 15:49 GMT
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
Comment by Alexander Oster (egotrippen) - Thursday, 25 May 2006, 16:04 GMT
hell, some things in playlist.c come up as undeclared, too
Comment by Mike Schmitt (Falco98) - Friday, 26 May 2006, 03:52 GMT
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...
Comment by Jonathan Gordon (jdgordon) - Friday, 26 May 2006, 03:56 GMT
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...
Comment by Jonathan Gordon (jdgordon) - Saturday, 27 May 2006, 09:26 GMT
bloody patch puts the stuff for playlist.c in the wrong place... all fixed..
Comment by Alexander Oster (egotrippen) - Thursday, 22 June 2006, 22:08 GMT
would you guys mind syncing this once more before 3.0 comes out? hopefully you won't have to again :)

big thanks, seriously.
Comment by Jonathan Gordon (jdgordon) - Thursday, 29 June 2006, 03:55 GMT
synced to cvs and cleaned up the fs page
Comment by Jonathan Gordon (jdgordon) - Tuesday, 08 August 2006, 05:36 GMT
synced to cvs (again....)
Comment by Mike Schmitt (Falco98) - Wednesday, 16 August 2006, 03:36 GMT
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...