Skip to content
This repository

Art fallback fixes, and don't save show/album/season art for seasons/songs/episodes #1760

Merged
8 commits merged into from over 1 year ago

2 participants

jmarshallnz Martijn Kaijser
jmarshallnz
Owner

With the ability for JSON-RPC to set art for items, there's the possibility of art such as "tvshow.poster" to be set at the episode level. We don't want this, as otherwise that particular episode will always have that tvshow poster, even after the user changes the tvshow poster. This will pollute the database with entries that then need to be manually changed one by one via Choose Art.

To fix this, we store all parent art using the . pattern. e.g. fanart for tvshows is tvshow.fanart. This still allows season-specific fanart to be used (they can just specify fanart at the season level) and, indeed, even episode-specific if that's appropriate.

To pull this off, we need the normal generic ListItem.Art(fanart) to drop back to the appropriate art. Thus, we generalise the fallback already available for ListItem.Art(thumb) to allow other fallbacks. It happens at fetch time, as otherwise the art map is populated with these items that can't be eliminated.

Tested with old skins and all fallbacks working. Tested with new skins and all fallbacks working.

@Montellese, @MartijnKaijser the change is subtle, but it will mean that you'll no longer retrieve "fanart" and "thumb" at the episode or season level, unless they're set at that level. You'll instead retrieve "tvshow.fanart". Same with songs/albums - you'll get artist.fanart and probably also album.thumb rather than thumb at the song level.

Jonathan Mar... added some commits
Jonathan Marshall [art] adds ClearArt to CGUIListItem 084d061
Jonathan Marshall [art] use a fallback map for art to generalise the fallback for the '…
…thumb' type, and set outside of CGUIListItem
49a3662
Jonathan Marshall [art] adds a param to AppendArt to specify the prefix, and utilise fo…
…r setting art for children of tvshows, seasons + albums
93a2523
Jonathan Marshall [art] use artist.fanart for artist fanart, and add fallbacks for fana…
…rt->artist.fanart and thumb->album.thumb for albums/songs
bd8a472
Jonathan Marshall [art] adds fallbacks for fanart->tvshow.fanart for episodes/seasons, …
…and container.thumb->(season|tvshow).(poster|banner)
b2d3bcb
Jonathan Marshall [art] don't add art from parent items when setting art on children in…
… the database
215474f
Jonathan Marshall [art] cleanup - no need for convoluted resetting of thumb now that it…
…'s just a fallback
f6ade12
Jonathan Marshall [art] set all art on the trailer when playing from video info 8f052f7
Martijn Kaijser

Tested with tvshows and works like it should.

Deleted user ghost merged commit a6f70e2 into from
Deleted user ghost closed this
jmarshallnz

Yeah - I always say the same thing and never get around to it :p

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

Showing 8 unique commits by 1 author.

Nov 10, 2012
Jonathan Marshall [art] adds ClearArt to CGUIListItem 084d061
Jonathan Marshall [art] use a fallback map for art to generalise the fallback for the '…
…thumb' type, and set outside of CGUIListItem
49a3662
Jonathan Marshall [art] adds a param to AppendArt to specify the prefix, and utilise fo…
…r setting art for children of tvshows, seasons + albums
93a2523
Jonathan Marshall [art] use artist.fanart for artist fanart, and add fallbacks for fana…
…rt->artist.fanart and thumb->album.thumb for albums/songs
bd8a472
Jonathan Marshall [art] adds fallbacks for fanart->tvshow.fanart for episodes/seasons, …
…and container.thumb->(season|tvshow).(poster|banner)
b2d3bcb
Jonathan Marshall [art] don't add art from parent items when setting art on children in…
… the database
215474f
Jonathan Marshall [art] cleanup - no need for convoluted resetting of thumb now that it…
…'s just a fallback
f6ade12
Jonathan Marshall [art] set all art on the trailer when playing from video info 8f052f7
This page is out of date. Refresh to see the latest.
53 xbmc/guilib/GUIListItem.cpp
@@ -119,25 +119,27 @@ void CGUIListItem::SetArt(const std::string &type, const std::string &url)
119 119 }
120 120 }
121 121
122   -void CGUIListItem::SetArt(const ArtMap &art, bool setFallback /* = true */)
  122 +void CGUIListItem::SetArt(const ArtMap &art)
