Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[video] Add TV show artwork to episodes/seasons with their own "fanart". #8645

Merged
merged 1 commit into from Oct 6, 2016

Conversation

Projects
None yet
7 participants
@rmrector
Copy link
Contributor

commented Dec 21, 2015

This block of code adds artwork from the parent TV show to episodes and seasons, and item.HasArt("fanart") seems intended to run it only if the TV show artwork hasn't yet been added. But these days, episodes and seasons can have their own artwork, "fanart" included, so this check isn't quite accurate.

For skins this means it's difficult to get TV show artwork from episodes or seasons that have their own fanart, as this prevents ListItem.Art(tvshow.*) and Player.Art(tvshow.*) from being populated.

This change updates it to check against the full "tvshow.fanart", restoring the intended behavior.

@Montellese

This comment has been minimized.

Copy link
Member

commented Dec 27, 2015

I don't fully know how the thumbloader and the artwork cache etc. works but this should be ok AFAICT.

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Dec 27, 2015

This fix looks good, although I'm having trouble understanding what the code is actually doing, so maybe an example will help clarify.

Artwork for an episode without episode fanart (with or without this PR):

            "art": {
              "season.poster": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/season01-poster.jpg/",
              "thumb": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/Season 1/Brass Eye S01E06-thumb.jpg/",
              "tvshow.banner": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/banner.jpg/",
              "tvshow.clearart": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/clearart.png/",
              "tvshow.clearlogo": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/logo.png/",
              "tvshow.fanart": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/fanart.jpg/",
              "tvshow.landscape": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/landscape.jpg/",
              "tvshow.poster": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/poster.jpg/"
            },

Once episode fanart is added, without this PR the other tvshow artwork types disappear:

            "art": {
              "fanart": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/Season 1/Brass Eye S01E06-fanart.jpg/",
              "season.poster": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/season01-poster.jpg/",
              "thumb": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/Season 1/Brass Eye S01E06-thumb.jpg/"
            },

However with this PR, all artwork is present as expected:

            "art": {
              "fanart": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/Season 1/Brass Eye S01E06-fanart.jpg/",
              "season.poster": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/season01-poster.jpg/",
              "thumb": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/Season 1/Brass Eye S01E06-thumb.jpg/",
              "tvshow.banner": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/banner.jpg/",
              "tvshow.clearart": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/clearart.png/",
              "tvshow.clearlogo": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/logo.png/",
              "tvshow.fanart": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/fanart.jpg/",
              "tvshow.landscape": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/landscape.jpg/",
              "tvshow.poster": "image://nfs://192.168.0.3/mnt/share/media/Video-Private/TVShows/Brass Eye/poster.jpg/"
            },
@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Dec 27, 2015

Kind of wondering if this also merits a JSON API bump (minor) - even though it's not touching any JSON code it is changing (for the better) the JSON results.

@Razzeee

This comment has been minimized.

Copy link
Member

commented Dec 27, 2015

+1 on the json api bump

@rmrector rmrector changed the title [video] Add season/TV show artwork to episodes/seasons even if they have their own fanart. [video] Add TV show artwork to episodes/seasons with their own "fanart". Jan 30, 2016

@rmrector

This comment has been minimized.

Copy link
Contributor Author

commented Jan 30, 2016

Took a stab at writing a better description. Milhouse's example is perfect, and skin access behaves the same with ListItem.Art().

@rmrector rmrector force-pushed the rmrector:yours_and_mine_artwork branch from a2f1898 to ae13e9b Aug 10, 2016

@rmrector

This comment has been minimized.

Copy link
Contributor Author

commented Aug 10, 2016

Whoops, I had assumed the API version bump would be done when this was merged in, but looking through other PRs that's generally not the case.

Rewrote history to bump JSON API minor version.

@hudokkow

This comment has been minimized.

Copy link
Member

commented Aug 10, 2016

jenkins build this please

@rmrector

This comment has been minimized.

Copy link
Contributor Author

commented Aug 16, 2016

Jenkins failed on Linux 64bit only; I took a look at the log but didn't see any error message other than the failed status. I came to take another look at it but now it's gone.

Is there anything I need to do for this?

@Razzeee

This comment has been minimized.

Copy link
Member

commented Aug 16, 2016

jenkins build this please

@rmrector

This comment has been minimized.

Copy link
Contributor Author

commented Sep 13, 2016

Can this be merged in?

@rmrector

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2016

Could someone merge this in, please?

I understand this is a small, obscure bug, but a bug nonetheless and should be fixed, especially since that fix is right here just waiting for someone to merge it in, and no one has expressed any concerns with it.

If this won't be merged in, or at least not for Krypton, could you tell me why? I am a new contributor to this community and would appreciate any help from veterans or Team Kodi members on what I may be missing.

@phil65

This comment has been minimized.

Copy link
Member

commented Oct 6, 2016

+1 for merge. I think this can get labelled as a fix, from a skinner´s perspective the current behaviour doesnt make sense.

Sorry for the lack of answers, sometimes PR´s slip through our fingers. :)

@MartijnKaijser MartijnKaijser merged commit 1085bc2 into xbmc:master Oct 6, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins.kodi.tv You did a great job. Have a cookie.
Details

@MartijnKaijser MartijnKaijser added this to the Krypton 17.0-beta4 milestone Oct 6, 2016

@rmrector

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2016

Great, thanks guys!

@rmrector rmrector deleted the rmrector:yours_and_mine_artwork branch Oct 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.