Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Small Refactor of MarkWatched function and execute it inside a separate job #644

Closed
wants to merge 3 commits into from

3 participants

@GabrielL

When marking as watched large directories, xbmc freezes a little. With this patch, the marking is done in a background task.

Also the first commit pushes the MarkWatched method in the base class CMediaWindow, mainly because it is used by music navigation and video navigation, but is implemented inside video navigation as a static method.

@jmarshallnz
Owner

It's video-specific, so I'd prefer it be left in the video window classes, unless there's some real saving in having it in the base class. The job can then go next to it.

@GabrielL

I don't know anything about xbmc code, I just find this bit a little annoying when updating lots of watched statuses. I will check about GUI_MSG_UPDATE, it seems more appropriate.

The Callback is never deleted, it is a mistake.

Yes it is video specific, but the code seemed a bit silly with CGUIWindowMusicNav using it. Perhaps inside CGUIMediaWindow is not the good solution either.

I will send new patches which take into account theses remarks.

Thanks.

@jmarshallnz
Owner

Sounds good - just commit any changes you make to your local master and they'll automatically show up here in the pull req.

I guess the MarkWatched function will be quite small by the time it's job-ised, so it might not matter where you place the function that fires off the job.

Oh, and I didn't say before, but great work - this will be a nice inclusion :)

@thezoggy

would this by any chance also make it so that someone could mark something as watched/unwatched while the library update process is going on? (or as a compromise allow it to be queued?)

@GabrielL

Here it is, like you asked.

GUI_MSG_UPDATE is quite better than the thing I was doing, thanks for the advice.

@thezoggy : there was a check in order to avoid that, I have keep it in CMarkWatchedJob::DoWork(). So it is not possible to mark as watched something when updating.

@jmarshallnz
Owner

Ok, a couple more minor points on the commits - it's looking great.

Next step would be to squash them down as I don't think you need more than the single commit?

@thezoggy

@GabrielL well what about queuing the requests then? if you watch a show while the library update is going and you finish the show.. it doesn't get marked as watch. you'd think that it would mark it when its done..... I understand locking the db when the library update is actually updating/adding new info.. but just when its scanning its locked.

@jmarshallnz
Owner

@thezoggy: Not really related to this, but yes, it could probably be removed.

@GabrielL: You gonna squash/rebase them down?

@GabrielL GabrielL Execute MarkWatched in another thread.
	* MarkWatched is now executed in another thread
	* Harmonisation of the behavior of mark as watched :
	  When marking a video as watched in Music mode, xbmc will not select
	  the next entry, but it was the case for Video mode.
2e75a73
@GabrielL

Done. All squashed.

Like always, comments are welcome

@GabrielL

Hi,

There is some problems with this patch, I need to see a little more about it, It is not ready as it is actually.

@jmarshallnz
Owner

No, we don't want to loop in a job, as you've just chewed up a job slot.

There's no particular problem with scan though - we don't hold a transaction during scan, so should be able to commit to it.

@jmarshallnz
Owner

@GabrielL: What's the problem?

@GabrielL

In fact, the problem is that I just moved out, and I am a little busy right now, but I will rework on this patch when I am settled.

@GabrielL

This feature is now a little more tested, and is working as expected with both db engines.

Last time, there was a problem with a MySQL Database, but it seems to have disappear with other code updates.

@GabrielL GabrielL Fixes on MarkWatched
	* MarkWatatched is tested on both sqlite3
	* MySQL now mark as watch even when there is a scan
bf64ad2
@jmarshallnz
Owner

Please squash down and I'll take another look. For future reference, please don't merge from master - instead, rebase onto master.

@GabrielL

see pull request #866 for the correct patches, properly squashed

@GabrielL GabrielL closed this
@tru tru referenced this pull request from a commit in plexinc/plex-home-theater-public
@tru tru Don't list airplay as a usuable output device.
Fixes #644
3233136
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 21, 2012
  1. @GabrielL

    Execute MarkWatched in another thread.

    GabrielL authored
    	* MarkWatched is now executed in another thread
    	* Harmonisation of the behavior of mark as watched :
    	  When marking a video as watched in Music mode, xbmc will not select
    	  the next entry, but it was the case for Video mode.
Commits on Apr 4, 2012
  1. @GabrielL
  2. @GabrielL

    Fixes on MarkWatched

    GabrielL authored
    	* MarkWatatched is tested on both sqlite3
    	* MySQL now mark as watch even when there is a scan
This page is out of date. Refresh to see the latest.
View
13 xbmc/music/windows/GUIWindowMusicNav.cpp
@@ -643,15 +643,16 @@ bool CGUIWindowMusicNav::OnContextButton(int itemNumber, CONTEXT_BUTTON button)
}
case CONTEXT_BUTTON_MARK_WATCHED:
- CGUIWindowVideoBase::MarkWatched(item,true);
- CUtil::DeleteVideoDatabaseDirectoryCache();
- Update(m_vecItems->GetPath());
- return true;
+ {
+ int newSelection = m_viewControl.GetSelectedItem() + 1;
+ CGUIWindowVideoBase::MarkWatched(item,true);
+ m_viewControl.SetSelectedItem(newSelection);
+
+ return true;
+ }
case CONTEXT_BUTTON_MARK_UNWATCHED:
CGUIWindowVideoBase::MarkWatched(item,false);
- CUtil::DeleteVideoDatabaseDirectoryCache();
- Update(m_vecItems->GetPath());
return true;
case CONTEXT_BUTTON_RENAME:
View
95 xbmc/video/windows/GUIWindowVideoBase.cpp
@@ -62,6 +62,8 @@
#include "utils/log.h"
#include "utils/FileUtils.h"
#include "utils/URIUtils.h"
+#include "utils/Job.h"
+#include "utils/JobManager.h"
#include "GUIUserMessages.h"
#include "addons/Skin.h"
#include "storage/MediaManager.h"
@@ -1285,14 +1287,10 @@ bool CGUIWindowVideoBase::OnContextButton(int itemNumber, CONTEXT_BUTTON button)
MarkWatched(item,true);
m_viewControl.SetSelectedItem(newSelection);
- CUtil::DeleteVideoDatabaseDirectoryCache();
- Update(m_vecItems->GetPath());
return true;
}
case CONTEXT_BUTTON_MARK_UNWATCHED:
MarkWatched(item,false);
- CUtil::DeleteVideoDatabaseDirectoryCache();
- Update(m_vecItems->GetPath());
return true;
case CONTEXT_BUTTON_PLAY_AND_QUEUE:
return OnPlayAndQueueMedia(item);
@@ -1488,53 +1486,76 @@ void CGUIWindowVideoBase::OnDeleteItem(CFileItemPtr item)
CFileUtils::DeleteItem(item);
}
-void CGUIWindowVideoBase::MarkWatched(const CFileItemPtr &item, bool bMark)
+class CMarkWatchedJob : public CJob
{
- if (!g_settings.GetCurrentProfile().canWriteDatabases())
- return;
- // dont allow update while scanning
- CGUIDialogVideoScan* pDialogScan = (CGUIDialogVideoScan*)g_windowManager.GetWindow(WINDOW_DIALOG_VIDEO_SCAN);
- if (pDialogScan && pDialogScan->IsScanning())
+public:
+ CMarkWatchedJob(const CFileItemPtr& item, bool bMark)
+ : m_item(item), m_bMark(bMark)
{
- CGUIDialogOK::ShowAndGetInput(257, 0, 14057, 0);
- return;
}
- CVideoDatabase database;
- if (database.Open())
+ virtual bool DoWork()
{
- CFileItemList items;
- if (item->m_bIsFolder)
- {
- CStdString strPath = item->GetPath();
- CDirectory::GetDirectory(strPath, items);
- }
- else
- items.Add(item);
+ if (!g_settings.GetCurrentProfile().canWriteDatabases())
+ return false;
- for (int i=0;i<items.Size();++i)
+ MarkWatched(m_item, m_bMark);
+
+ CGUIMessage msg(GUI_MSG_NOTIFY_ALL, 0, 0, GUI_MSG_UPDATE);
+ g_windowManager.SendThreadMessage(msg);
+
+ return true;
+ }
+
+ virtual const char *GetType() const { return "markwatched"; };
+
+private:
+ void MarkWatched(const CFileItemPtr &item, bool bMark)
+ {
+ CVideoDatabase database;
+ if (database.Open())
{
- CFileItemPtr pItem=items[i];
- if (pItem->m_bIsFolder)
+ CFileItemList items;
+ if (item->m_bIsFolder)
{
- MarkWatched(pItem, bMark);
- continue;
+ CStdString strPath = item->GetPath();
+ XFILE::CDirectory::GetDirectory(strPath, items);
}
+ else
+ items.Add(item);
- if (pItem->HasVideoInfoTag() &&
- (( bMark && pItem->GetVideoInfoTag()->m_playCount) ||
- (!bMark && !(pItem->GetVideoInfoTag()->m_playCount))))
- continue;
+ for (int i = 0; i < items.Size(); ++i)
+ {
+ CFileItemPtr pItem = items[i];
+ if (pItem->m_bIsFolder)
+ {
+ MarkWatched(pItem, bMark);
+ continue;
+ }
- // Clear resume bookmark
- if (bMark)
- database.ClearBookMarksOfFile(pItem->GetPath(), CBookmark::RESUME);
+ if (pItem->HasVideoInfoTag() &&
+ (( bMark && pItem->GetVideoInfoTag()->m_playCount) ||
+ (!bMark && !(pItem->GetVideoInfoTag()->m_playCount))))
+ continue;
- database.SetPlayCount(*pItem, bMark ? 1 : 0);
+ // Clear resume bookmark
+ if (bMark)
+ database.ClearBookMarksOfFile(pItem->GetPath(), CBookmark::RESUME);
+
+ database.SetPlayCount(*pItem, bMark ? 1 : 0);
+ }
+
+ database.Close();
}
-
- database.Close();
}
+
+ const CFileItemPtr &m_item;
+ bool m_bMark;
+};
+
+void CGUIWindowVideoBase::MarkWatched(const CFileItemPtr &pItem, bool bMark)
+{
+ CJobManager::GetInstance().AddJob(new CMarkWatchedJob(pItem, bMark), 0);
}
//Add change a title's name
Something went wrong with that request. Please try again.