PDA

View Full Version : Advice needed on development in Command module



mwphoto
2005-05-28, 00:53
I want to add some functionality to slim and I'd like some advice on the best approach. What I want to do is to change the preferences depending on whether a playlist or album is playing. In particular I want to:

1) set crossfade to none for albums
2) set crossfade to some value for playlists

I've been digging around and I can see several ways of doing this, so I'd like some help from the people that know it better which is the best way. One thing to bear in mind - I'd like to make sure the changes make it into the production version so I don't exist on another code branch for too long.

Approach 1a

Establish a callback that happens on playlist 'addalbum' and playlist 'add' and then write a plugin to do the job. This is the cleanest simplest I think. As I understand it the callback mechanism for Commands is already in use by Browse, BrowseID3 etc so I'm not sure how I could use this without making changes in Browse(ID3) as well.

While I'm on the topic of callbacks I haven't found a list of events that can trigger callbacks yet - if anyone can point me at one I'd be grateful.

Approach 1b

More generically implement the possibility of 'listener' functionality in the Command module so a plugin can define and register a 'listener' that gets called every time a command executes. The listener would get called with the command parameters and could do what it liked with them. This would be relatively straightforward but I'm concerned about performance as every command would be replicated to the listener (even if the listener did nothing)

Approach 1c
Long term the Command module could be reworked into a more object model so a plugin should simply override 'playlist' 'addalbum' with a new function. This is much more compatible with the listener model above. However if this approach is to be used then obviously the developers need to buy in to this approach, it's too radical and too close to the core of the system to sit in a branch on its own.

Approach 2

Just hack the code for Browse and BrowseID3. This provides a somewhat fixed solution which I doubt would get accepted in the production code branch.

Approach 3

Put in a more generic preference changing framework (lots of work) but potentially very powerful. I was thinking of something like a 'pseudotrack' that can exist in a playlist. When the 'track' is played it changes the preferences. This allows preferences to change over the duration of a playlist (although it leaves some difficulty with what to do with track shuffling.

For now I'll probably just hack a bit to get a better feeling for the code. I'd be interested in hearing views on the above approaches, and any alternatives that might suit my purpose.

Thanks all

Malcolm

kdf
2005-05-28, 02:05
Quoting mwphoto <mwphoto.1pqf1n (AT) no-mx (DOT) forums.slimdevices.com>:


> While I'm on the topic of callbacks I haven't found a list of events
> that can trigger callbacks yet - if anyone can point me at one I'd be
> grateful.

The top part of Command.pm lists most (if not all) of the commands and
parameters for the CLI in a comment block. All of these commands will issue a
callback

> Approach 1b
>
> More generically implement the possibility of 'listener' functionality
> in the Command module so a plugin can define and register a 'listener'
> that gets called every time a command executes. The listener would get
> called with the command parameters and could do what it liked with
> them. This would be relatively straightforward but I'm concerned about
> performance as every command would be replicated to the listener (even
> if the listener did nothing)

I like the possibilities from this approach. It sounds to me like profiles,
where you can set a group of params with one identifier. There are bug
requests for keeping things like volume, shuffle, repeat mode and current song
across use of a given playlist. The db could store a 'last used listener' for
a playlist url and keep that. Possibly, a conf file could be used to store the
listener combinations, and even defauls on when to use (based on commmands or
the objects used in those commands). Linking to the object is probably more
long term. The commands in Command.pm are a bit redundant, and I would expect
that to be cleaned up at some point.
>
> Approach 1c
> Long term the Command module could be reworked into a more object model
> so a plugin should simply override 'playlist' 'addalbum' with a new
> function. This is much more compatible with the listener model above.
> However if this approach is to be used then obviously the developers
> need to buy in to this approach, it's too radical and too close to the
> core of the system to sit in a branch on its own.
>
> Approach 2
>
> Just hack the code for Browse and BrowseID3. This provides a somewhat
> fixed solution which I doubt would get accepted in the production code
> branch.

Don't bother with BrowseID3. Its still in use for 6.0.x, but is already gone
from 6.1 nightlies and will likely never see another official release. Look
into BrowseDB if you want to go this route.

> For now I'll probably just hack a bit to get a better feeling for the
> code. I'd be interested in hearing views on the above approaches, and
> any alternatives that might suit my purpose.

Try taking a look through bugs.slimdevices.com while you are at it. You may
twig on a few existing reports that could be easily fold into what you are
already working on. More work, yes, but also will be a great help for
slimserver development and make it even more desireable to merge it into core
code.

For hacking, stick with 6.1 nightlies or the subversion trunk. The 6.0.x
branch/builds have fallen behind the state-of-the-art.

-kdf

Marc Sherman
2005-05-28, 07:47
kdf wrote:
>
> I like the possibilities from this approach. It sounds to me like
> profiles, where you can set a group of params with one identifier.
> There are bug requests for keeping things like volume, shuffle,
> repeat mode and current song across use of a given playlist. The db
> could store a 'last used listener' for a playlist url and keep that.
> Possibly, a conf file could be used to store the listener
> combinations, and even defauls on when to use (based on commmands or
> the objects used in those commands). Linking to the object is
> probably more long term. The commands in Command.pm are a bit
> redundant, and I would expect that to be cleaned up at some point.

Just to clarify the confusion of terminology here, mwphoto wasn't using
the word "listener" to refer to a set of playback paramters; rather, he
was referring to a Listener interface as used in Java, which in other
languages is usually referred to as a callback interface.

Given the predominance of Java in the Slimserver community (ie:
Softsqueeze), it would be a bad thing to start using the word Listener
to mean anything else in the SS codebase.

- Marc

kdf
2005-05-28, 08:15
Quoting Marc Sherman <msherman (AT) projectile (DOT) ca>:


> Just to clarify the confusion of terminology here, mwphoto wasn't using
> the word "listener" to refer to a set of playback paramters; rather, he
> was referring to a Listener interface as used in Java, which in other
> languages is usually referred to as a callback interface.
ah, right, good point!

lets call my response a 'profile' and I'l still like that idea ;)

If some sort of plugin or helper app could do the trick, it would be great. Not
everyone would need the feature but those who did can easily add it. An
internal process would probably have an easier time handling linking of
paramters to specific track/library date, however.

-kdf

Robert Moser
2005-05-28, 10:23
mwphoto blurted out:
> Approach 1b
>
> More generically implement the possibility of 'listener' functionality
> in the Command module so a plugin can define and register a 'listener'
> that gets called every time a command executes. The listener would get
> called with the command parameters and could do what it liked with
> them. This would be relatively straightforward but I'm concerned about
> performance as every command would be replicated to the listener (even
> if the listener did nothing)

I believe this is already present. Use the
Slim::Control::Command::setExecuteCallback() function to register the
'listener'. It will get called every time a command executes and it
will be provided the list of parameters used in that command.

I believe the SlimScrobbler plugin uses this, so that might be a good
place to look.

mwphoto
2005-05-29, 09:36
Ok,

Thanks for the help, I used slimscrobbler as a starting point (as it already uses the callback mechanism). I've now got a plugin that changes the preferences the way I want.

I have 2 further observations:

1) Is there a standard place people are keeping plugins? At the moment I'm not sure anyone would want the plugin I've written, but I'm sure I'll develop it further and I'm happy to share it.