123 123 {
124 124 m_art = art;
125   - // ensure that the fallback "thumb" is available
126   - if (setFallback && m_art.find("thumb") == m_art.end())
127   - {
128   - if (HasArt("poster"))
129   - m_art["thumb"] = m_art["poster"];
130   - else if (HasArt("banner"))
131   - m_art["thumb"] = m_art["banner"];
132   - }
133   -
134 125 SetInvalid();
135 126 }
136 127
137   -void CGUIListItem::AppendArt(const ArtMap &art)
  128 +void CGUIListItem::SetArtFallback(const std::string &from, const std::string &to)
  129 +{
  130 + m_artFallbacks[from] = to;
  131 +}
  132 +
  133 +void CGUIListItem::ClearArt()
  134 +{
  135 + m_art.clear();
  136 + m_artFallbacks.clear();
  137 +}
  138 +
  139 +void CGUIListItem::AppendArt(const ArtMap &art, const std::string &prefix)
138 140 {
139 141 for (ArtMap::const_iterator i = art.begin(); i != art.end(); ++i)
140   - SetArt(i->first, i->second);
  142 + SetArt(prefix.empty() ? i->first : prefix + '.' + i->first, i->second);
141 143 }
142 144
143 145 std::string CGUIListItem::GetArt(const std::string &type) const
@@ -145,6 +147,13 @@ std::string CGUIListItem::GetArt(const std::string &type) const
145 147 ArtMap::const_iterator i = m_art.find(type);
146 148 if (i != m_art.end())
147 149 return i->second;
  150 + i = m_artFallbacks.find(type);
  151 + if (i != m_artFallbacks.end())
  152 + {
  153 + ArtMap::const_iterator j = m_art.find(i->second);
  154 + if (j != m_art.end())
  155 + return j->second;
  156 + }
148 157 return "";
149 158 }
150 159
@@ -155,8 +164,7 @@ const CGUIListItem::ArtMap &CGUIListItem::GetArt() const
155 164
156 165 bool CGUIListItem::HasArt(const std::string &type) const
157 166 {
158   - ArtMap::const_iterator i = m_art.find(type);
159   - return (i != m_art.end() && !i->second.empty());
  167 + return !GetArt(type).empty();
160 168 }
161 169
162 170 void CGUIListItem::SetIconImage(const CStdString& strIcon)
@@ -239,6 +247,7 @@ const CGUIListItem& CGUIListItem::operator =(const CGUIListItem& item)
239 247 m_bIsFolder = item.m_bIsFolder;
240 248 m_mapProperties = item.m_mapProperties;
241 249 m_art = item.m_art;
  250 + m_artFallbacks = item.m_artFallbacks;
242 251 SetInvalid();
243 252 return *this;
244 253 }
@@ -266,6 +275,12 @@ void CGUIListItem::Archive(CArchive &ar)
266 275 ar << i->first;
267 276 ar << i->second;
268 277 }
  278 + ar << (int)m_artFallbacks.size();
  279 + for (ArtMap::const_iterator i = m_artFallbacks.begin(); i != m_artFallbacks.end(); i++)
  280 + {
  281 + ar << i->first;
  282 + ar << i->second;
  283 + }
269 284 }
270 285 else
271 286 {
@@ -298,6 +313,14 @@ void CGUIListItem::Archive(CArchive &ar)
298 313 ar >> value;
299 314 m_art.insert(make_pair(key, value));
300 315 }
  316 + ar >> mapSize;
  317 + for (int i = 0; i < mapSize; i++)
  318 + {
  319 + std::string key, value;
  320 + ar >> key;
  321 + ar >> value;
  322 + m_artFallbacks.insert(make_pair(key, value));
  323 + }
