Kodi Community Forum

Full Version: Library updates - test build 0801 results
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
Pages: 1 2
I think I will make a series of posts in a thread as I work through the changes to make things more digestible.

Change info provider (albums)

I open up the album node in the library. The context menu has "change info provider"

1. Selecting this launches a second context menu. The second option is "Set for all albums shown" I don't understand what this means? Is this "all the albums in this view"? In my test source, it changed every album (idInfoSettings set to 1 -- was 0 -- and idSetting 1 strScraperPath set to metadata.local).

The third option is "set default information provider" (in Estuary with Arial font the label is truncated BTW). Selecting this brings up a select dialog. This is very nice improvement, as it is analog to the video source edit. Launching addon settings dialog from here works as expected. Same with "choose info provider".

Changing info provider launches 2 confirm dialogs, first "are you sure" then "refresh all". AFAICT, the effect of this is the same as changes the settings/media/music default provider setting. Changing it in settings does not launch these confirm dialogs.

2. Maybe deprecate the settings default info provider setting (my vote) or consider launching these same 2 confirm dialogs on changing info provider in settings.

scott s.
.
Good to have someone work through this, thanks Scott.
[My response edited for clarity by light of day]

1) What "Set for all albums shown" means:
"Change info provider" is available from any node with content = "albums" e.g. even a smart playlist showing just albums with year = 1960. This option is to set the scraper addon settings for just those (possibly filtered) albums. I guess the confusion is does it mean on the page, or all pages?

Setting the "Set default info provider" (3rd button) changes the default addon setting, and removes all previously set album settings, not just those shown (possibly filtered) on the node.

As you found, from the albums library node (which is unfiltered) "Set for all albums shown" causes all albums to be given an info setting, so is much the same result as "Set default info provider" for those albums, but does not change the addon settings. Hence a newly added album would be still have addon settings set to the unchanged default values.

a) Is that confusing? Should the "Set for all albums shown" button be hidden when all albums are showing on the node?

And both buttons are different from changing the default provider from the settings menu (in the two separate places - which addon to use, and the settings). That just changes the addon and any defaut settings, but does not change the library. It does not remove previously set album settings, nor offer refresh, but what is offered on the context menu does change the library contents hence the comfirmation dialogs.

So
b) Is that reasonable?

c) How can it best be made clear? As you point out the number of characters visible on the label is limited, so what else could these context menu buttons be called? Or should it be using some other UI approach?


2) We need to keep the settings for those that want the "fetch online info on update" when adding the first music source, as need somewhere to set what scraper addon will be used. Initially when no source the albums nodes will all be empty, so no context menu route available.

d) But maybe the change from settings should also clear and refresh any existsing albums or artists? Although generally settings changes don't get applied to the library retrospectively when they are changed
(2017-08-04, 00:38)DaveBlake Wrote: [ -> ]Good to have someone work through this, thanks Scott.
[My response edited for clarity by light of day]

1) What "Set for all albums shown" means:
"Change info provider" is available from any node with content = "albums" e.g. even a smart playlist showing just albums with year = 1960. This option is to set the scraper addon settings for just those (possibly filtered) albums. I guess the confusion is does it mean on the page, or all pages?

Setting the "Set default info provider" (3rd button) changes the default addon setting, and removes all previously set album settings, not just those shown (possibly filtered) on the node.

As you found, from the albums library node (which is unfiltered) "Set for all albums shown" causes all albums to be given an info setting, so is much the same result as "Set default info provider" for those albums, but does not change the addon settings. Hence a newly added album would be still have addon settings set to the unchanged default values.

a) Is that confusing? Should the "Set for all albums shown" button be hidden when all albums are showing on the node?

I would suggest "Set for all albums/artists in current view" though maybe that's too long. Now that I see the intent, it works very well.

And both buttons are different from changing the default provider from the settings menu (in the two separate places - which addon to use, and the settings). That just changes the addon and any defaut settings, but does not change the library. It does not remove previously set album settings, nor offer refresh, but what is offered on the context menu does change the library contents hence the comfirmation dialogs.

So
b) Is that reasonable?

c) How can it best be made clear? As you point out the number of characters visible on the label is limited, so what else could these context menu buttons be called? Or should it be using some other UI approach?


It is really a skinner issue, but having to format a long label makes a complication for the skinner (and there are unofficial Estuary mods using font with larger point size which compounds the issue. I was able to get a PR approved that adds a label for fontset in use, so a skinner could reformat his/her labels based on font size.
I'm not sure I see exactly what we do now with info provider setings. When are they stored in the addon settings, and when in the library? I was testing with Uni Album Scraper and it didn't seem right, but I'm not sure.


2) We need to keep the settings for those that want the "fetch online info on update" when adding the first music source, as need somewhere to set what scraper addon will be used. Initially when no source the albums nodes will all be empty, so no context menu route available.

d) But maybe the change from settings should also clear and refresh any existsing albums or artists? Although generally settings changes don't get applied to the library retrospectively when they are changed

