Skip to content

Commit

Permalink
fixed: fire the OnUpdate announce after fetching the artwork - fixes …
Browse files Browse the repository at this point in the history 
…empty recently-added thumbs if the library is updating while XBMC is on the home screen
  • Loading branch information
@mkortstiege
mkortstiege committed Dec 21, 2011
1 parent 4cd482d commit 227d928
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 8 deletions.
8 changes: 0 additions & 8 deletions xbmc/video/VideoDatabase.cpp
View file
Expand Up @@ -1804,8 +1804,6 @@ int CVideoDatabase::SetDetailsForMovie(const CStdString& strFilenameAndPath, con
m_pDS->exec(sql.c_str());
CommitTransaction();

AnnounceUpdate("movie", idMovie);

return idMovie;
}
catch (...)
Expand Down Expand Up @@ -1866,8 +1864,6 @@ int CVideoDatabase::SetDetailsForTvShow(const CStdString& strPath, const CVideoI
m_pDS->exec(sql.c_str());
CommitTransaction();

AnnounceUpdate("tvshow", idTvShow);

return idTvShow;
}
catch (...)
Expand Down Expand Up @@ -1943,8 +1939,6 @@ int CVideoDatabase::SetDetailsForEpisode(const CStdString& strFilenameAndPath, c
m_pDS->exec(sql.c_str());
CommitTransaction();

AnnounceUpdate("episode", idEpisode);

return idEpisode;
}
catch (...)
Expand Down Expand Up @@ -2017,8 +2011,6 @@ int CVideoDatabase::SetDetailsForMusicVideo(const CStdString& strFilenameAndPath
m_pDS->exec(sql.c_str());
CommitTransaction();

AnnounceUpdate("musicvideo", idMVideo);

return idMVideo;
}
catch (...)
Expand Down
5 changes: 5 additions & 0 deletions xbmc/video/VideoInfoScanner.cpp
View file
Expand Up @@ -36,6 +36,7 @@
#include "dialogs/GUIDialogProgress.h"
#include "dialogs/GUIDialogYesNo.h"
#include "dialogs/GUIDialogOK.h"
#include "interfaces/AnnouncementManager.h"
#include "settings/AdvancedSettings.h"
#include "settings/GUISettings.h"
#include "settings/Settings.h"
Expand Down Expand Up @@ -1155,6 +1156,10 @@ namespace VIDEO
FetchActorThumbs(movieDetails.m_cast, parentDir);
if (bApplyToDir)
ApplyThumbToFolder(parentDir, cachedThumb);

CVariant data;
data["id"] = movieDetails.m_iDbId;
ANNOUNCEMENT::CAnnouncementManager::Announce(ANNOUNCEMENT::VideoLibrary, "xbmc", "OnUpdate", data);
}

void CVideoInfoScanner::DownloadImage(const CStdString &url, const CStdString &destination, bool asThumb /*= true */, CGUIDialogProgress *progress /*= NULL */)
Expand Down

5 comments on commit 227d928

@Hitcher
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow that was quick, wish I'd reported it sooner.

Thanks.

@Montellese
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to say I don't like this for three reasons:

  1. CAnnouncementManager is meant for providing 3rd party clients with event information and not for communicating stuff in core (but you didn't introduce that)
  2. The way it is done now, the OnUpdate announcement doesn't provide the "type" property anymore. Because of that JSON-RPC clients will get a notification that the item with the "id" xyz has changed but it could be a movie, a tvshow, an episode, a music video or whatever but there's no way anymore for the announcement receivers to know which one it really was. The "old" AnnounceUpdate (that was removed) provided this information.
  3. JSON-RPC will soon be extended to provide functionality to update meta data of database items and if one client changes something the other clients need to know about those changes as well. But if those changes don't go through the VideoInfoScanner (which would be a rather stupid implementation) the announcement won't be sent anymore.

I (once again) vote for not "abusing" CAnnouncementManager for this kind of XBMC internal update and use a proper GUI_MSG_FOO as it is done with GUI_MSG_UPDATE_ITEM or any of the other GUI messages and revert this so that the JSON-RPC announcements work as they were intended to.

@mkortstiege
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but the way we did it before was just wrong as the announce was triggered too early. I've left out the type intentionally as I thought the AnnounceManager is handling this (https://github.com/xbmc/xbmc/blob/master/xbmc/interfaces/AnnouncementManager.cpp#L85) based on the item.

@Montellese
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When running the video scanner it was triggered too early yes. The AnnouncementManager can only add the type automatically if you provide the CFileItemPtr of the newly added item with the call to Announce() which is not the case the way it is now. If you can do that I won't be against this change (although I still think it's an abuse of CAnnouncementManager but maybe I'm alone on this).

@mkortstiege
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed in f30aef5. For after Eden we have to come up with a solution that fits.

Please sign in to comment.