301 324 }
302 325 }
303 326 void CGUIListItem::Serialize(CVariant &value)
@@ -320,7 +343,7 @@ void CGUIListItem::Serialize(CVariant &value)
320 343 void CGUIListItem::FreeIcons()
321 344 {
322 345 FreeMemory();
323   - m_art.clear();
  346 + ClearArt();
324 347 m_strIcon = "";
325 348 SetInvalid();
326 349 }
19 xbmc/guilib/GUIListItem.h
@@ -85,16 +85,28 @@ class CGUIListItem
85 85
86 86 /*! \brief set artwork for an item
87 87 \param art a type:url map for artwork
88   - \param setFallback whether to set the "thumb" fallback, defaults to true.
89 88 \sa GetArt
90 89 */
91   - void SetArt(const ArtMap &art, bool setFallback = true);
  90 + void SetArt(const ArtMap &art);
92 91
93 92 /*! \brief append artwork to an item
94 93 \param art a type:url map for artwork
  94 + \param prefix a prefix for the art, if applicable.
95 95 \sa GetArt
96 96 */
97   - void AppendArt(const ArtMap &art);
  97 + void AppendArt(const ArtMap &art, const std::string &prefix = "");
  98 +
  99 + /*! \brief set a fallback image for art
  100 + \param from the type to fallback from
  101 + \param to the type to fallback to
  102 + \sa SetArt
  103 + */
  104 + void SetArtFallback(const std::string &from, const std::string &to);
  105 +
  106 + /*! \brief clear art on an item
  107 + \sa SetArt
  108 + */
  109 + void ClearArt();
