PDA

View Full Version : Patch: Playlists in the database



Dan Sully
2005-05-31, 18:28
Here's round one of a patch to put the Now Playing playlists in the database.
Previously the now playing playlist was written out as '__$clientMac' in the
playlist directory. This was slow for both reading (upon startup) and writing
to disk, which would go back to the database many times for objectToUrl
calls, as the 'playlist' in memory was just a list of urls. The in-memory
playlist is now a list of objects, which makes loading the status page much
faster as well.

I've done an initial conversion of the Default skin to not use the old
'append' and 'play' methods with itempath, and use the *tracks commands instead.

Still todo is keeping user created playlists in the db, and periodically
writing them out to disk.

Please try it out and break it. I'm sure kdf will within moments of applying. :)

-D
--
It is dark. You are likely to be eaten by a grue.

kdf
2005-06-01, 01:04
Quoting Dan Sully <dan (AT) slimdevices (DOT) com>:


> Please try it out and break it. I'm sure kdf will within moments of applying.
> :)

not yet ;)

of course, I'm only using the road library with 800 songs, osx 10.3.9. Seems ok
so far. Bad day for a big test patch like this for me since I'm off to Cork in
about an hour. I wont have time to connect home and try this out until
saturday.

-kdf

Robert Moser
2005-06-01, 08:45
When using the listref version of the *tracks command, there is no need
to do $client->param just for that listref.

Instead of this:

$client->param('searchResults', [ $currentItem ]);
$client->execute(["playlist", $command, 'listref=searchResults&']);

You can do this:

$client->execute(["playlist", $command, 'listref', [ $currentItem ] ]);

And instead of this:

$client->param('nowPlayingPlaylist', \@tracks);
$client->execute(['playlist', 'addtracks','listRef=nowPlayingPlaylist'],
\&initial_add_done,[$client, $currsong]);

You can do this:

$client->execute(['playlist', 'addtracks','listref', \@tracks ],
\&initial_add_done,[$client, $currsong]);


Also, in Slim::Player::Client::startup() you have a redundant
$::d_client && in front of the $client->debug.

I haven't actually tried the patch out yet, but I will soon.

Robert Moser
2005-06-01, 09:10
The only problem I've found so far is that there is a bit of a lag
between playlist modification and writing that modification to the
database. If you load up a bunch of tracks, then kill the server, when
you restart, the tracks won't be there. Similarly, if you clear your
playlist, then restart the server, the old pre-cleared playlist will be
restored.

Maybe we need to forceCommit in the main cleanup() routine?

Dan Sully
2005-06-01, 09:36
* Robert Moser shaped the electrons to say...

>The only problem I've found so far is that there is a bit of a lag
>between playlist modification and writing that modification to the
>database. If you load up a bunch of tracks, then kill the server, when
>you restart, the tracks won't be there. Similarly, if you clear your
>playlist, then restart the server, the old pre-cleared playlist will be
>restored.
>
>Maybe we need to forceCommit in the main cleanup() routine?

Yeah - I noticed that as well, and was pondering the same thing.

I'll add it.

-D
--
There was supposed to be a big kaboom.

Robert Moser
2005-06-01, 09:57
Dan Sully wrote:
> * Robert Moser shaped the electrons to say...
>
>> The only problem I've found so far is that there is a bit of a lag
>> between playlist modification and writing that modification to the
>> database. If you load up a bunch of tracks, then kill the server,
>> when you restart, the tracks won't be there. Similarly, if you clear
>> your playlist, then restart the server, the old pre-cleared playlist
>> will be restored.
>>
>> Maybe we need to forceCommit in the main cleanup() routine?
>
>
> Yeah - I noticed that as well, and was pondering the same thing.
>
> I'll add it.
>
> -D

Unfortunately it won't work for Windows systems, because we don't trap
$SIG{INT} there.

If I move the
$SIG{INT} = \&sigint;
line out of the
if (Slim::Utils::OSDetect::OS() ne 'win') {}
block then having
(Slim::Music::Info::getCurrentDataStore())->forceCommit();
in cleanup() does the trick.

So, why don't we trap $SIG{INT} under Windows?

Dan Sully
2005-06-01, 10:12
* Robert Moser shaped the electrons to say...

>Unfortunately it won't work for Windows systems, because we don't trap $SIG{INT} there.
>
>So, why don't we trap $SIG{INT} under Windows?

That's a Dean question - it goes back to 'rev 2' in subversion, which means
it was imported from SourceForge, and we lost history there.

This might provide some information though:

http://use.perl.org/~djberg96/journal/22147

-D
--
<dmercer> Because that is what our industry does.
Churns out useless shit. Followed by inferior re-implementations of useless shit.

Robert Moser
2005-06-01, 10:15
Robert Moser wrote:
> So, why don't we trap $SIG{INT} under Windows?

Deep digging revealed this (the funny part being that it was me who
checked it in):

Revision 1.190 Aug 29, 2002 (server.pl)
"Catching $SIG{INT} under Windows 98 Active Perl 5.6.1 was causing an
exception in kernel32.dll and a page fault in perl56.dll."

So we could probably throw in a version check in addition to checking
for Windows. There definitely isn't a problem with ActivePerl 5.8.4
under Windows XP. This problem was most likely fixed in Perl v5.8, as
Safe Signals is listed as a core enhancement in perl58delta.

Robert Moser
2005-06-01, 10:23
Dan Sully wrote:
> That's a Dean question - it goes back to 'rev 2' in subversion, which means
> it was imported from SourceForge, and we lost history there.

The history is still there in SourceForge, it just didn't get migrated
to the SlimDevices CVS. So for deep digging just go to
http://cvs.sourceforge.net/viewcvs.py/slimp3/slimp3/. I had to visit
the Attic to find the change that took out $SIG{INT}, but it was
definitely still there.

Dan Sully
2005-06-01, 10:28
* Robert Moser shaped the electrons to say...

>>So, why don't we trap $SIG{INT} under Windows?
>
>Deep digging revealed this (the funny part being that it was me who
>checked it in):
>
>Revision 1.190 Aug 29, 2002 (server.pl)
>"Catching $SIG{INT} under Windows 98 Active Perl 5.6.1 was causing an
>exception in kernel32.dll and a page fault in perl56.dll."
>
>So we could probably throw in a version check in addition to checking
>for Windows. There definitely isn't a problem with ActivePerl 5.8.4
>under Windows XP. This problem was most likely fixed in Perl v5.8, as
>Safe Signals is listed as a core enhancement in perl58delta.

Works for me. I don't see why we even need a check.. We're perl 5.8 all the time on Windows.

-D
--
<Nigel> Please refrain from fearing the reaper.