I added some comments in the quoted text, but need to do some more testing.

scott s.
.
(2017-08-05, 02:52)scott967 Wrote: [ -> ]I'm not sure I see exactly what we do now with info provider settings. When are they stored in the addon settings, and when in the library? I was testing with Uni Album Scraper and it didn't seem right, but I'm not sure.

Old behaviour (v17 and before)
The addon settings are only changed via the System >Addons > My Addons > Information Providers > Albums > Universal Album Scraper menu route. These get used if nothing specific for an album is found in the library.

The "Change Information Provider" context menu lets you change both the scraper used and the settings for an album. This both gets held in the content table, and when you subsequently scrape/re-scrape that album those are the settings used. But there is no route back to using the default.

The "Prefer online info" setting will effect how new scraper results get merged into older scraped results, when disabled if previous scraping fetched year or album type (things that can come from tags, but also can be scraped), then newly scraped values will not overwrite. But data like moods, styles etc. that ony comes from scraping just gets the last scarped value or blank.

I hope that helps, but keep testing Smile
Haven't yet tested with "prefer online info".

After doing some more change info provider for single album, I see now how the saved scraper settings work and so far it is both logical from UI perspective and seems to update correctly in the db.

started some test of multi-album and see that for an album smart playlist, the context menu doesn't offer "change info provider" nor "rescan all". It seems like this may be a logical use of a smart playlist, but maybe it opens a "can of worms" and the multi-album rescan/change provider is best restricted to library nodes. An example of potential use is a smart playlist with multiple genres, compared to having to visit each genre node in turn. I kind get around this in my personal library by using a "top level" genre along with "secondary genres" within the Genre tag, so I can get "all Classical" for example with a single genre filter, but other users might only have what I would call sub-genres defined.

As an aside, could you remind me what the bScrapedMBID is used for?

scott s.
.
(2017-08-07, 23:57)scott967 Wrote: [ -> ]started some test of multi-album and see that for an album smart playlist, the context menu doesn't offer "change info provider" nor "rescan all".
Odd, I see both on albums and artists smart playlists. Could you look a this again, is it something about the playlist rules? Could it be skin related (I'm using Confluence) how content is set? I don't see why this would not work for you but does for me.

As you go on to describe it is a useful and flexible thing to be able to do.

Quote:As an aside, could you remind me what the bScrapedMBID is used for?
It flags if the artist or album mbid came from scraping e.g. a name lookup then taking first match so could be wrong, or from embedded tags in the music files (taken as definitive).

When doing refresh from the info dialog, if bScrapedMBID is true then the lookup by name is done again (rather than use the mbid) to allow mistaken identity to be corrected. The merge processing also takes care to only overwrite mbid if it was empty or scraped.

Thank you for testing and reviewing this in detail, it is appreciated.
(2017-08-08, 07:55)DaveBlake Wrote: [ -> ]
(2017-08-07, 23:57)scott967 Wrote: [ -> ]started some test of multi-album and see that for an album smart playlist, the context menu doesn't offer "change info provider" nor "rescan all".
Odd, I see both on albums and artists smart playlists. Could you look a this again, is it something about the playlist rules? Could it be skin related (I'm using Confluence) how content is set? I don't see why this would not work for you but does for me.

As you go on to describe it is a useful and flexible thing to be able to do.

Argg. I'm an idiot. Playlist was "songs" by mistake. But I fixed the playlist to "albums" and do get the correct context menu, but it doesn't work correctly.

1. see log line numbers
946 I open the playlist trot.xsp
959 Open the context menu on first item in list view. Select "Change info provider"
982 Open second context menu. Select "Set for all albums shown"
990 Settings dialog opens for the current scraper and settings. Select "Choose info provider" (current provider shown in right label)
1003 Select dialog opens with enabled scrapers and local info only. Select a different provider (tested both scraper and local info which is also default) and "OK" button
1030 Confirm dialog "Use this IP for all albums shown here" Select "yes" button
1043 Back to music window. Check MYMusic db unchanged. I expected the idInfoSetting for each album in playlist to change.

I repeated this except selecting "Set for this album". Then repeated selecting the new info provider". This time the confirm dialog was "Do you want to refresh info for this item now". I selected "yes" and the info provider was changed for all the albums in the playlist. Also looking at MyMusic, I was expecting to see the idInfoSetting change from current (4) to (0) since I had selected the default provider -- local info only. But instead the idInfoSetting was unchanged, and instead in infosetting idSetting 4 was changed from metadata.1ting to metadata.local. This doesn't seem right to me?

(2017-08-08, 07:55)DaveBlake Wrote: [ -> ]
Quote:As an aside, could you remind me what the bScrapedMBID is used for?
It flags if the artist or album mbid came from scraping e.g. a name lookup then taking first match so could be wrong, or from embedded tags in the music files (taken as definitive).

When doing refresh from the info dialog, if bScrapedMBID is true then the lookup by name is done again (rather than use the mbid) to allow mistaken identity to be corrected. The merge processing also takes care to only overwrite mbid if it was empty or scraped.

Thank you for testing and reviewing this in detail, it is appreciated.

Thanks. I can try to do some testing of that.

scott s.
.
Been waiting up for you Smile

Actually I need to look more closely at your playlist test results by light of day tomorrow. But one quick piece of feedback, is that for simplicity (?) there is only the facility to set the default srcaper/settings and set all items to use it. So if you change the settings for a single item, even if that is so something that matches the default scraper settings, it is held as a setting in its own right. I think that is the other behaviour you noticed.

EDIT: I can see why the playlist stuff isn't working. Gremlin crept in.
Who needs daylisght when it is full moon!
New test build KodiSetup-20170809-a130a8e139-InfoSettings

The only part changed is the processing of "Set for all artists/albums shown". It should now apply info provider settings correctly when viewing "artists" or "albums" type smart playlists, or if the a filter has been applied to the node (from the "filter" on the sideblade). So have another look at playlists, and filtering Smile
(2017-08-09, 19:49)DaveBlake Wrote: [ -> ]New test build KodiSetup-20170809-a130a8e139-InfoSettings

The only part changed is the processing of "Set for all artists/albums shown". It should now apply info provider settings correctly when viewing "artists" or "albums" type smart playlists, or if the a filter has been applied to the node (from the "filter" on the sideblade). So have another look at playlists, and filtering Smile

OK so I ran tests on a fresh MyMusic67 and two tests (one album and one artist) that failed on the previous build now pass. Also I now get a confirm dialog "Do you want to refresh info" after changing info provider for both album and artist shown in smart playlists.

Specific test case:

1 Set default info provider in settings/media to local info both albums and artists
2. Add new music source and scan (infosettings is empty and both album and artist idInfoSetting is 0)
3. Open music album view
4. On context menu for first album select "change default info provider" and set to "1ting" / "use for all" / "rescan no"
5. Observe infosettings is empty and both album and artist idInfoSettings is 0 (PASS)

6. Open music album smart playlist view.
7. On context menu for first album select "change info provider for albums shown" and set to "local info only" / "set for all shown" / "rescan no"
8. Observe infosettings has new entry 1 for metadata.local and album idInfoSettings is 1 for all albums in the smart playlist (PASS)

9. Open music artist view
10. On context menu for first artist select "change default info provider" and set to "Encyclopaedia Metallica" / "use for all" / "rescan no"
11. Observe infosettings is unchanged (1 entry) and artist idInfoSettings remains 0 (PASS)

12. Open music artist smart playlist view
13. On context menu for first artist select "change info provider for artists shown" and set to "tadb artist scraper" / "set for all shown" / "rescan no"
14. Observe infosettings has new entry 2 for metadata.artists.theaudiodb.com and artist idInfoSettings is 2 for all artists in smart playlist. (PASS)

scott s.
.
I did some MyMusic60 --> MyMusic67 migration testing and it seemed to do OK, though I'm not sure of all the ways MyMusic60 content table can be manipulated in Kodi 17.

scott s.
.
Migration of content table in v17 into infosetting table in v18 entails taking any simple specifc artist or album paths e.g. "musicdb://artists/1765", and converting them. But anything involving genre is ignored, as is a path that covers all artists e.g. "musicdb://artists/"

It is the best I could think of to do, especially given that specifying "content" for a genre was pretty much broken, so setting the scraper for an artist or album was the only thing that people could be using. But I am interested to hear otherwise if you can see something I can't. You have possibly been using this, I'm just looking at it from the outside.
(2017-08-10, 23:39)DaveBlake Wrote: [ -> ]Migration of content table in v17 into infosetting table in v18 entails taking any simple specifc artist or album paths e.g. "musicdb://artists/1765", and converting them. But anything involving genre is ignored, as is a path that covers all artists e.g. "musicdb://artists/"

It is the best I could think of to do, especially given that specifying "content" for a genre was pretty much broken, so setting the scraper for an artist or album was the only thing that people could be using. But I am interested to hear otherwise if you can see something I can't. You have possibly been using this, I'm just looking at it from the outside.

I'm sure that's fine. I doubt anyone knew that the capability even existed, TBH.

I want to do some more single album/artist testing but things are looking good so far.

scott s..
Thanks for all this testing work Scott.

So far I have not found anyone on the team to review/approve the code changes on Git, please feel free to add any comments there. But happy I can report having had some testing other than me.
Did some single item I.P. change (both artist and album). Everything seems to be set correctly. I did notice a couple things:

1. I set "local info only" on single items. It appears for each one a new entry is made in infosettings with strScraperPath of metadata.local. Then I did a "change for all artists shown" and it generated another entry (no overlap between artists shown and individual artists). Is that intended? It seems to be kind of wasteful, but I guess it does save effort of seeing if an infosetting already exists.

2. I got a removed idSetting (after changing an artist's I.P twice). Does the database ever get compacted? (Don't know that much DB practice, maybe that is too costly to reindex?)

scott s.
.
Pages: 1 2