2010-01-28, 00:22
Hi there,
I've had a quick review of the first patch, and have some general questions to begin with. We can take this to trac if you think it will be better there (after all, this is general discussion) - please feel free to open a ticket, attach your patch (no zipping please) so that others can review it as well.
1. What (if anything) prevents the directory browsing side of it being done in a plugin. Other than small portions (such as where you've injected spotify:// nodes into the library), it appears that very little of this couldn't be done directly externally (whether in a python plugin or via a dll).
2. What (if anything) is required for the streaming side - I'd much prefer if this was split off from the listing side of things where feasible.
3. Why are you adding spotify "albums" to the local database? What's the purpose of this?
Some suggestions for cleanup:
1. Get rid of the spotify-specific messages. They're unneeded as far as I can tell - just send general GUI_MSG_UPDATE_PATH messages.
2. Move the spotify URL stuff that's currently sitting in OnClick() directly into the spotify directory handler. One presumes that your search stuff could be done directly in there as well (i.e. fire off the search, throw up the progress dialog and wait for it's return) further eliminating callback stuff.
Some questions regarding the future:
1. This (like many plugins) appears to provide both browsing ability and search. Perhaps the search side of things could be hooked in in a more general way to XBMC's built-in music search, or perhaps some "find similar items" type of thing at each level in the music lib?
Cheers,
Jonathan
I've had a quick review of the first patch, and have some general questions to begin with. We can take this to trac if you think it will be better there (after all, this is general discussion) - please feel free to open a ticket, attach your patch (no zipping please) so that others can review it as well.
1. What (if anything) prevents the directory browsing side of it being done in a plugin. Other than small portions (such as where you've injected spotify:// nodes into the library), it appears that very little of this couldn't be done directly externally (whether in a python plugin or via a dll).
2. What (if anything) is required for the streaming side - I'd much prefer if this was split off from the listing side of things where feasible.
3. Why are you adding spotify "albums" to the local database? What's the purpose of this?
Some suggestions for cleanup:
1. Get rid of the spotify-specific messages. They're unneeded as far as I can tell - just send general GUI_MSG_UPDATE_PATH messages.
2. Move the spotify URL stuff that's currently sitting in OnClick() directly into the spotify directory handler. One presumes that your search stuff could be done directly in there as well (i.e. fire off the search, throw up the progress dialog and wait for it's return) further eliminating callback stuff.
Some questions regarding the future:
1. This (like many plugins) appears to provide both browsing ability and search. Perhaps the search side of things could be hooked in in a more general way to XBMC's built-in music search, or perhaps some "find similar items" type of thing at each level in the music lib?
Cheers,
Jonathan