Bugzilla – Bug 6023
New plugin hooks to implement scanning functions
Last modified: 2011-02-24 05:52:04 UTC
It should be possible for third party plugins to implement scanning actions that are exectued within the scanning process. Today the bundled iTunes and MusicMagic plugins have a special case that lets them execute within the scanner, but there is no way for third party plugins to do the same. The only option available for third party plugins is subscribing on the "rescan done" event and start their scanning action due to that notification. However, there are at least a number of problems with this: 1. It causes the third party plugin scanning action to be executed after the scanning has been marked as finished, which is confusing for the user. 2. The third party plugin scanning is executed in the main SqueezeCenter process instead of in the scanner process. The result is that unless the third party plugin is able to divide its scanning operation in millisecond sized chunks the SqueezeCenter user interface will be really slow during the time and in worst case it might also cause music playback issues. 3. There is no way for third party plugins to control which order their scanning actions are exectued. My suggestion is to make a number of new plugin hooks for the scanner.exe/scanner.pl process which plugins can register for. 1. One hook for plugins that like to run scanning actions that find new tracks, this should be executed in the same place as the iTunes and MusicMagic Import scanners are running today. Some plugins that could use this hook is the bundled iTunes and MusicMagic plugins. 2. One hook for plugins that like to modify or extend the information provided by the standard SqueezeCenter scanning process, this should be executed in the scanner.exe/scanner.pl after Slim::Music::Import->runPostProcessing has been executed. Some plugins that could use this hook is the Lazy Search and the Custom Scan third party plugins. To make the hooks good I think it also would be appropriate to make it possible for the plugins to register a weight parameter. This can be use when sorting the registered scanning plugin to decide in which order they should be executed. Another thing that also might be good is to make it possible for the scanning plugins to get information about which tracks that has been scanned, as an example I think the plugins of the type 2 hooks will benefit of getting information about the last scan time if a "Rescan new and changed songs" scanning has been performed. This would make it possible for these plugin to optimize their scanning to only scan new and change tracks.
I think the two hooks identified in that bug are sensible, and the hook weighting a very good idea, but thinking of Lazy Search I think one or more additional ones would also be helpful. What I'd like to do is register a hook that is called (with weightings to control the order if multiple plugins register) for each track/artist/album etc object before it is to be committed to the database. That could be a separate hook for each object type, or a single one with a discriminating parameter. That would make it trivial for me to compute the Lazy Search 'customscan' column value and poke that into the object before it is committed. That way there wouldn't be any need for a post-scan activity in my case and I could be sure that entries can't be added to the database without them also being 'lazified' (a problem that sometimes occurs at pI think the two hooks identified in that bug are sensible, and the hook weighting a very good idea, but thinking of Lazy Search I think one or more additional ones would also be helpful. What I'd like to do is register a hook that is called (with weightings to control the order if multiple plugins register) for each track/artist/album etc object before it is to be committed to the database. That could be a separate hook for each object type, or a single one with a discriminating parameter. That would make it trivial for me to compute the Lazy Search 'customscan' column value and poke that into the object before it is committed. That way there wouldn't be any need for a post-scan activity in my case and I could be sure that entries can't be added to the database without them also being 'lazified' (a problem that sometimes occurs at present).resent).
Good stuff. Will look at again after 7.0.
Created attachment 3492 [details] Patch for pluggable scaning modules This patch implements: - Removed hard coded references to MusicMagic and iTunes plugins in scanner.pl - scanner.pl dynamically loads plugins which have an "importmodule" element in their install.xml (this has been added to iTunes and MusicMagic) - The scanning modules are executed by weight ordering in two possible phases: -- After the music folder scanning module has been executed (unless they have a 'postScan' key in their importer hash) -- After the artwork scanning (if they have a 'postScan' key in their importer hash) Special changes/considerations: - The --musicip and --itunes arguments is no longer supported in scanner.pl (as far as I can see there is no reason to have them there) - scanner.pl loads the scanning modules through PluginManager but doesn't use the cached manifest data, the reason is that we don't want to write to the cache from scanner.pl since it doesn't load all plugins - The version check in PluginManager is disabled in case there isn't a $::VERSION (it felt better to disable it instead of having $::VERSIOn in both scanner.pl and slimserver.pl) I've verified the patch on Linux but someone else will have to verify that it works on Windows since I have no way to compile the exe binaries. It should be enough to verify that the iTunes integration works on Windows.
Created attachment 3534 [details] Patch for additional scanning hooks This is an additional patch for the features Stuart wanted for the LazySearch plugin. This patch makes four registration functions available for third party plugins: Slim::Schema::registerTrackPostScanner Slim::Schema::registerGenrePostScanner Slim::Schema::registerAlbumPostScanner Slim::Schema::registerContributorPostScanner When ever SqueezeCenter changes anything in these objects by calling Slim::Schema::newTrack or Slim::Schema::updateOrCreate the callbacks registered with these functions will be called. This makes it easy for LazySearch plugin to use to re-calculate the customsearch column when a track, album or artist title is changed.
Created attachment 3535 [details] PostScanner sample plugin This is a sample plugin that uses the functions in the patches and can be used to see some sample code and to verify the patches. The sample plugin does some stupid tests to verify that it works like: - Reverse the namesort/titlesort fiels on all albums, artists, tracks and genres - Add a "Artist " prefix on all contributors which has the ARTIST or ALBUMARTIST role.
That does sound excellent, and just what I wanted. I'll take a look at that latest patch, but I don't think I'll be able to get to it for a few days, unfortunately. I'll report back here.
I've spent a while trying out the patch, and in particular the registerTrackPostScanner etc hooks that it introduces. It seems to work very well indeed. It significantly simplifies my plugin with regards to getting the timing of the scanning correct and adding my search information, and I suspect it's less prone to going wrong as the database can't be changed without these hooks being triggered. With the introduction of these hooks I was able to shorten the plugin by about 300 lines. I think this would be a welcome addition that I'd appreciate being merged.
hey erland, i take it the main scanner is hardcoded, and what you are asking for here is basically the ability to add "after the main scanner" scanner plugins? is that right? i assume someone could have as many of these as they wished? one i would suggest to you, is an "artwork only scan" that would let someone skip redoing the hardcoded scanner part, but let them run JUST the plugins you're talking about here, (assuming someone would create an artwork only scan plugin) ultimately, as i said in that thread, i think making the entire scanning process modular via plugin is the best choice, as well as having separate plugins for audio vs artwork files, and also clearly delineating the options between ML options (ie AFTER scan options), and scanner options (ie PRE scan options). too much of all that is muddled into one catchall pond now, without good reason. but i think you're onto something. hopefully we can bring these ideas to bear pre 7.3
Chris (or someone else at Logitech) is there a possibility to re-target this to 7.3, this really is something that should be thought about when designing the new schema and doing the changes in the scanning process which looks to be planned for 7.3. It contains the necessary patches and has been verified to work both together with Custom Scan plugin, LazySearch plugin and the attached sample plugin. So after someone with deeper knowledge has verified the patches it should just be a matter of committing it to svn. This enhancement would make it so much easier to implement plugin that want to provide additional data than the information normally handled by the standard SqueezeCenter scanning.
I'll cc Brandon as well since he's wrangling the new schema work.
Brandon can keep this in mind and we'll revisit this as the new schema comes together.
Will look at this for 7.4, it's a good time to add this as the scanner is being reworked.
Moving all SQLite-related bugs to 8.0.
Can we get the target for this changed to 7.6.0? If the auto-scanning changes to 7.6 cause problems for plugins that need to run following a scan then maybe this should be considered a bug rather than an enhancement.
Yes I will get to this for 7.6.
SQLite bugs need to go back to 7.6 target.
== Auto-comment from SVN commit #31746 to the slim repo by agrundman == == http://svn.slimdevices.com/slim?view=revision&revision=31746 == Bug 6023, allow plugins to define importers that are run in the external scanner. Move iTunes and MusicIP out of being hardcoded into the scanner. Based on a patch by erland
== Auto-comment from SVN commit #31747 to the slim repo by agrundman == == http://svn.slimdevices.com/slim?view=revision&revision=31747 == Bug 6023, API for plugins to register hooks for different scanner events, see documentation in the Slim::Utils::Scanner::API module. Converted MusicIP to use new and changed hooks
The previous 2 checkins are a start towards implementing this enhancement. Can everyone with a scanner-based plugin please try adapting your plugins and let me know what you think? I intentionally left out hooks for onNewContributor, onNewAlbum, and so on, as I thought this would complicate things a lot in the code. The scanner operates at the track level and so the plugin hooks are only for tracks.
(In reply to comment #19) > The previous 2 checkins are a start towards implementing this enhancement. Can > everyone with a scanner-based plugin please try adapting your plugins and let > me know what you think? > > I intentionally left out hooks for onNewContributor, onNewAlbum, and so on, as > I thought this would complicate things a lot in the code. The scanner operates > at the track level and so the plugin hooks are only for tracks. Thanks for getting going on this Andy, I appreciate it. Looking at the API and modified standard plugins it looks good to me. Presumably, if you pass "want_object => 1" to the callback register functions then the callback has a chance to modify the object before it's committed to the database? That would be important for my LazySearch plugin. It would be a problem for me, though, if there aren't equivalent hooks for Contributor, Genre and Album since they also need to have their 'CUSTOMSEARCH' attribute modified when those objects are created or modified in the database - without similar hooks for these the LazySearch plugin wouldn't be able to take full advantage of this. Perhaps I'm missing a trick, but I think I would need those similar hooks in order to be able to significantly simplify the LazySearch plugin.
(In reply to comment #20) > > It would be a problem for me, though, if there aren't equivalent hooks for > Contributor, Genre and Album since they also need to have their 'CUSTOMSEARCH' > attribute modified when those objects are created or modified in the database - > without similar hooks for these the LazySearch plugin wouldn't be able to take > full advantage of this. Perhaps I'm missing a trick, but I think I would need > those similar hooks in order to be able to significantly simplify the > LazySearch plugin. I've only done some brief tests yet, will test it further during the next week or so. Stuart, as I look at it you have two choices: 1. Integrate with the track hooks, and when a track is added/updated you also go through all it's relations and ensure that their 'CUSTOMSEARCH' attribute is updated. This will work good in incremental scanning since you only add a few tracks, it will not work as good in the full rescan since it might result in a lot of duplicate updates because the same artist/album is reused in a lot of tracks. Go through the relations is pretty easy, I can post a sample plugin during the weekend that handles this if it helps you. 2. In the full rescan, handle it the same way as you do today. The difference is that with the new API you will be able to integrate it as a phase in the scanning process, so it shows up in the scanning process in the web interface. It also executes in the scanner process instead of the standard SBS process, so the audio disruptions should be a lot less than the previous solution. You don't have to setup a "rescan only" subscription, instead you just register your module as an Importer. Since you specify different modules in install.xml for the SBS process "module" (point 1 and incremental scanning) and for the scanner process "importmodule" (point 2 and full rescan), you will also be able to ensure that the track hooks only is used for the incremental scanning.
(In reply to comment #21) > (In reply to comment #20) > > > It would be a problem for me, though, if there aren't equivalent hooks for > > Contributor, Genre and Album since they also need to have their 'CUSTOMSEARCH' > > attribute modified when those objects are created or modified in the database - > > without similar hooks for these the LazySearch plugin wouldn't be able to take > > full advantage of this. Perhaps I'm missing a trick, but I think I would need > > those similar hooks in order to be able to significantly simplify the > > LazySearch plugin. > > > I've only done some brief tests yet, will test it further during the next week > or so. > > Stuart, as I look at it you have two choices: > 1. > Integrate with the track hooks, and when a track is added/updated you also go > through all it's relations and ensure that their 'CUSTOMSEARCH' attribute is > updated. This will work good in incremental scanning since you only add a few > tracks, it will not work as good in the full rescan since it might result in a > lot of duplicate updates because the same artist/album is reused in a lot of > tracks. Go through the relations is pretty easy, I can post a sample plugin > during the weekend that handles this if it helps you. > > 2. > In the full rescan, handle it the same way as you do today. The difference is > that with the new API you will be able to integrate it as a phase in the > scanning process, so it shows up in the scanning process in the web interface. > It also executes in the scanner process instead of the standard SBS process, so > the audio disruptions should be a lot less than the previous solution. You > don't have to setup a "rescan only" subscription, instead you just register > your module as an Importer. > > Since you specify different modules in install.xml for the SBS process "module" > (point 1 and incremental scanning) and for the scanner process "importmodule" > (point 2 and full rescan), you will also be able to ensure that the track hooks > only is used for the incremental scanning. Ah, thanks for the clarification - I'd not thought to add it as an importer for the full rescan. I think I'll try the following approach, then: 1. Register for track 'changes', but not 'adds'. This allows me to update the track's CUSTOMSEARCH and any associated contributors, albums and genres in a reasonably efficient way. I think I can work out the object navigation as I already do quite a bit of that in the plugin, but thanks for the offer. 2. Register a new importer and use the current process to go through all tracks, genres, contributors and albums. This will still simplify things since I won't have to split up the process into small chunks and fiddle with timers to prevent audio interruptions. Thanks for the pointers, I'll give it a whirl.
I see two options for efficiency: 1) - When you get a callback for each new/changed track, update the track, and get a list of the contributors and album. Update each one, unless they have already been updated (keep a list of ones that have you have updated). 2) - When you get a callback for each new/changed track, you can update that track and also keep a list of unique contributors and albums. - When you get a callback for "end of scan", then update each unique contributor and album. That ensures efficiency of not needing to update the contributors and albums more than once each rescan. The only thing is, does this work with albums that are split/merged due to tidy up phases at the end of the scanner process? It would be easier if this was worked out by the scanner and contributor + album change events raised.
I think I've found a small issue. Currently if I like to perform some post processing at the end of a rescan, I have no way to hook into the scanning process besides subscribing on "rescan done". I can still subscribe to the "rescan done" event and execute my logic after the scanning process but the "rescan done" event is generated always when an incremental scanning is performed, independent if it found any files with updated timestamps or not. For example, "rescan done" seems to be always generated at startup. I can create a workaround in the plugin to simply register the track callbacks and keep a counter of number of changed/added/deleted tracks and subscribe on the "rescan done" event and only execute its logic if something was changed, so it's not critical. However, it would feel better if: - Importers of type "post" would be executed at the end of a rescan also in incremental scanning. They are executed currently if you do a clear and rescan but not in incremental scans. - There was some way to get information if the main SBS scanner actually found any changes. - It feels like the "rescan done" event should only be generated if there actually were any updated files. The advantage of a hook at the end of a rescan is also that it would be possible to show the progress to the user as long as this hook is executed before the call to: Slim::Music::Import->setIsScanning(0); I think the progress bars disappears as soon as setIsScanning(0) is called and this currently happens before "rescan done" event. For anyone interested in implementing the new API, I've also uploaded a stupid sample plugin that creates a small mess in your library in the sorting columns and artist names. The intention is just to prove the concept, so you want to run a full rescan with the plugin disabled to restore your library afterwards: http://erland.isaksson.info/arkiv/slimserver/ReverseSorter-1.0.zip
How about I add another hook that is called when in-process rescan is finished, and is passed the number of changes that were made?
(In reply to comment #25) > How about I add another hook that is called when in-process rescan is finished, > and is passed the number of changes that were made? I think that should work.
== Auto-comment from SVN commit #31889 to the slim repo by agrundman == == http://svn.slimdevices.com/slim?view=revision&revision=31889 == Bug 6023, add onFinished hook
(In reply to comment #27) > == Auto-comment from SVN commit #31889 to the slim repo by agrundman == > == http://svn.slimdevices.com/slim?view=revision&revision=31889 == > > Bug 6023, add onFinished hook I'm afraid you have named the variable incorrectly so it currently doesn't compile. Should be: my @Finished; Instead of: my @Finish; I've patched it in my local installation and will continue testing, I just wanted to let you know that it currently doesn't work at all due to the above error.
== Auto-comment from SVN commit #31894 to the slim repo by agrundman == == http://svn.slimdevices.com/slim?view=revision&revision=31894 == Bug 6023, fix typo
Andy, I've taken a look at the latest change regarding onFinished handler in a simple test plugin and it works as I like. So if you like to mark this enhancement as resolved that's ok from my perspective. I'll try to incorporate the changes in my Custom Scan plugin but it will take some time since it's a bit of work, so if you like to get rid of the bug from your open list you can close it now and I can reopen it later if I find some additional issues when implementing it in a real plugin.
Thanks!