2) One major drawback for me is that there isn't a consistent set of commands for doing jobs. What I've done is got the plugin to respond to the 'addalbum' commands. However if you use the web interface it calls 'inserttracks' which then makes it difficult to identify that the job is actually adding an album.

I wonder if it's worth cleaning up the command interface so there's only one way of doing things? In general having multiple ways of doing the same job is useful, but in this case they all seem to based around duplicated code. It means that anyone that wants to intercept commands has a difficult job of reproducing the execution logic to work out what is about to happen. (There are comments to this effect in the SlimScrobbler plugin). I have a feeling the 'flexible' interface should be higher up and the callbacks should be implemented at a lower level. I'm not sure that's very clear - but I haven't got a better way of expressing it right now.

More later as my plugin developments continue :)

Malcolm

Fred
2005-05-29, 15:33
> 2) One major drawback for me is that there isn't a consistent set of
> commands for doing jobs.

I agree with you (see below), but at the end of the day, even if there
is a single ADD_ALBUM_TRACKS command/callback, nothing prevents "rogue"
code to add an album one song at a time using ADD_SINGLE_TRACK.

It seems to me that only the resulting playlist counts for your
application, no ?

Anyway...

> What I've done is got the plugin to respond to
> the 'addalbum' commands. However if you use the web interface it calls
> 'inserttracks' which then makes it difficult to identify that the job
> is actually adding an album.
>
> I wonder if it's worth cleaning up the command interface so there's
> only one way of doing things? In general having multiple ways of doing
> the same job is useful, but in this case they all seem to based around
> duplicated code. It means that anyone that wants to intercept commands
> has a difficult job of reproducing the execution logic to work out
> what is about to happen.

Agreed. And I was planning to try and revisit some of that to support a
"higher level" asynchronous CLI. Right now "listen" is basically a
callback for every event... and it is quite confusing. So I know how you
feel in the plug-in.

One reason for the current state of affairs is that the transition from
URL to IDs is still partial or hacked out together. In my view
Command.pm should operate only on IDs and callers (plug-ins or http or
CLI) should worry about any URL to ID translation (f.e. to support
legacy CLI calls). But then again so far playlist are still URL based.

Also Command.pm and the callback today is used for CLI queries which
change nothing. That could be updated as well.

Enough to keep us busy for the next year or so :-)

Fred




mwphoto wrote:
> Ok,
>
> Thanks for the help, I used slimscrobbler as a starting point (as it
> already uses the callback mechanism). I've now got a plugin that
> changes the preferences the way I want.
>
> I have 2 further observations:
>
> 1) Is there a standard place people are keeping plugins? At the moment
> I'm not sure anyone would want the plugin I've written, but I'm sure
> I'll develop it further and I'm happy to share it.
>
> 2) One major drawback for me is that there isn't a consistent set of
> commands for doing jobs. What I've done is got the plugin to respond to
> the 'addalbum' commands. However if you use the web interface it calls
> 'inserttracks' which then makes it difficult to identify that the job
> is actually adding an album.
>
> I wonder if it's worth cleaning up the command interface so there's
> only one way of doing things? In general having multiple ways of doing
> the same job is useful, but in this case they all seem to based around
> duplicated code. It means that anyone that wants to intercept commands
> has a difficult job of reproducing the execution logic to work out what
> is about to happen. (There are comments to this effect in the
> SlimScrobbler plugin). I have a feeling the 'flexible' interface should
> be higher up and the callbacks should be implemented at a lower level.
> I'm not sure that's very clear - but I haven't got a better way of
> expressing it right now.
>
> More later as my plugin developments continue :)
>
> Malcolm
>
>