Cleanup video music thumbloaders #1609

10 commits merged into from Oct 15, 2012

3 participants

Team Kodi member
  1. Splits music/video thumbloaders into their own .cpp/.h.
  2. Cleans up some unnecessary members in a couple classes.
  3. Removes unnecessary backwards compatibility for artist art fetching - all music is rescanned to fetch art anyway.
  4. Adds a cache for album art, so that we hit the db once per album only in the music thumb loader.
  5. Adds a cache for tvshow art, so that we hit the db once per episode/season only in the video thumb loader.

Note: Doesn't speed up retrieval of fanart for songs/albums: Reason is, we don't have the artist id here, so need to hit the database anyway for that information. To speed this up, we could consider storing the primary artist id in the song/album tables.

Team Kodi member

@Montellese, @vdrfan I think this contains some of what you were going to do originally for the recently added job. This takes care of it for video items with the exception of the season art, which is fetched in the RA job - dunno if it's any use to simplify this or not.

@night199uk Any thoughts on the primary artist id in the song/album table?

@cptspiff This is almost all fix/speedups, but your call :)

@Montellese Montellese and 1 other commented on an outdated diff Oct 13, 2012
+ m_database->Open();
+ if (pItem->HasVideoInfoTag() && !pItem->GetVideoInfoTag()->HasStreamDetails() &&
+ (pItem->GetVideoInfoTag()->m_type == "movie" || pItem->GetVideoInfoTag()->m_type == "episode" || pItem->GetVideoInfoTag()->m_type == "musicvideo"))
+ {
+ if (m_database->GetStreamDetails(*pItem->GetVideoInfoTag()))
+ pItem->SetInvalid();
+ }
+ // video db items normally have info in the database
+ if (pItem->HasVideoInfoTag() && pItem->GetArt().empty())
+ {
+ FillLibraryArt(*pItem);
+ if (pItem->GetVideoInfoTag()->m_type == "set" ||
Team Kodi member

Something I was wondering last time I looked into this code. Why don't we use a list of items that need additional art here (obviously negated and with &&) instead of a list that doesn't need additional art? Because if we add a new type and it doesn't make sense to fetch any artwork for it (e.g. "tag" right now) we never look into CVideoThumbloader and add it to this list (as you can see "tag" is missing in this list because of that reason). Buf if I add a new media type (e.g. "sport") and I want artwork for it I automatically look into this implementation and see that I have to add it to this list.

Team Kodi member

Sure - will reverse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Team Kodi member

Looks good to me although I still think that "CVideoThumbloader" is not an appropriate name as it also handles stream details ;-)

Jonathan Mar... added some commits Oct 13, 2012
Jonathan Marshall don't fill in the thumb or icon in CApplicationMessenger::MediaPlay -…
… that's not it's job
Jonathan Marshall move video and music thumbloaders to their own .cpp/h files c2f09a9
Jonathan Marshall there is no reason for the recently added job to hold a CVideoThumbLo…
…ader member
Jonathan Marshall there is no reason for the video info dialog to hold CVideoThumbLoade…
…r or CGUIDialogProgress members
Jonathan Marshall cosmetics: cleanup CGUIListItem's art functions a bit 5b157ab
Jonathan Marshall adds AppendArt to CGUIListItem b51a768
Jonathan Marshall drop backward compatibility for artist art - a rescan of music is req…
…uired anyway, which will yield this moot
Jonathan Marshall add cache of album art and show art in the music and video thumbloade…
…rs, to reduce repeated database queries
Jonathan Marshall initialize the music and video thumbloaders in the recently added job…
… to take advantage of database and art caching
Jonathan Marshall use a list of allowed types rather than disallowed types of library i…
…tems to filter out items that don't require further processing.
@ghost ghost merged commit 7e109e4 into xbmc:master Oct 15, 2012
Team Kodi member

@jmarshallnz : This causes a regression breaking fanart/thumbs for non videodb-items. Please see 50b1b72 & 8c1f246

I think a proper fix should be ?

Team Kodi member
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment