[artwork] Scrapers set separate artwork for a movie and the collection

[rmrector]

Description

Scrapers still provide movie set/collection info in the same bundle of info as the movie itself, but instead of being assigned the same artwork as the first scanned movie, scrapers can add separate artwork as "set.poster", "set.fanart", and "set.[arttype]" for any art type and artwork will be split up to the correct media items before assigning to the database. This only affects adding new items with the SetDetailsForMovie method, JSON-RPC and GUI changes use UpdateDetailsForMovie and SetArtForItem.

Movies will get "poster", "banner", and so on like usual, and "set." will be stripped from set artwork and applied to movie collections as "poster", "fanart", and "[arttype]". There are no hardcoded art types so all artwork is treated the same. If no set artwork is available, artwork from the movie is added as before rather than leaving it empty.

The second commit adjusts the "Choose art" dialog to include the collection artwork, and still allows selection of artwork from all movies.

This still leaves collection artwork without a home in the file system, but I like what Dave has done recently with the "Artist information folder" in the music library and think it would fit us well here, rather than in each movie folder.

Edit: I tried to write a clearer description of what happens.

Motivation and Context

Forum thread. This is an alternative to #13371, and avoids duplicating collection artwork to movies in the database, and vice versa. This also works for other artwork like "clearlogo" and "banner", and all artwork is stored and referenced with the same artwork names used in the rest of the video library. Finally, the "set." prefix matches that used in other areas of Kodi ("artist.fanart", "tvshow.poster"), and as artwork can be freely named, avoids colliding with movie art types that just happen to start with "set" (some crazy skinner will one day find a use for artwork named setter).

How Has This Been Tested?

Added some new movies to my test library, some with an associated collection, some without. Movies ended up with their own artwork, collections got their own artwork, and neither ended up with the other. I've also patched my main HTPC for live testing and a handful of new movies worked as described above.

Screenshots (if appropriate):

Types of change

• Bug fix (non-breaking change which fixes an issue)
• Improvement (non-breaking change which improves existing functionality)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the Code guidelines of this project
• My change requires a change to the documentation, either Doxygen or wiki
• I have updated the documentation accordingly
• I have read the CONTRIBUTING document
• I have added tests to cover my change
• All new and existing tests passed

[MilhouseVH]

Just so that I understand, aren't these prefixes somewhat "meta" in that the artwork type for a collection in the database is only ever fanart, poster, clearlogo, banner, etc., but if you were to list (query) the collection artwork within the context of a movie (not currently possible in JSON, btw) then you'd want to include the set. prefix to avoid confusion within the "hierarchy" of artwork items?

Is that what this PR does, automatically adding the set. prefix when required, or is it actually creating artwork types in the database with the prefix, ie. set.fanart rather than fanart (which would be wrong IMHO as collection artwork works/caches/displays correctly without a static set. prefix)?

Take tvshows, which as you know use/create artwork types named fanart, poster etc., but when the tvshow artwork is listed within the context of the season (which has it's own fanart, poster etc.) then the artwork for the tvshow parent is automatically prefixed with tvshow.. And the fanart for the season is shown as season.fanart when querying episodes within that season (as the episode may have it's own fanart). Psuedo-types tvshow.fanart and season.fanart etc. are not actual artwork types, as such.

Thanks in advance for the clarification!

[MartijnKaijser]

It seems create set.fanart and not fanart on the set item in the database itself. so yes this is wrong

[rmrector]

@MilhouseVH #13564 does what you've described, automatically adding the "meta"/psuedo artwork for skin/JSON lookups from the database. This PR is basically the opposite, allowing scrapers/nfo to provide both "poster" and the meta "set.poster" when scanning in a movie (there isn't a separate process for scraping collections), and then separating those to the proper database records, peeling off the "set." prefix for collection artwork. CVideoDatabase::SetArtForItem already makes sure that any art type with a "." isn't actually saved to the database, which prevents this artwork from being assigned to the movie.

The changes to DialogVideoInfo is to work with the "set.poster" type artwork that is still attached to movies in the "available art" XML structure thing stored in the database.

[MilhouseVH]

Thanks.

