DJDoubleD / refreezer

An alternative Deezer music streaming & downloading client, based on Freezer.
GNU General Public License v3.0
276 stars 10 forks source link

[Bug] Multiple bugs report #41

Closed PetitPrinc3 closed 1 week ago

PetitPrinc3 commented 1 week ago

šŸž Bug Report

Describe the bug

BUG 1

If more than 1 playlist is made "offline" then none of the offline playlists are available offline. Worse, none of the offline tracks are available. This also happens when you download big playlists (eg. 80 tracks). I couldn't find the issue in the code though I think it comes from the loadOfflineTrack function... Any idea ?

BUG 2 / Feature

The tracks are downloaded twice if you download them and then make them "offline". Any chance we can edit the code so downloaded tracks are available offline in the app ? From what I see, it means editing the Stream builder so it can use files not named /offline/trackId.mp3 I couldn't get it to work though. Any idea ? I guess it should end up being something like :

Additional context

For anyone reading this and interested in working on this project, feel free to get in touch. Discord : petitprinc3

*

DJDoubleD commented 1 week ago

This should really have been 2 separate issues, but:

Bug: offline playlists disappear

I'm unable to reproduce this. I made 2 test playlists with 2 tracks in them each. Then made both of them offline, waited for them to be downloaded and disabled internet connectivity. I was able to see both playlists in the offline library and was able to playback all 4 tracks.

Maybe the problem was that you didn't wait for the entire large playlist of 80 tracks to be downloaded, before going offline?

Feature (not bug): merge externally downloaded content with offline content

I think the feature that is described here, overlaps with what's asked in #11. The app is build as a streaming app that has offline streaming capabilities and a separate external download feature. The offline content is only accessible by the app itself and can therefore reliable be kept in the internal db.

The external download feature is aimed at people wanting to archive the content or make it accessible to other local players. In your analysis, you assume that the externally downloaded files will remain at the download location (with the same filename). This won't always be the case (often, it won't), so tracking the externally downloaded files as offline content (and adding it to the db), will lead to problems.

What is basically asked here (and in #11), is that the app would also incorporate the features of a local music player (like AIMP etc.), which can scan local folders for music and merge these items with the offline content in the library. This would require an extensive rewrite and even then, moving externally downloaded files outside of the app, will remove offline content of the app. Currently, a rewrite like this is outside of my scope for the app.

I personally don't use discord much, but I'll try to add you when I have some time to actually chat more in depth.

PetitPrinc3 commented 1 week ago

Duly noted for the download feature. About the offline bug, I am able to reproduce by downloading albums as well. I did wait for the downloads to be over.

Screenshot_20241108-145217.png

Screenshot_20241108-145204.png

Screenshot_20241108-145159.png

Screenshot_20241108-145533.png

Screenshot_20241108-145537.png

DJDoubleD commented 1 week ago

Context mattered in this case: there was a casting error for tracks with a fallback. So the bug only manifested when a favorite album or playlist contained such a track.

PetitPrinc3 commented 1 week ago

Awesome, I'll test this tomorrow. I'm interested in finding out how you found this if you happen to have some time to spare šŸ˜‰ Anyways, thanks !

DJDoubleD commented 1 week ago

I'm interested in finding out how you found this

I spend a bit of time debugging with your example albums (the Ed Sheeran one had such a track, would be nice to add links in text next time ;) ). Once I could reproduce the bug, I found the error thrown by the int fetched from the sqlite db being used as a String pretty fast and I just remembered the context (unfortunately I missed this bug back when I added Fallback support, I had assumed the Id was saved as a String). Then I decided to first do some testing and experimented with creating a dedicated widget for the AlbumList (avoid double loading etc), before I pushed the commits.

I noticed you made quite some progress on your mod and it looks good. I still would love it if we could somehow make (even part of?) your UI changes into a skin (I prefer being able to pick blue over the Deezer purple and an OLED black theme is a must :)). Even if it requires some widgets to have different versions for the "classic" and "new" look, it would be nice to try and keep the back-end / middleware consistent so that bugfixes and architectural improvements can easily be shared.

When you fix bugs or make improvements to the code, that can be made universal, please do try to push them back upstream. Just like I made a separate branch from your PR in my repo, you could make a branch in yours from my master and when you make structural fixes/improvements, port them to that branch and make a PR. It's always nice to avoid double work and I know there is still a lot that can be done. There is a reason the version number hasn't reached 1.0 yet ;)

Unfortunately, I have not had much time to spend on the project and to reach out to you on discord. I also haven't had time to install your version and see how it behaves in landscape mode, on tablet and on TV (I would love to find a fix for the remote controle focus issues).

PetitPrinc3 commented 1 week ago

Thank you for taking the time to reply. I will try to do what you suggest though I had to rewrite some widgets and I'm not sure about the relevance of having a refreezerUI/deezerUI switch in this case... (The code would become quite big and probably not very optimized) For information I kept the features such as "pick accent color", "light theme", "dark theme", etc. I know it is not yet handling landscape mode for most screens. I will take this issue soon I hope. I will try to create a separate branch as you suggested in the upcoming weeks if I get the chance. Cheers !