Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

add resume point to database if available (e.g. in <resume> from an NFO) #895

Merged
merged 1 commit into from Jun 1, 2012

Conversation

Projects
None yet
5 participants
Owner

Montellese commented Apr 22, 2012

This adds a check if there's a valid resume point in a CVideoInfoTag to CVideoDatabase::SetDetailsForMovie and adds the resume point to the database. This allows us to actually make use of the resume point data we write to in an NFO when exporting the video database. Up until now the point has already been read by CVideoInfoTag when reading an NFO but we didn't actually do anything with it. IMO if we export to we should also import it again.

Member

JezzX commented Apr 22, 2012

I'm against exporting and importing resume points it could be months later that you need tip reimport a file or maybe even a different PC and the information would be way way way out of date and your stuck with it unless you do the overwrite old files on export

Member

arnova commented Apr 22, 2012

I'm also against doing this. I belief NFO files are not the place to store info like this. And if you really really must do this, make it at least optional in some way...

Owner

MartijnKaijser commented Apr 22, 2012

this would only be usefull if you are doing a direct reinstall of XBMC and watch to transfer your resume point. This data is much to volitile to store in an xml file for later use. and if it goes in it should indeed go through as.xml with something like
importresumepoints =True

Member

jmarshallnz commented Apr 22, 2012

IMO the goal should be to export the resume point in a full db dump, but not in a single nfo dump.

We can detect this case already as ExportToXML passes in a flag to indicate exporting of file and path info I think?

Similarly, we can likely detect it in ImportFromXML as well.

Owner

Montellese commented May 14, 2012

OK I have changed the implementation to only import the resume point if the advancedsetting "importresumepoint" is set or if the user is running a full library import. While implementing that I noticed that the importing of the playcount is done in the same way but that on a full library import they may be a few extra and unnecessary calls to CVideoDatabase::SetPlayCount so I fixed that as well.

@ghost ghost assigned Montellese May 14, 2012

Member

jmarshallnz commented May 14, 2012

looks ok to me, but why not just use the existing as.xml? Can't see a reason why you'd want one but not the other.

Owner

Montellese commented May 14, 2012

I'm pretty sure you'll find a user who want one but not the other ;-)
That being said I'd be ok with merging the two but we'd need to change the name to something more generic.

Owner

MartijnKaijser commented May 14, 2012

I'm such a user ;)

I do want playcount but don't care about resume points

Member

jmarshallnz commented May 14, 2012

If you don't care, you don't mind if they're imported :p

Owner

MartijnKaijser commented May 14, 2012

hehe don't care in don't want them ;)
however i'm not against this ofcourse if majority does want this merged in one setting (and ease of coding).
I can image users wouldn't want this merged. Up to you ofcourse

Montellese added a commit that referenced this pull request Jun 1, 2012

Merge pull request #895 from Montellese/import_resume
add resume point to database if available (e.g. in <resume> from an NFO)

@Montellese Montellese merged commit 5840e98 into xbmc:master Jun 1, 2012

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