I'm not sure why you'd want to physically attach the set artwork to the movie when you can get this information with a query (the movie is a member of the set, so it's basically a database join) and then use the set. psuedo type prefix to avoid any confusion with the "parent" (which in this case would be the movie as you want to list the set artwork for the corresponding movie). Similarly if you wanted to list all movie artwork within each set then you'd use a movie. psuedo prefix for the movie artwork. At least I think that's how it should be done, and might have been how #13564 is doing it.

Joining the data in the database would avoid duplicating data, and would avoid the issue of a set having two movies and each movie being assigned a different set.poster.

So yeah... this is probably wrong, and the better solution would be to extend/enhance the existing mechanism which more or less already supports this but may be lacking some of the queries/joins you require to surface the relevant information and relationships. Creating the movie/set relationship by duplicating artwork data within each movie is not the right solution IMHO.

[rmrector]

You've got it backwards, this PR does not attach set artwork to movies but specifically separates them to the correct database records.

The current behavior right now in Leia, after a recent change to the scraper, is duplicating data by assigning "set" and "setfanart" to every movie in the database. This PR removes that duplication by assigning only "set." prefixed artwork to the database for the collection (without the prefix), and only non prefixed artwork to movies (as mentioned, Kodi will not assign prefixed artwork to the art table at all).

[MilhouseVH]

is duplicating data by assigning "set" and "setfanart" to every movie in the database.

Eugh. OK, thanks, that does sound like a massive clanger.

Just so I have this straight (sorry, coming to this a bit late!) the MovieSet artwork is being set via the Movie, correct? Ie. when a Movie is scraped which has set.poster artwork the set. prefix is detected in VideoDatabase and instead of assigning set.poster on the Movie (which would fail silently due to the .) the poster - without prefix - it is instead assigned to the associated MovieSet (assuming there is one)?

Assuming this is the case, and although I'm not really qualified to say if this is the best approach, it does seem to be doing "magical stuff behind the scenes" based on a compound artwork type rather than having the various sources (scraper, JSON, GUI etc.) directly calling the Movie or MovieSet setter-methods appropriate for the artwork media_type (movie, set) and type (poster, fanart etc.). Would it not be better/cleaner if the source (ie. scraper etc.) could be made more aware of the MovieSet/Movie hierarchy rather than relying on a compound artwork type?

Further to this argument, we don't for instance allow the tvshow poster to be changed by setting tvhow.poster on the season (I know, it's dropped on the floor because of the .) which is analogous to what is (or may be) happening here, so this new behaviour (if I've understood it correctly) would be inconsistent with everything else.

Anyway this is just my opinion/thoughts - my experience with scrapers is virtually non-existent so don't take my comments negatively!

[rmrector]

Yeah, now we're on the same page. I do agree that it's a bit magical, but it's only the scraper/nfo interface (adding new items with SetDetailsForMovie), the GUI and JSON interfaces do set artwork directly to media items without much magic (updating existing items with SetArtForItem and UpdateDetailsForMovie) and will still ignore "set.poster".

For now I think we still need to keep a light touch on the scraper interface, see how the Python scrapers work out over the next year before bigger changes are made.

[rmrector]

Anyone care to review this for merging?

Movie sets (and to be clear movies themselves) are not assigned "set.fanart", because a fundamental behavior of Kodi's artwork handling is that it does not assign artwork with a "." in it, as that artwork belongs to a parent item. See the code.

This does still bundle set artwork with the movie artwork in the "available artwork" XML structure, but that is about worthless anyhow. It goes out of date quickly and can't be changed or refreshed without clobbering everything else about the media item. I wouldn't mind breaking that out to another table similar to the 'art' table in the future, so that it is just as flexible as assigned artwork.

@notspiff what do you think? The existing artwork handling is not broken, it just needs a bit more work to finish it off. Note how effectively Dave's recent work fully supports freely-named artwork in the music library; the video library has the same artwork handling bones and there is much less that is unfinished.

[garbear]

No problem apart from the minors.

[DaveTBlake]

No problem apart from the minors.

Same here. Get them fixed and let's get this merged, it has lurked long enough.

[notspiff]

looks good.