CappielloAntonio / tempo

An open source and lightweight music client for Subsonic, designed and built natively for Android.
GNU General Public License v3.0
1.06k stars 49 forks source link

[BUG] - some album art doesn't show up #89

Closed Cambi0nn closed 11 months ago

Cambi0nn commented 1 year ago

Describe the bug

While most do, some album art doesn't show up despite being in the metadata and showing up on my file browser on my OS as well as Astiga (which I use as subsonic server).

Expected behavior

Always show album art when it's available.

To Reproduce

I'm not sure what makes some not show up.

Environment

Logs or Screenshots

Example of an album art not showing despite being there: e8510990-606f-4a87-97cb-6eeb3b6aa171

Additional context

N/A

Cambi0nn commented 1 year ago

I also noticed, none of the podcasts are showing their image. Not sure if it's related or a separate issue.

CappielloAntonio commented 1 year ago

Hi @Cambi0nn could you make sure that the server responds correctly to you using this endpoint?

As with all other covers, I do nothing other than query this API to receive the information I need, I do not distinguish between the various covers of the various objects (tracks, albums, artists, podcast episodes, channels, etc.).

Cambi0nn commented 11 months ago

Outside of podcasts, about 99% of the album art loads fine, only a select view don't (it's consequent which). They are all handled the same by Astiga. I do notice that they tend to take a bit longer to load (when not in cache) than other art. Perhaps the issue is there? Maybe something along the lines of Tempo timing out before it got the file?

gravelld commented 11 months ago

@CappielloAntonio When Astiga receives getPodcasts it returns something like:

{
    "subsonic-response": {
        "status": "ok",
        "version": "1.16.0",
        "serverVersion": "Astiga/production",
        "podcasts": {
            "channel": [
                {
                    "id": "8aaa4e06-683a-11ee-af7d-0242ac120003",
                    "url": "http://rss.acast.com/timesthegame",
                    "title": "The Game Football Podcast",
                    "description": "<p>The Game is the premier football podcast from The Times, with the finest writers previewing and reviewing all the action throughout the 2023/24 season. Twice a week, Gregor Robertson and Tom Clarke speak to the leading football writers from The Times and The Sunday Times, offering unparalleled analysis&nbsp;of the latest results and commentary on emerging issues.&nbsp;</p><br><p>On Mondays, Alyson Rudd, Tom Roddy and Tony Cascarino review the weekend's action and on Thursdays Martin Samuel tackles the biggest issues of the week alongside the likes of Jonathan Northcroft and Ian Hawkey.</p><br><p>Subscribe to The Times to enjoy our award-winning sports journalism on your smartphone or tablet. Just search The Times online.</p><br /><hr><p style='color:grey; font-size:0.75em;'> Hosted on Acast. See <a style='color:grey;' target='_blank' rel='noopener noreferrer' href='https://acast.com/privacy'>acast.com/privacy</a> for more information.</p>",
                    "coverArt": "110729",
                    "originalImageUrl": "https://assets.pippa.io/shows/61ba0e501a8cbe71d83cf13e/show-cover.jpg",
                    "status": "completed"
                }
            ]
        }
    }
}

I observed in the server logs requests made for:

/rest/getCoverArt?...c=Tempo&id=https://assets.pippa.io/shows/61ba0e501a8cbe71d83cf13e/show-cover.jpg

This suggests originalImageUrl is being used for the getCoverArt call. As per http://www.subsonic.org/pages/inc/api/examples/podcasts_example_1.xml I think this should be used as a globally accessible, non-contextual field.

If, instead, the call was made for:

/rest/getCoverArt?...c=Tempo&id=110729

(i.e. using the subsonic ID) ... the call should work.

Thoughts?

CappielloAntonio commented 11 months ago

Then yes, this was my mistake. It should not be:

        CustomGlideRequest.Builder
                .from(holder.itemView.getContext(), podcastChannel.getOriginalImageUrl(), CustomGlideRequest.ResourceType.Podcast)
                .build()
                .into(holder.item.podcastChannelCoverImageView);

but:

        CustomGlideRequest.Builder
                .from(holder.itemView.getContext(), podcastChannel.getCoverArtId(), CustomGlideRequest.ResourceType.Podcast)
                .build()
                .into(holder.item.podcastChannelCoverImageView);

If you like, you can try this test apk: app-tempo-debug.zip

CappielloAntonio commented 11 months ago

As for the slow loading of some images, you could always try decreasing their size from the app settings. Useful for requesting the server to send a reduced size image if the original is too large or too heavy.

Cambi0nn commented 11 months ago

As for the slow loading of some images, you could always try decreasing their size from the app settings. Useful for requesting the server to send a reduced size image if the original is too large or too heavy.

Putting it to medium makes them show up indeed! Funny, in my head I was thinking I have some big files and unlimited data, if so I should keep the quality to high for best results. Ah well, consider those album arts solved.

If you like, you can try this test apk: app-tempo-debug.zip

Sadly the pidcasts still look like this instead of showing images. Screenshot_20231011-182347_1

CappielloAntonio commented 11 months ago

My bad, my bad. I incorrectly deserialized the channel object the server sends me. And I also take back what I said about Gonic, it was my mistake. As for the episodes, could you check from the server logs if the cover is visible?

It should work fine now: app-tempo-debug.zip

Cambi0nn commented 11 months ago

It works great in the channel part now!

@gravelld could you possibly check the episode image part? Screenshot_20231011-184207_1

gravelld commented 11 months ago

(I'm still using 3.5.7 here, not sure if that affects things)

I can't see any calls to getCoverArt being made for any of the episodes. Here's some example JSON for getNewestPodcasts (I also don't see any calls being made for getPodcasts where includeEpisodes=true).

getNewestPodcasts.json

CappielloAntonio commented 11 months ago

Each podcast episode has an art property, while I expect it to be called coverArt

gravelld commented 11 months ago

Thanks - I'll look into changing that.

gravelld commented 11 months ago

Ok - I've changed that. @Cambi0nn can you report behaviour? You might have to force a refresh.

Cambi0nn commented 11 months ago

Ok - I've changed that. @Cambi0nn can you report behaviour? You might have to force a refresh.

Yes it works perfectly now! Awesome! Screenshot_20231012-164933_1

CappielloAntonio commented 11 months ago

Thank you so much, you have been an invaluable help!

molohov commented 10 months ago

When will this fix be available in a release?