kabouzeid / Phonograph

A material designed music player for Android
GNU General Public License v3.0
2.73k stars 728 forks source link

No more artist image #685

Closed astlab closed 5 years ago

astlab commented 5 years ago

After the last update, all the artist image are gone

-

Device info:

App version1.3.0
App version code172
Android build versionC900B131
Android release version6.0
Android SDK version23
Android build IDFRD-L09C900B131
Device brandHONOR
Device manufacturerHUAWEI
Device nameHWFRD
Device modelFRD-L09
Device product nameFRD-L09
Device hardware namehi3650
ABIs[arm64-v8a, armeabi-v7a, armeabi]
ABIs (32bit)[armeabi-v7a, armeabi]
ABIs (64bit)[arm64-v8a]
GeoZac commented 5 years ago

Duplicate of #677 Not app related issue

astlab commented 5 years ago

Ok, but you should find a workaround...for example show one of the artist's album cover...it's really annoying to have a list of stars instead of a picture, don't you think so?

alvinR0W commented 5 years ago

yes, please do find some workaround, even if it just means disabling the artist images all-together and just present the user a simple text only list of the artists.

a list of huge white stars just doesn't look appealing and i really don't want to add an artist image manually to all my artists. i love phonograph, i even bought the pro version way back because it just looks very good, runs super well and the visual presentation of my library has always been important to me. if this little irk would be ironed out i ( and many others) would probably be super happy.

Orycteropus commented 5 years ago

As far as I'm concern I developed the feature that display a mosaic of album covers as artist image. If only one album exists it displays this album cover. If two exist it displays one album cover at x = 0, y=0 and the second cover at x=artist image size / 2 and y=artist image size / 2. It's limited at 4 covers displayed at the same time. With more, you can't see anything.

Let me know if you are interested in me sharing the development I made for myself.

arkon commented 5 years ago

@Orycteropus That sounds pretty nice. I've seen Vinyl using Deezer, but their selection of images seems pretty lacking.

Orycteropus commented 5 years ago

I tried to find another good API to find the artist images but I didn't take a look at Deezer's. I'll try to mix the Deezer API call with the mosaic of covers to have the fewest images missing.

ocaballeror commented 5 years ago

If we're talking good API providers for the artist images, I've always found Spotify pretty reliable. It's not as good as lastfm, but they definitely have quite a decent database.

tralph3 commented 5 years ago

Yeah, Spotify would be good. However I support the alternative of having an imageless list.

astlab commented 5 years ago

Another thing to keep in mind is to have a dedicated directory in the gallery for storing all the images, instead to have them together with all the other images. And avoid to load them all each time from the internet. I did not see the code, but the behavior seems to be this.

Orycteropus commented 5 years ago

I'm not sure that is what was done before. There was a cache enabling not to fetch the result from internet.

astlab commented 5 years ago

yes, perhaps the behavior I observed was due to loading from the cache, but it's really slow, specially with big libraries

Orycteropus commented 5 years ago

I just implemented the Deezer API call to retrieve the artist image. If not found it displays a mosaic of covers found on the local storage (same method used to retrieved album cover).

It seems to work pretty well, I'll test it a little bit more. Let me know if you are interested by this features and I'd share my developments.

astlab commented 5 years ago

Yes, please share, I will test it

Orycteropus commented 5 years ago

I opened a PR : https://github.com/kabouzeid/Phonograph/pull/713

The code needs to be more tested, documented and refactored a little bit.

Orycteropus commented 5 years ago

I made some modifications allowing the mosaic to always be filled. The limit of four covers has been removed. Eventually it's nicer to see a lot of covers as artist images.

astlab commented 5 years ago

I installed your debug apk and, for my opinion, phonograph has reborn. Great job. only one thing, if the cover are less than 4 maybe the mosaic should not be shown.

Orycteropus commented 5 years ago

Thanks for reporting your feelings about the changes. I'll remove the mosaic if there is less than 4 albums. Eventually I agree with you, the result is not so good with 2 covers.

Orycteropus commented 5 years ago

I just made the changes if you want to update your app. If you have any ideas about how to improve the artist image when you don't have enough cover to make a mosaic, I'd be pleased to hear them.

astlab commented 5 years ago

I tested the new release, but the result was better before, in my opinion, because now I have several artists without image. In my opinion, the best way to solve this problem, is to show the last album cover if the artist doesn't have enough covers to show the mosaic

Orycteropus commented 5 years ago

Mmh it should be possible and it seems nice to me that way. I'll notice you when it will be available.

Orycteropus commented 5 years ago

Changes have been made and you're right it's better like this. :+1:

astlab commented 5 years ago

Great job man, Now all the artists' library is with an image. Still remain an error with last.fm (should be changed with deezer) when you search for an album's cover a (while editing the tag)...but maybe this is not the right place to report it

Orycteropus commented 5 years ago

I don't know how it's supposed to work. Yesterday I tried to download a cover from the Lastfm api through Phonograph (while editing the tags) and it worked. I was surprised because it never worked before and the tags never got edited.

astlab commented 5 years ago

Yes I did the same, but for me doesn't worked...

astlab commented 5 years ago

Hey, I try again now (I'm playing with the few artist without a cover) and the lastfm's service work... :D

liber-dovat commented 5 years ago

Hello. Where can I find @Orycteropus Phonograph debug apk?

astlab commented 5 years ago

Hi, This is the PR: https://github.com/kabouzeid/Phonograph/pull/713

liber-dovat commented 5 years ago

Thanks!

Orycteropus commented 5 years ago

For what I've seen, Spotify's api seems more complete compared to Deezer's.

I will try to update my branch this week to use Spotify's API.

arkon commented 5 years ago

I think it should be one or the other — photos of artists or just a collage of albums. @kabouzeid probably has a stronger opinion on that.

Orycteropus commented 5 years ago

Yes this is what I meant. To replace the use of Deezer's Api by Spotify's.

kabouzeid commented 5 years ago

@arkon you mean it should be either only artist images or only mosaics?

kabouzeid commented 5 years ago

@Orycteropus haven’t tried your changes yet, but by the look of the code I have a good feeling abou this, thanks.

arkon commented 5 years ago

@kabouzeid Yeah.

Orycteropus commented 5 years ago

I've checked and Pulsar uses the same system. Except that it displays only a cover from an album if no artist image found. In "my" (our) solution, I display an artist image, if not found a mosaic and if it's not possible the cover of the last album released. I find it pretty cool and natural actually. But @kabouzeid may have another opinion about that.

kabouzeid commented 5 years ago

So I just tested your implementation and I really liked it. However I am not sure whether I am allowed to use this API. (https://developers.deezer.com/termsofuse)

astlab commented 5 years ago

I think you're right, you can use it only for non commercial use, so you can use it only in the free version... I suppose.

astlab commented 5 years ago

...and it's the same for spotify

Orycteropus commented 5 years ago

I'm not sure the part IV forbids you to use the API.

It says: "It means that the developer will not perceive, receive, generate, directly or indirectly, any moneys, incomes, revenues, or any other consideration in connection with the services of the Services themselves, nor any other all Content accessed through the Services. "

As I understand it, it means that the call of the API should not be used to earn money. The use of the API is available in the free version of the app. What is not available in the free Phonograph version has nothing to do with the API.

I do not know.

astlab commented 5 years ago

anyway, I think that this paragraph is written especially for the streaming, and phonograph effectively doesn't generate any revenue from the use of the deezer's service

kabouzeid commented 5 years ago

Yeah I’m not sure too. I’ll write them an email. With a bit of luck they‘ll answer.

kabouzeid commented 5 years ago

I haven't found an alternative that we could use yet. So I'm thinking about merging the mosaic functionality only. Then we at least have some sort of images.

Orycteropus commented 5 years ago

@kabouzeid Do you want me to split my branch into two or you take over ?

kabouzeid commented 5 years ago

If you’ve got the time for it, go ahead :)

Otherwise, I’ll try to find some time later today.

Orycteropus commented 5 years ago

I opened a new PR with only the mosaic feature ;)

https://github.com/kabouzeid/Phonograph/pull/718

kabouzeid commented 5 years ago

https://github.com/kabouzeid/Phonograph/pull/718

astlab commented 5 years ago

I haven't found an alternative that we could use yet. So I'm thinking about merging the mosaic functionality only. Then we at least have some sort of images.

Maybe youtube? is not specific for this, but using the API you could grab some interesting infos: this is a sample: https://developers.google.com/apis-explorer/#p/youtube/v3/youtube.search.list?part=snippet&q=sting&relevanceLanguage=en&_h=2&

phloose commented 5 years ago

love your work! when do you think this patch will be available in the playstore?

Orycteropus commented 5 years ago

I developed for myself the call to the Spotify API. I read the terms of use and I think it can be implemented to Phonograph.

The Spotify API is more complete compared to the Deezer one but you have to implement an authentication process in OAuth2. You need to store in the application a client_id and a client_secret.

Hence I think that if somebody have to implement this feature, it should be an "official" developer who can follow the evolution of his app.