Tvshow multipaths parentid #4977

Merged
merged 27 commits into from Jul 4, 2014

Conversation

Projects
None yet
7 participants
@jmarshallnz
Member

jmarshallnz commented Jul 2, 2014

This replaces #4843 with storing the parentid in the path table.

Later on we can use this for GetScraperByPath() etc, but I'd prefer to get it in and work on that during the next month.

@Montellese any particular opposition to merging as-is?

@jmarshallnz

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Jul 2, 2014

Member

jenkins build this please

Member

jmarshallnz commented Jul 2, 2014

jenkins build this please

@Montellese

This comment has been minimized.

Show comment Hide comment
@Montellese

Montellese Jul 3, 2014

Member

Not that I remember no.

Member

Montellese commented Jul 3, 2014

Not that I remember no.

@Montellese Montellese added this to the Helix 14.0-alpha2 milestone Jul 3, 2014

Jonathan Marshall and others added some commits Mar 30, 2014

Jonathan Marshall
[videodb] search for existing tvshows that match rather than adding a…
… duplicate when adding the same show with a different path
Jonathan Marshall
[videodb] if we have multiple links from tvshow to path, we need to b…
…e careful with the grouping in the tvshowview.
Jonathan Marshall
[videos] when showing the info dialog on an item from the videodb, ut…
…ilise the database id rather than searching by path
Jonathan Marshall
[videos] match multipath folders to items matching the first path fro…
…m the videodb when fetching db info from files view
Jonathan Marshall
[videodb] don't remove streamdetails or bookmarks when removing movie…
…s/episodes/musicvideos - they're deleted when the file entry is removed
@jmarshallnz

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Jul 3, 2014

Member

Thanks. jenkins build this please

Member

jmarshallnz commented Jul 3, 2014

Thanks. jenkins build this please

jmarshallnz added a commit that referenced this pull request Jul 4, 2014

@jmarshallnz jmarshallnz merged commit eade965 into xbmc:master Jul 4, 2014

1 check passed

default Merged build #975 succeeded in 1 hr 6 min
Details

@jmarshallnz jmarshallnz deleted the jmarshallnz:tvshow_multipaths_parentid branch Jul 4, 2014

@popcornmix

This comment has been minimized.

Show comment Hide comment
@popcornmix

popcornmix Jul 4, 2014

Member

The database update fails with an exception leaving xbmc video database unusable:

ERROR: SQL: Undefined MySQL error: Code (1093)
                                            Query: DELETE FROM seasons WHERE idSeason NOT IN (SELECT min(idSeason) FROM seasons GROUP BY idShow,season)

http://paste.ubuntu.com/7749159/

The database update fails with an exception leaving xbmc video database unusable:

ERROR: SQL: Undefined MySQL error: Code (1093)
                                            Query: DELETE FROM seasons WHERE idSeason NOT IN (SELECT min(idSeason) FROM seasons GROUP BY idShow,season)

http://paste.ubuntu.com/7749159/

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Jul 5, 2014

Member

Thanks for the heads up:

#4985

Member

jmarshallnz replied Jul 5, 2014

Thanks for the heads up:

#4985

+ }
+ }
+ // cleanup duplicate seasons
+ m_pDS->exec("DELETE FROM seasons WHERE idSeason NOT IN (SELECT min(idSeason) FROM seasons GROUP BY idShow,season)");

This comment has been minimized.

Show comment Hide comment
@MilhouseVH

MilhouseVH Jul 5, 2014

Contributor

The above SQL is throwing error 1093 during the db upgrade.

According to MySQL Workbench, error 1093 is:

Error Code: 1093. You can't specify target table 'seasons' for update in FROM clause

Log extract:

05:45:06  19.740480 T:3059241504   ERROR: Unable to open database: MyVideos85 [1049](Unknown database 'MyVideos85')
05:45:06  19.746349 T:3059241504   ERROR: Unable to open database: MyVideos84 [1049](Unknown database 'MyVideos84')
05:45:06  19.752527 T:3059241504   ERROR: Unable to open database: MyVideos83 [1049](Unknown database 'MyVideos83')
05:45:06  19.759798 T:3059241504   ERROR: Unable to open database: MyVideos82 [1049](Unknown database 'MyVideos82')
05:45:06  19.765892 T:3059241504   ERROR: Unable to open database: MyVideos81 [1049](Unknown database 'MyVideos81')
05:45:06  19.772024 T:3059241504   ERROR: Unable to open database: MyVideos80 [1049](Unknown database 'MyVideos80')
05:45:06  19.783268 T:3059241504  NOTICE: Old database found - updating from version 79 to 85
05:45:17  30.576199 T:3059241504  NOTICE: Attempting to update the database MyVideos85 from version 79 to 85
05:45:27  40.041969 T:3059241504   ERROR: SQL: Undefined MySQL error: Code (1093)
                                            Query: DELETE FROM seasons WHERE idSeason NOT IN (SELECT min(idSeason) FROM seasons GROUP BY idShow,season)
05:45:27  40.049629 T:3059241504   ERROR: Exception updating database MyVideos85 from version 79 to 85
05:45:27  40.049843 T:3059241504   ERROR: Error updating database MyVideos85 from version 79 to 85
05:45:27  40.075493 T:3059241504  NOTICE: Old database found - updating from version 78 to 85
05:45:27  40.080177 T:3059241504   ERROR: SQL: Can't create database for copy: 'MyVideos78' (1007)
05:45:27  40.080475 T:3059241504   ERROR: Unable to copy old database MyVideos78 to new version MyVideos85
@MilhouseVH

MilhouseVH Jul 5, 2014

Contributor

The above SQL is throwing error 1093 during the db upgrade.

According to MySQL Workbench, error 1093 is:

Error Code: 1093. You can't specify target table 'seasons' for update in FROM clause

Log extract:

05:45:06  19.740480 T:3059241504   ERROR: Unable to open database: MyVideos85 [1049](Unknown database 'MyVideos85')
05:45:06  19.746349 T:3059241504   ERROR: Unable to open database: MyVideos84 [1049](Unknown database 'MyVideos84')
05:45:06  19.752527 T:3059241504   ERROR: Unable to open database: MyVideos83 [1049](Unknown database 'MyVideos83')
05:45:06  19.759798 T:3059241504   ERROR: Unable to open database: MyVideos82 [1049](Unknown database 'MyVideos82')
05:45:06  19.765892 T:3059241504   ERROR: Unable to open database: MyVideos81 [1049](Unknown database 'MyVideos81')
05:45:06  19.772024 T:3059241504   ERROR: Unable to open database: MyVideos80 [1049](Unknown database 'MyVideos80')
05:45:06  19.783268 T:3059241504  NOTICE: Old database found - updating from version 79 to 85
05:45:17  30.576199 T:3059241504  NOTICE: Attempting to update the database MyVideos85 from version 79 to 85
05:45:27  40.041969 T:3059241504   ERROR: SQL: Undefined MySQL error: Code (1093)
                                            Query: DELETE FROM seasons WHERE idSeason NOT IN (SELECT min(idSeason) FROM seasons GROUP BY idShow,season)
05:45:27  40.049629 T:3059241504   ERROR: Exception updating database MyVideos85 from version 79 to 85
05:45:27  40.049843 T:3059241504   ERROR: Error updating database MyVideos85 from version 79 to 85
05:45:27  40.075493 T:3059241504  NOTICE: Old database found - updating from version 78 to 85
05:45:27  40.080177 T:3059241504   ERROR: SQL: Can't create database for copy: 'MyVideos78' (1007)
05:45:27  40.080475 T:3059241504   ERROR: Unable to copy old database MyVideos78 to new version MyVideos85

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Jul 5, 2014

Member
@MilhouseVH

This comment has been minimized.

Show comment Hide comment
@MilhouseVH

MilhouseVH Jul 10, 2014

Contributor

Not sure if this problem is related to this commit or not, happy to open a new Trac issue if not.

I've just added a new tv show to a build based on git master (af67986) and a rather strange thing has happened.

The new TV show (Extant) has only one episode, and so far there is no season01-poster.jpg for this show.

The scrape went OK, no errors, however a season poster has been assigned from a completely different show (CSI), and using an invalid path.

The details are here: http://pastebin.com/raw.php?i=WBvX6av4

Note that the path used for the Extant season 1 poster - which, remember, doesn't exist - is not actually valid. A valid path for the poster used (CSI season 1 poster) would be nfs://192.168.0.3/mnt/share/media/Video/TVShows/CSI Crime Scene Investigation/season01-poster.jpg, as I have some tvshows in Video, and other tvshows in Video-Private (the former I rsync to a remote server, the latter I don't).

I also scraped this same show (and files) on a Gotham (OpenELEC 4.0.6) machine and it scraped correctly, with no poster being assigned to Season 1.

MySQL is being used for both machines (MyVideos86 for Helix/master, and MyVideos78 for Gotham).

Contributor

MilhouseVH commented Jul 10, 2014

Not sure if this problem is related to this commit or not, happy to open a new Trac issue if not.

I've just added a new tv show to a build based on git master (af67986) and a rather strange thing has happened.

The new TV show (Extant) has only one episode, and so far there is no season01-poster.jpg for this show.

The scrape went OK, no errors, however a season poster has been assigned from a completely different show (CSI), and using an invalid path.

The details are here: http://pastebin.com/raw.php?i=WBvX6av4

Note that the path used for the Extant season 1 poster - which, remember, doesn't exist - is not actually valid. A valid path for the poster used (CSI season 1 poster) would be nfs://192.168.0.3/mnt/share/media/Video/TVShows/CSI Crime Scene Investigation/season01-poster.jpg, as I have some tvshows in Video, and other tvshows in Video-Private (the former I rsync to a remote server, the latter I don't).

I also scraped this same show (and files) on a Gotham (OpenELEC 4.0.6) machine and it scraped correctly, with no poster being assigned to Season 1.

MySQL is being used for both machines (MyVideos86 for Helix/master, and MyVideos78 for Gotham).

@jmarshallnz

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Jul 10, 2014

Member

The problem is that the season art was previously assigned to a different season (of a different show) and it wasn't cleared out at all is my guess, as there's no trigger on the art table to delete art when a season goes AWOL. Sound reasonable? Note that this other show (CSI) may have existed some time ago.

Member

jmarshallnz commented Jul 10, 2014

The problem is that the season art was previously assigned to a different season (of a different show) and it wasn't cleared out at all is my guess, as there's no trigger on the art table to delete art when a season goes AWOL. Sound reasonable? Note that this other show (CSI) may have existed some time ago.

@MilhouseVH

This comment has been minimized.

Show comment Hide comment
@MilhouseVH

MilhouseVH Jul 10, 2014

Contributor

It's possible, yes, as at one point I did move all but the latest season of CSI files to Videos-Private to free up space on the remote server, but this caused more problems than it solved so found a better solution (specify an exclude file on the rsync command) and so moved the files back, then "cleaned" the database before re-scraping from their original location. So I guess this accounts for the crap in the art table.

Obviously this new tvshow shouldn't have any season artwork assigned right now, so whatever the actual reason, that still looks like a regression. Is there any way to avoid assigning random artwork to new shows that don't (yet) have their own artwork? Maybe clear out the art table of any junk during the db upgrade?

Contributor

MilhouseVH commented Jul 10, 2014

It's possible, yes, as at one point I did move all but the latest season of CSI files to Videos-Private to free up space on the remote server, but this caused more problems than it solved so found a better solution (specify an exclude file on the rsync command) and so moved the files back, then "cleaned" the database before re-scraping from their original location. So I guess this accounts for the crap in the art table.

Obviously this new tvshow shouldn't have any season artwork assigned right now, so whatever the actual reason, that still looks like a regression. Is there any way to avoid assigning random artwork to new shows that don't (yet) have their own artwork? Maybe clear out the art table of any junk during the db upgrade?

@jmarshallnz

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Jul 10, 2014

Member

We clear the art table of everything except season art whenever the item itself is removed, so yes, that's what needs to be done. On update (when the seasons were removed) that would have taken care of it.

The reason the artwork was assigned to the newly added season is because it was already in the art table with the same season id (a left-over) and the new season had no art: If the season had had art it would have overridden what was already in there.

Member

jmarshallnz commented Jul 10, 2014

We clear the art table of everything except season art whenever the item itself is removed, so yes, that's what needs to be done. On update (when the seasons were removed) that would have taken care of it.

The reason the artwork was assigned to the newly added season is because it was already in the art table with the same season id (a left-over) and the new season had no art: If the season had had art it would have overridden what was already in there.

@MilhouseVH

This comment has been minimized.

Show comment Hide comment
@MilhouseVH

MilhouseVH Jul 10, 2014

Contributor

Understood, so it sounds like a corner-case but hopefully one that can be prevented in future. Do you want a Trac ticket for this?

Contributor

MilhouseVH commented Jul 10, 2014

Understood, so it sounds like a corner-case but hopefully one that can be prevented in future. Do you want a Trac ticket for this?

@jmarshallnz

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Jul 10, 2014

Member

Nope - this is enough reminder. I'll get to it as soon as I have a spare moment with a fresh branch :)

Member

jmarshallnz commented Jul 10, 2014

Nope - this is enough reminder. I'll get to it as soon as I have a spare moment with a fresh branch :)

@jmarshallnz

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Jul 11, 2014

Member
@Forage

This comment has been minimized.

Show comment Hide comment
@Forage

Forage Jul 18, 2014

Ever since build #706 of @MilhouseVH his OpenElec RPi build (http://forum.xbmc.org/showthread.php?tid=192380&pid=1747949#pid1747949) XBMC is downloading TV Show episode details/info every time you go and view them.
At the same time, this build is also the start of XBMC crashes when you start to play episodes from the details/info page even though the same episodes played fine before and it doesn't happen when you select either OMXplayer of DVDplayer from the "Play Using..." context menu.

Judged by the build changelog it's this PR that's the cause of the behaviour change as far as I can tell.
The downloading of info is quite annoying when you have the "OK" button (remote) set to display this page/info by default.
Is the downloading of the info new behaviour by design? I hope not.

Movies info and playing is just fine.

Here's a debug log, first issuing the info download, followed by the XBMC crash: http://xbmclogs.com/show.php?id=249119

Forage commented Jul 18, 2014

Ever since build #706 of @MilhouseVH his OpenElec RPi build (http://forum.xbmc.org/showthread.php?tid=192380&pid=1747949#pid1747949) XBMC is downloading TV Show episode details/info every time you go and view them.
At the same time, this build is also the start of XBMC crashes when you start to play episodes from the details/info page even though the same episodes played fine before and it doesn't happen when you select either OMXplayer of DVDplayer from the "Play Using..." context menu.

Judged by the build changelog it's this PR that's the cause of the behaviour change as far as I can tell.
The downloading of info is quite annoying when you have the "OK" button (remote) set to display this page/info by default.
Is the downloading of the info new behaviour by design? I hope not.

Movies info and playing is just fine.

Here's a debug log, first issuing the info download, followed by the XBMC crash: http://xbmclogs.com/show.php?id=249119

@jmarshallnz

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Jul 18, 2014

Member

I'm aware of the issue with the episode info. You'll note it's not actually downloading information - rather it's just running through the scanner when it shouldn't need to (the items have video information already).

Member

jmarshallnz commented Jul 18, 2014

I'm aware of the issue with the episode info. You'll note it's not actually downloading information - rather it's just running through the scanner when it shouldn't need to (the items have video information already).

@jmarshallnz

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Jul 19, 2014

Member

The videoinfoscanner detour is fixed in e279a29

Member

jmarshallnz commented Jul 19, 2014

The videoinfoscanner detour is fixed in e279a29

@jmarshallnz

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Jul 19, 2014

Member

(And no crash here btw)

Member

jmarshallnz commented Jul 19, 2014

(And no crash here btw)

@Forage

This comment has been minimized.

Show comment Hide comment
@Forage

Forage Jul 19, 2014

Thank you for fixing this.

The crash might not be related to this PR.
"Normally" playing starts with the UI still partially visible, than it follows by a brief flicker, blacking out the screen shortly, following with resuming playback. The crash now happens a the flickering point, right after a view frames have been played. It could be related to this PR since it only happens with TV Shows and it started occurring after this commit, it could also be the screen refresh rate adjustment for all I know.
I'll try to reproduce it with the new build which includes this dialog fix and try out a debug build if it is still present.

Forage commented Jul 19, 2014

Thank you for fixing this.

The crash might not be related to this PR.
"Normally" playing starts with the UI still partially visible, than it follows by a brief flicker, blacking out the screen shortly, following with resuming playback. The crash now happens a the flickering point, right after a view frames have been played. It could be related to this PR since it only happens with TV Shows and it started occurring after this commit, it could also be the screen refresh rate adjustment for all I know.
I'll try to reproduce it with the new build which includes this dialog fix and try out a debug build if it is still present.

@Forage

This comment has been minimized.

Show comment Hide comment
@Forage

Forage Jul 19, 2014

The fix works like a charm, great, many thanks. So far no crashes either, oddly enough, but they started at a later stage in the past as well. Ow well, we'll see.

Forage commented Jul 19, 2014

The fix works like a charm, great, many thanks. So far no crashes either, oddly enough, but they started at a later stage in the past as well. Ow well, we'll see.

@Montellese

This comment has been minimized.

Show comment Hide comment
@Montellese

Montellese Aug 13, 2014

Member

What's the reasoning being GROUP BY tvshow.idShow not being part of the tvshowview definition? The change in GetTvShowsByWhere is causing trouble with the retrieval of the total number of tvshows when using limiting (e.g. through JSON-RPC). The resulting query (created on line 6382) is SELECT COUNT(1) FROM tvshowview GROUP BY tvshowview.idShow which results in a result set with multiple records all consisting of a single result with the value 1 instead of a result set with a single record containing the number of rows in the tvshowview.

Member

Montellese commented on c5691f1 Aug 13, 2014

What's the reasoning being GROUP BY tvshow.idShow not being part of the tvshowview definition? The change in GetTvShowsByWhere is causing trouble with the retrieval of the total number of tvshows when using limiting (e.g. through JSON-RPC). The resulting query (created on line 6382) is SELECT COUNT(1) FROM tvshowview GROUP BY tvshowview.idShow which results in a result set with multiple records all consisting of a single result with the value 1 instead of a result set with a single record containing the number of rows in the tvshowview.

This comment has been minimized.

Show comment Hide comment
@Tolriq

Tolriq Aug 20, 2014

Contributor

Ping @jmarshallnz in case he missed this comment as some things are broken (http://forum.xbmc.org/showthread.php?tid=68263&pid=1770810#pid1770810)

Contributor

Tolriq replied Aug 20, 2014

Ping @jmarshallnz in case he missed this comment as some things are broken (http://forum.xbmc.org/showthread.php?tid=68263&pid=1770810#pid1770810)

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