98 110
99 111 /*! \brief Get a particular art type for an item
100 112 \param type type of art to fetch.
@@ -186,6 +198,7 @@ class CGUIListItem
186 198 CStdString m_strLabel; // text of column1
187 199
188 200 ArtMap m_art;
  201 + ArtMap m_artFallbacks;
189 202 };
190 203 #endif
191 204
4 xbmc/music/MusicDatabase.cpp
@@ -5151,6 +5151,10 @@ void CMusicDatabase::SetArtForItem(int mediaId, const string &mediaType, const s
5151 5151 if (NULL == m_pDB.get()) return;
5152 5152 if (NULL == m_pDS.get()) return;
5153 5153
  5154 + // don't set <foo>.<bar> art types - these are derivative types from parent items
  5155 + if (artType.find('.') != string::npos)
  5156 + return;
  5157 +
5154 5158 CStdString sql = PrepareSQL("SELECT art_id FROM art WHERE media_id=%i AND media_type='%s' AND type='%s'", mediaId, mediaType.c_str(), artType.c_str());
5155 5159 m_pDS->query(sql.c_str());
5156 5160 if (!m_pDS->eof())
25 xbmc/music/MusicThumbLoader.cpp
@@ -80,7 +80,10 @@ bool CMusicThumbLoader::LoadItem(CFileItem* pItem)
80 80 {
81 81 string fanart = m_database->GetArtForItem(idArtist, "artist", "fanart");
82 82 if (!fanart.empty())
83   - pItem->SetArt("fanart", fanart);
  83 + {
  84 + pItem->SetArt("artist.fanart", fanart);
  85 + pItem->SetArtFallback("fanart", "artist.fanart");
  86 + }
84 87 }
85 88 m_database->Close();
86 89 }
@@ -119,18 +122,26 @@ bool CMusicThumbLoader::FillLibraryArt(CFileItem &item)
119 122 else if (tag.GetType() == "song")
120 123 { // no art for the song, try the album
121 124 ArtCache::const_iterator i = m_albumArt.find(tag.GetAlbumId());
  125 + if (i == m_albumArt.end())
  126 + {
  127 + m_database->GetArtForItem(tag.GetAlbumId(), "album", artwork);
  128 + i = m_albumArt.insert(make_pair(tag.GetAlbumId(), artwork)).first;
  129 + }
122 130 if (i != m_albumArt.end())
123   - item.SetArt(i->second);
124   - else
125 131 {
126   - if (m_database->GetArtForItem(tag.GetAlbumId(), "album", artwork))
127   - item.SetArt(artwork);
128   - m_albumArt.insert(make_pair(tag.GetAlbumId(), artwork));
  132 + item.AppendArt(i->second, "album");
  133 + for (map<string, string>::const_iterator j = i->second.begin(); j != i->second.end(); ++j)
  134 + item.SetArtFallback(j->first, "album." + j->first);
129 135 }
130 136 }
131 137 if (tag.GetType() == "song" || tag.GetType() == "album")
132 138 { // fanart from the artist
133   - item.SetArt("fanart", m_database->GetArtistArtForItem(tag.GetDatabaseId(), tag.GetType(), "fanart"));
  139 + string fanart = m_database->GetArtistArtForItem(tag.GetDatabaseId(), tag.GetType(), "fanart");
  140 + if (!fanart.empty())
  141 + {
  142 + item.SetArt("artist.fanart", fanart);
  143 + item.SetArtFallback("fanart", "artist.fanart");
  144 + }
134 145 }
135 146 m_database->Close();
136 147 }
4 xbmc/video/VideoDatabase.cpp
@@ -3568,6 +3568,10 @@ void CVideoDatabase::SetArtForItem(int mediaId, const string &mediaType, const s
3568 3568 if (NULL == m_pDB.get()) return;
3569 3569 if (NULL == m_pDS.get()) return;
3570 3570
  3571 + // don't set <foo>.<bar> art types - these are derivative types from parent items
  3572 + if (artType.find('.') != string::npos)
  3573 + return;
  3574 +
3571 3575 CStdString sql = PrepareSQL("SELECT art_id FROM art WHERE media_id=%i AND media_type='%s' AND type='%s'", mediaId, mediaType.c_str(), artType.c_str());
3572 3576 m_pDS->query(sql.c_str());
3573 3577 if (!m_pDS->eof())
2  xbmc/video/VideoInfoScanner.cpp
@@ -1239,7 +1239,7 @@ namespace VIDEO
1239 1239 for (CGUIListItem::ArtMap::const_iterator i = art.begin(); i != art.end(); ++i)
1240 1240 CTextureCache::Get().BackgroundCacheImage(i->second);
1241 1241
1242   - pItem->SetArt(art, false); // don't set fallbacks
  1242 + pItem->SetArt(art);
1243 1243
1244 1244 // parent folder to apply the thumb to and to search for local actor thumbs
1245 1245 CStdString parentDir = GetParentDir(*pItem);
39 xbmc/video/VideoThumbLoader.cpp
@@ -248,7 +248,7 @@ bool CVideoThumbLoader::LoadItem(CFileItem* pItem)
248 248 artwork.insert(make_pair(type, art));
249 249 }
250 250 }
251   - pItem->SetArt(artwork);
  251 + SetArt(*pItem, artwork);
252 252 }
253 253
254 254 // thumbnails are special-cased due to auto-generation
@@ -298,6 +298,18 @@ bool CVideoThumbLoader::LoadItem(CFileItem* pItem)
298 298 return true;
299 299 }
300 300
  301 +void CVideoThumbLoader::SetArt(CFileItem &item, const map<string, string> &artwork)
  302 +{
  303 + item.SetArt(artwork);
  304 + if (artwork.find("thumb") == artwork.end())
  305 + { // set fallback for "thumb"
  306 + if (artwork.find("poster") != artwork.end())
  307 + item.SetArtFallback("thumb", "poster");
  308 + else if (artwork.find("poster") != artwork.end())
  309 + item.SetArtFallback("thumb", "banner");
  310 + }
  311 +}
  312 +
301 313 bool CVideoThumbLoader::FillLibraryArt(CFileItem &item)
302 314 {
303 315 CVideoInfoTag &tag = *item.GetVideoInfoTag();
@@ -306,7 +318,7 @@ bool CVideoThumbLoader::FillLibraryArt(CFileItem &item)
306 318 map<string, string> artwork;
307 319 m_database->Open();
308 320 if (m_database->GetArtForItem(tag.m_iDbId, tag.m_type, artwork))
309   - item.SetArt(artwork);
  321 + SetArt(item, artwork);
310 322 else if (tag.m_type == "artist")
311 323 { // we retrieve music video art from the music database (no backward compat)
312 324 CMusicDatabase database;
@@ -327,23 +339,16 @@ bool CVideoThumbLoader::FillLibraryArt(CFileItem &item)
327 339 if (!item.HasArt("fanart") && tag.m_iIdShow >= 0)
328 340 {
329 341 ArtCache::const_iterator i = m_showArt.find(tag.m_iIdShow);
  342 + if (i == m_showArt.end())
  343 + {
  344 + map<string, string> showArt;
  345 + m_database->GetArtForItem(tag.m_iIdShow, "tvshow", showArt);
  346 + i = m_showArt.insert(make_pair(tag.m_iIdShow, showArt)).first;
  347 + }
330 348 if (i != m_showArt.end())
331   - item.AppendArt(i->second);
332   - else
333 349 {
334   - map<string, string> showArt, cacheArt;
335   - if (m_database->GetArtForItem(tag.m_iIdShow, "tvshow", showArt))
336   - {
337   - for (CGUIListItem::ArtMap::iterator i = showArt.begin(); i != showArt.end(); ++i)
338   - {
339   - if (i->first == "fanart")
340   - cacheArt.insert(*i);
341   - else
342   - cacheArt.insert(make_pair("tvshow." + i->first, i->second));
343   - }
344   - item.AppendArt(cacheArt);
345   - }
346   - m_showArt.insert(make_pair(tag.m_iIdShow, cacheArt));
  350 + item.AppendArt(i->second, "tvshow");
  351 + item.SetArtFallback("fanart", "tvshow.fanart");
347 352 }
348 353 }
349 354 m_database->Close();
2  xbmc/video/VideoThumbLoader.h
@@ -121,6 +121,8 @@ class CVideoThumbLoader : public CThumbLoader, public CJobQueue
121 121 virtual void OnLoaderStart();
122 122 virtual void OnLoaderFinish();
123 123
  124 + void SetArt(CFileItem &item, const std::map<std::string, std::string> &artwork);
  125 +
124 126 IStreamDetailsObserver *m_pStreamDetailsObs;
125 127 CVideoDatabase *m_database;
126 128 typedef std::map<int, std::map<std::string, std::string> > ArtCache;
12 xbmc/video/dialogs/GUIDialogVideoInfo.cpp
@@ -733,15 +733,7 @@ void CGUIDialogVideoInfo::OnGetArt()
733 733 db.Close();
734 734 }
735 735 CUtil::DeleteVideoDatabaseDirectoryCache(); // to get them new thumbs to show
736   - // update the art - we need to call the map version of SetArt to force
737   - // the thumb image to update in the case it's a fallback image
738   - map<string, string> itemArt(m_movieItem->GetArt());
739   - if (currentArt.find("thumb") == currentArt.end())
740   - { // no "thumb" image, so make sure we reset the thumb fallback
741   - itemArt.erase("thumb");
742   - }
743   - itemArt[type] = newThumb;
744   - m_movieItem->SetArt(itemArt);
  736 + m_movieItem->SetArt(type, newThumb);
745 737 if (m_movieItem->HasProperty("set_folder_thumb"))
746 738 { // have a folder thumb to set as well
747 739 VIDEO::CVideoInfoScanner::ApplyThumbToFolder(m_movieItem->GetProperty("set_folder_thumb").asString(), newThumb);
@@ -855,7 +847,7 @@ void CGUIDialogVideoInfo::PlayTrailer()
855 847 *item.GetVideoInfoTag() = *m_movieItem->GetVideoInfoTag();
856 848 item.GetVideoInfoTag()->m_streamDetails.Reset();
857 849 item.GetVideoInfoTag()->m_strTitle.Format("%s (%s)",m_movieItem->GetVideoInfoTag()->m_strTitle.c_str(),g_localizeStrings.Get(20410));
858   - item.SetArt("thumb", m_movieItem->GetArt("thumb"));
  850 + item.SetArt(m_movieItem->GetArt());
859 851 item.GetVideoInfoTag()->m_iDbId = -1;
860 852 item.GetVideoInfoTag()->m_iFileId = -1;
861 853
2  xbmc/video/windows/GUIWindowVideoBase.cpp
@@ -288,7 +288,7 @@ void CGUIWindowVideoBase::OnInfo(CFileItem* pItem, const ADDON::ScraperPtr& scra
288 288 {
289 289 if (item.GetVideoInfoTag()->m_type == "season")
290 290 { // clear out the art - we're really grabbing the info on the show here
291   - item.SetArt(map<string, string>());
  291 + item.ClearArt();
292 292 }
293 293 item.SetPath(item.GetVideoInfoTag()->GetPath());
294 294 }
29 xbmc/video/windows/GUIWindowVideoNav.cpp
@@ -293,18 +293,14 @@ bool CGUIWindowVideoNav::GetDirectory(const CStdString &strDirectory, CFileItemL
293 293 map<string, string> art;
294 294 if (m_database.GetArtForItem(details.m_iDbId, details.m_type, art))
295 295 {
296   - for (CGUIListItem::ArtMap::iterator i = art.begin(); i != art.end(); ++i)
297   - {
298   - if (i->first == "fanart")
299   - items.SetArt(i->first, i->second);
300   - else
301   - items.SetArt("tvshow." + i->first, i->second);
302   - }
  296 + items.AppendArt(art, "tvshow");
  297 + items.SetArtFallback("fanart", "tvshow.fanart");
303 298 if (node == NODE_TYPE_SEASONS)
304   - {
305   - CFileItem showItem;
306   - showItem.SetArt(art);
307   - items.SetArt("thumb", showItem.GetArt("thumb"));
  299 + { // set an art fallback for "thumb"
  300 + if (items.HasArt("tvshow.poster"))
  301 + items.SetArtFallback("thumb", "tvshow.poster");
  302 + else if (items.HasArt("tvshow.banner"))
  303 + items.SetArtFallback("thumb", "tvshow.banner");
308 304 }
309 305 }
310 306
@@ -325,11 +321,12 @@ bool CGUIWindowVideoNav::GetDirectory(const CStdString &strDirectory, CFileItemL
325 321 CGUIListItem::ArtMap seasonArt;
326 322 if (m_database.GetArtForItem(seasonID, "season", seasonArt))
327 323 {
328   - for (CGUIListItem::ArtMap::iterator i = art.begin(); i != art.end(); ++i)
329   - items.SetArt("season." + i->first, i->second);
330   - CFileItem seasonItem;
331   - seasonItem.SetArt(seasonArt);
332   - items.SetArt("thumb", seasonItem.GetArt("thumb"));
  324 + items.AppendArt(art, "season");
  325 + // set an art fallback for "thumb"
  326 + if (items.HasArt("season.poster"))
  327 + items.SetArtFallback("thumb", "season.poster");
  328 + else if (items.HasArt("season.banner"))
  329 + items.SetArtFallback("thumb", "season.banner");
333 330 }
334 331 }
335 332 else

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.