Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

UPnP track file state on remote server when supported #2547

Merged
merged 4 commits into from

5 participants

@alcoheca
Collaborator

This adds support for saving resume points and updating watched status on a server rather than locally, which means users can watch a file on one machine, stop it then pick up on another. Changes on either server or client are mirrored instantly on the other. Lots of users have complained about the lack of this feature, as changes currently only propagate from server -> client. This is currently video items only, though I'll add another PR which updates playcounts for music tracks soon.

Most of this should hopefully be uncontroversial, though I had to push a specific announce message in CSaveFileStateJob when a file is modified on the server, as currently AddBookMarksToFile/ClearBookmarks do not announce. Adding an announcement to those is probably preferable but much more code is needed to determine the content of the file (as they operates on all files not just library items).

xbmc/network/upnp/UPnP.h
@@ -24,6 +24,8 @@
#pragma once
#include "utils/StdString.h"
+#include "FileItem.h"
+#include "video/Bookmark.h"
@jmarshallnz Owner

By the looks these can be forwarded?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmarshallnz jmarshallnz commented on the diff
xbmc/network/upnp/UPnPServer.cpp
((85 lines not shown))
+ && playCount.Compare(current_vals["playCount"]) != 0) {
+
+ NPT_UInt32 count;
+ NPT_CHECK_LABEL(playCount.ToInteger32(count), args);
+ db.SetPlayCount(updated, count);
+ updatelisting = true;
+ }
+
+ // we must load the changed settings before propagating to local UI
+ if (updatelisting) {
+ db.LoadVideoInfo(file_path, tag);
+ updated.SetFromVideoInfoTag(tag);
+ }
+
+ } else if (updated.IsMusicDb()) {
+
@jmarshallnz Owner

// TODO: implement me ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmarshallnz
Owner

Looks OK from a quick look, though @elupus is probably the better person to review the upnp stuff.

xbmc/network/upnp/UPnP.cpp
@@ -346,6 +422,36 @@ CUPnPServer* CUPnP::GetServer()
}
/*----------------------------------------------------------------------
+| CUPnP::MarkWatched
++---------------------------------------------------------------------*/
+bool
+CUPnP::MarkWatched(const CFileItem& item, const bool watched)
+{
+ if (upnp && upnp->m_MediaBrowser) {
+ // dynamic_cast is safe here, avoids polluting CUPnP.h header file
+ CMediaBrowser* browser = dynamic_cast<CMediaBrowser*>(upnp->m_MediaBrowser);
+ assert(browser);
@elupus Collaborator
elupus added a note

assert and dynamic cast make no sense imho.. Just log if it's a possibility of not being a CMediaBrowser or allow the crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@elupus
Collaborator

Looks very nice. Fix the above things then ok for this window imho.

@alcoheca
Collaborator

Ditched the assert, allowing the crash

@alcoheca alcoheca merged commit 3095950 into xbmc:master
@mikkle

This breaks updating the videodb for items played through the video library. After this change, "original_listitem_url" is always used instead of m_item.GetPath() or ...->m_strFileNameAndPath.
Items played from the videdb actually contain that property. it looks something like "videodb://tvshows/genres/13/364/-2/9519?genreid=13&season=-2&tvshowid=364"

Collaborator

Just wanted to comment the same here. I was debugging why after I play a movie through recently added items the resume bookmark doesn't stick: my path is like "videodb://recentlyaddedmovies/284". My guess is the IsPlugin check needs to be put back here plus whatever you need for UPnP...

Collaborator

Ah crap - sorry I thought original_listitem_url was only used for UPnP & plugins, will fix this afternoon

Collaborator

I just tested and everything now seems ok, but please let me know if there's still problems after e71c8d0

Collaborator

I confirm. Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.