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

[music]Fix fanart for library items on playback from file view #13854

Merged
merged 1 commit into from May 9, 2018

Conversation

DaveTBlake
Copy link
Member

@DaveTBlake DaveTBlake commented May 5, 2018

Art type "thumb" was sometimes being added to a song with an empty URL, preventing artist fanart from being shown on playback of library items from file view.

A simple fix: song.strThumb is never read from the db, hence results in a blank entry. Since the art is managed elsewhere, this line should be removed from CFileItem::LoadMusicTag(). It was an historic mistake, the effects of which had been previously avoided because of code elsewhere that was recently removed. To test: play music that is in the library and has artist fanart from file view, note fanart shown on player OSD

@MilhouseVH this fixes the issue reported here https://forum.kodi.tv/showthread.php?tid=298461&pid=2730270#pid2730270

…own when playing library items from file view. Song.strThumb is never read from db
@DaveTBlake DaveTBlake added Type: Fix non-breaking change which fixes an issue Component: Music v18 Leia labels May 5, 2018
@DaveTBlake DaveTBlake added this to the Leia 18.0-alpha2 milestone May 5, 2018
@MilhouseVH
Copy link
Contributor

Hi Dave, this doesn't seem to have had any effect as far as I can tell - Files fanart still isn't being displayed in a test build that includes this PR.

@DaveTBlake
Copy link
Member Author

Odd @MilhouseVH , it works for me. Also "file fanart"? The facility only applies to items that are in the library (with art) and then happen to be played from fileview. Is that what you tested?

Not intended to work for non-library music files, even if there is a fanart.jpg in the folders above.

@MilhouseVH
Copy link
Contributor

The facility only applies to items that are in the library (with art) and then happen to be played from fileview. Is that what you tested?

It doesn't seem to make any difference if the album is in the library or not - I've tested both cases, and both fail to show fanart artwork after #13352 (which first appeared in test build #0120).

This is my test setup:

Folder #1, "Alphabet City" (album NOT in library although the artist, ABC, is in the libary):
s1

This is the metadata for the artist:

[
  {
    "artist": "ABC",
    "artistid": 1553,
    "fanart": "image://http://assets.fanart.tv/fanart/music/87199477-b0df-4ead-84ee-9b54b4abfc3d/artistbackground/abc-5068da2e9a303.JPG/",
    "label": "ABC",
    "thumbnail": "image://http://assets.fanart.tv/fanart/music/87199477-b0df-4ead-84ee-9b54b4abfc3d/artistthumb/abc-514adabe2db34.jpg/"
  }
]

When I navigate to the album folder "Alphabet City" via Files then - prior to #13352 - I'm shown the fanart artwork associated with the artist ABC. I have confirmed this by changing the artist fanart and the fanart shown when browsing the "Alphabet City" folder automatically changes to match whatever I set in the database (via JSON). Kodi never seems to load the contents of fanart.jpg.

Files view in test build #0119 without #13352:
s2

Same Files view in test build #0120 with #13352:
s3

Folder #2, "Beauty Stab" (album IS in the library):

[
  {
    "albumid": 2,
    "artist": [
      "ABC"
    ],
    "fanart": "image://http://assets.fanart.tv/fanart/music/87199477-b0df-4ead-84ee-9b54b4abfc3d/artistbackground/abc-5068da2e9a303.JPG/",
    "label": "Beauty Stab",
    "thumbnail": "image://nfs://192.168.0.3/mnt/share/data/Music/MP3/ABC/Beauty Stab/cover.jpg/",
    "title": "Beauty Stab"
  }
]

Again, the fanart is shown in #0119, but not in #0120.

Maybe my testing isn't quite the same as your own (or the user that reported this) but it does certainly show different behaviour before/after #13352, and this PR does not (currently) restore this old behaviour.


One other observation which is unrelated to #13352:

I changed the album artwork for "Beauty Stab" and Kodi (with build #0119) continued to show me the artist artwork for the album, and not the new album fanart artwork I had just set in the library. Only when I navigated to the "Beauty Stab" album in the Library did Kodi show me the correct album fanart. Maybe this is expected?

@DaveTBlake
Copy link
Member Author

Thanks for the extra details @MilhouseVH

Maybe my testing isn't quite the same as your own (or the user that reported this) but it does certainly show different behaviour before/after #13352, and this PR does not (currently) restore this old behaviour.

Yes, what you are doing is different, but I understand the results. However this PR does not claim to restore the pre #13352 behaviour, but just to show artist fanart when playing albums/songs that are in the library from fileview. You didn't test that.

What you did do was interesting, and it did show some other changed behaviour (not what the user complained about).

"Alphabet City" is not in the library, so this PR does not make it show artist fanart from fileview. But this situation - doing a lookup for fanart on artist name for non-library albums/songs when artist is in the library - is something that was dropped by #13352, and I will restore in another PR.

Kodi never seems to load the contents of fanart.jpg.

That is because your fanart.jpg is in the album folder, Kodi looks in the artist folder for local artist fanart.

"Beauty Stab" is in the library, but from the JSON output you seem to have added "fanart" for the album (not the artist). Did you use JSON for that? I think you must have, and note it is something v17 couldn't do added by #13101. Not expecting playback from file view to show album "fanart" in any version. In #0119 despite having db id it will be doing a lookup on artist name and getting that art - it should be using db id but the line of code this PR removes stops it working. In #0120 it does not get art because of that line, and I removed the look-up on name (that was masking the empty thumb flaw).

If with this PR you would like to try playing a library album by an artist with artist fanart from file view I'm sure you will see the fanart.

@MilhouseVH
Copy link
Contributor

That is because your fanart.jpg is in the album folder, Kodi looks in the artist folder for local artist fanart.

Ah OK, I was (mistakenly) focusing on the album art with my testing.

"Beauty Stab" is in the library, but from the JSON output you seem to have added "fanart" for the album (not the artist). Did you use JSON for that?

Yes, I used JSON to change the album fanart - I used texturecache.py:

  1. Determine the albumid for "Beauty Stab" (in this case, 2):
$ ./texturecache.py jd albums 'beauty stab'
[
  {
    "albumid": 2,
    "artist": [
      "ABC"
    ],
    "fanart": "image://http://assets.fanart.tv/fanart/music/87199477-b0df-4ead-84ee-9b54b4abfc3d/artistbackground/abc-5068da2e9a303.JPG/",
    "label": "Beauty Stab",
    "thumbnail": "image://nfs://192.168.0.3/mnt/share/data/Music/MP3/ABC/Beauty Stab/cover.jpg/",
    "title": "Beauty Stab"
  }
]
  1. Replace the album fanart artwork for album #2 (in this case using Blur fanart so the difference is clear):
$ ./texturecache.py set album 2 art.fanart "https://fanart.tv/fanart/music/ba853904-ae25-4ebb-89d6-c44cfbd71bd2/artistbackground/blur-53e5652d4c894.jpg"

Not expecting playback from file view to show album "fanart" in any version

OK fair enough, and that seems to be the case indeed.

If with this PR you would like to try playing a library album by an artist with artist fanart from file view I'm sure you will see the fanart.

I'll have to take your word for it - I think my head just exploded. :)

By the way the user has reported no change in behaviour with this PR: https://forum.kodi.tv/showthread.php?tid=298461&pid=2732543#pid2732543

@DaveTBlake
Copy link
Member Author

@MilhouseVH thanks for testing, confirming details of what you did and for including this in build #0505.

By the way the user has reported no change in behaviour with this PR

Not sure I trust his report accuracy (too excitable) but that probably means that, contrary to what he said initially, what he wants is to see artist fanart for albums that are not in the library i.e. try to lookup on artist name.

My thoughts are to merge this - it is a needed change - and rework the lookup on name in a separate PR. It was previous dropped because it was unnecessarily doing artist fanart lookup on artist name for everything, and corrected to use ID and get more types of artist art not just fanart. In a new PR I will restore that lookup by artist name for non-library albums and extend to other artist art types.

Definitely can't have your head exploding :p

@DaveTBlake DaveTBlake merged commit 8719bd2 into xbmc:master May 9, 2018
@DaveTBlake DaveTBlake deleted the FileviewFanartFix branch May 9, 2018 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Music Type: Fix non-breaking change which fixes an issue v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants