search

vanilla-music / vanilla

Vanilla Music Player for Android
GNU General Public License v3.0
1.17k stars 295 forks source link

Improve playlists import #643

Closed andiandi13 closed 5 years ago

andiandi13 commented 7 years ago

Description

I wrote an answer to the FAQ question "Why does my playlists keep disappearing ?" after some experimentations, which is the following :

The deletion of entries inside playlists can happens when SD Card is unmounted while the device is turned on. Make sure to turn it off before removing your SD Card.

Among your playlists, you can only restore system playlists by deleting your empty playlists and re-scan your library with "Flush media database" option activated.

If you have too much playlists to delete and your device is rooted you can go to root/data/data/ch.blinkenlights.android.vanilla/databases/ with a file explorer and delete media-library.db file before flushing the database in Vanilla, every playlists will then reappear.

It shows something that should IMO be improved : when playlists are deleted its hard to re-add their content since we have to delete each and every playlist with a long press, and then flush the media library. I felt that I had to provide the little root trick to not only give a hard solution.

And an important other thing : even if its not about deletion, if I add some new songs to a system playlist (e.g in stock music player) and I want them to appear in Vanilla, I have to delete the concerned system playlist in Vanilla and re-add it by flushing library :/

So why not add a new box under flush media library, named for example "refresh system playlists" or "re-import system playlists" or even "delete all playlists" or whatever but the idea is that it will delete the system playlists entries from the .db file (and ignore vanilla's internal playlists in preference) and re-import system playlists so that if they are deleted, we could quickly restore them, and if they are updated (by adding new songs into them with a stock player for example), we could refresh them.

I've experience a lot of playlists deletion since I use Vanilla and I imagine I'm not alone, so it would be cool if it's not a big deal anymore. And update our playlists easily would be even better thanks to that option.

Steps to reproduce the issue

Delete the content of a (system) playlists in Vanilla without deleting the playlist itself (faster to test), and do a re-scan + flush media library : your playlists should still be empty. You can also add new songs to your system playlists and rescan : no new songs added...

adrian-bl commented 7 years ago

I agree that our current handling of system playlists isn't that nice.

I'm just not sure how we should implement the syncing so that we do not overwrite our own changes.

One way could be that we always write our own content to the system playlist and store a hash value: If the hash value changes, we know that our value is 'outdated' and that we should drop our own copy and use the one of the system.

However: i'm not sure how well this will work in practice :-/

andiandi13 commented 7 years ago

our own changes.

You mean Vanilla's internal playlists + all tracks added to imported system playlists ?

Why not put internal playlists informations in another .db file aside covercache and media-library so that it will not be wiped if you add a new option to erase system playlists ?

adrian-bl commented 7 years ago

You mean Vanilla's internal playlists + all tracks added to imported system playlists ?

I mean that we could store a 'copy' of our own playlist as an 'android system' playlist.

Why not put internal playlists informations in another .db file aside covercache and media-library so that it will not be wiped if you add a new option to erase system playlists ?

Sorry, but now i'm confused - why would we want an option to erase system playlists?

andiandi13 commented 7 years ago

why would we want an option to erase system playlists?

Not erase SYSTEM playlists, but playlists in Vanilla that were imported from the system playlists. Because as I said in the description of the issue, I cannot update or retrieve my deleted playlists (their content is deleted not the playlist) until they are deleted one by one, its so long...

adrian-bl commented 7 years ago

Not erase SYSTEM playlists, but playlists in Vanilla that were imported from the system playlists.

Ah, so you mean that we shouldn't blindly sync an empty android playlist over an existing one? Yeah, having some heuristics there might be a good idea.

andiandi13 commented 7 years ago

If I have an Android system playlist named "Songs" for example, I can see it in every players on the device and Vanilla imported it as normal. But if tracks are deleted from that "Songs" playlists but only by Vanilla (e.g SDcard removal), there will still be a "Songs" playlist in Vanilla but with nothing inside it.

Even if I flush the database, Vanilla would think "Oh, the 'Songs' playlist is already there, no need to import tracks into it" so to restore tracks into it I have to delete it first so that Vanilla will import it from the system because it doesn't exists in its own library.

And the same thing happens if new tracks are added into a system playlist (e.g with stock music player), I have to delete the old one from Vanilla to import the brand new playlist by flushing database, if I don't, then I'll never have my new tracks.

Maybe its what you said by "blindly sync an empty playlist over an existing one".

So, yeah, Vanilla should RE-IMPORT system playlist above the old one it has already imported, whether its empty or not.

I hope I have explained well :)

ghost commented 7 years ago

My main problem with playlists is that I like to change (/generate) them on my PC and the ones on my phone don't update (easily). The android media library handles that very poorly already (sometimes I have to delete its data and reboot / re-index several times to make it work). And after that's finally done, getting them into vanilla player is another PITA.

( Also I hate loosing the favourites and top100 / play counts )

That being said, I can't think of a default behaviour for vanilla player that would suit most users well here. It might be a step in the right direction to clearly separate (with different tabs or indicator / prefix) the system, vanilla player and filesystem (m3u etc) playlists from each others - that could offer a solution for some people and also make problems and potential solutions more visible (Half the time I can't even tell if my "playlist troubles of the day" are a vanilla player issue or an upstream problem).

( Personally, I'd love to just store play count, favourite status, playlist membership and everything directly as custom tags in the music files in a neat portable way and get rid of all those annoying databases / playlists except as a cache that can be deleted any time without data loss... but that's probably not something many people would prefer. )

mase76 commented 7 years ago

I also have problems to import my m3u playlists into vanilla. Even when they have been successfully recognized by the system, it is either not present in vanilla, or it is empty. Vanilla has its own music library, why not its own playlist handling? A simple load m3u and maybe save the queue as m3u would be enough for the beginning. Later some sorting options for the queue, and everything would be fine. Or can you exactly tell me step for step, how I can import a m3u playlist, so that it is fully recognized by the system and vanilla? I was not able to reproduce. I put a m3u into the playlist folder, did reboots, did full rescans of the library, and somehow in the end, I had the playlist in vanilla, while it was in the system much ealier. But I do not know the exact steps to do. It could put a copy into the system, but should not try to pick up system playlists. I think, vanilla should do the playlist stuff independent from all. Android's library and playlist handling is a mess. It empties the playlists, when the files are not accessible for just a moment, moved the folder or not mounted.

andiandi13 commented 7 years ago

@mase76 Did you checked out the FAQ ?

https://github.com/vanilla-music/vanilla-music.github.io/wiki/FAQ#playlists-management

If your playlist are loaded from m3u to system, and they are empty in vanilla, delete them and then do a flush of the database, they should reappear full.

ghost commented 7 years ago

Does the "flush" option delete "favorites" & "top" playlists / play counts?

mase76 commented 7 years ago

Yes I checked the FAQ. But this does not work every time.

andiandi13 commented 7 years ago

@yaymeh it does not unless you cancel the operation before it ends.

Again use the 'help' section of the app, there is some useful things.

andiandi13 commented 7 years ago

@mase76 I import playlist using this nice app https://play.google.com/store/apps/details?id=org.ssi.playlistbackup.

Try to import your m3u with its "import" section, then delete empty playlists in vanilla and flush the database.

mase76 commented 7 years ago

I am running my system without gapps. F-droid is my store. And why using another app just for playlist stuff?

andiandi13 commented 7 years ago

@mase76 here : https://drive.google.com/open?id=0B7yLYKIM8DZYNEt4Rk4zUDBvYUU

Just to test, to begin. Its not ideal but sometimes android is slow to import our playlist, the app can accerelerate the process.

mase76 commented 7 years ago

@andiandi13 Thx! The thing is not getting this app, I know the app. It is sad, that we have to workaround strange android behaviours with such apps.

In the end, I got my m3u to vanilla. But this should be easier. Easy to exchange with pc edited playlists, multiple lists, and saving to m3u. Is loading and saving playlists rocket science? I would implement it by myself, but I am not with java. Just take it as a feature request.

andiandi13 commented 7 years ago

@mase76 I agree, its why I opened the issue, I'm not part of the Dev team though.

mase76 commented 7 years ago

It seems, that google does not expect, that people use their smartphone to listen to music.

andiandi13 commented 7 years ago

Still better that iTunes xD

I have a cool xposed module named Xposed Media Scanner Optimizer, you can manually update your media library (import playlists, detect new tracks), without having to restart your phone or waiting some minutes..

If you are rooted obviously.

adrian-bl commented 7 years ago

I really need to write an m3u parser.

Would someone mind uploading a few sample m3u files? (Created by various applications, like itunes or windows media player)

ghost commented 7 years ago

As far as I know, there aren't many differences. I usually just:

Theoretically you can also replace playlists in playlists with their content recursively, but I don't know if anyone really uses that feature on purpose or if people just drag & drop playlists into playlists by accident somehow.

mase76 commented 7 years ago

Here is a big one created with clementine on linux. playlist.zip

andiandi13 commented 7 years ago

M3u created by Phonograph player https://drive.google.com/file/d/0B7yLYKIM8DZYOEFyanluTW9rSms/view?usp=drivesdk

adrian-bl commented 7 years ago

Cool, thanks for the samples.

I wonder whats the best way to implement this: My current preferred idea is to just always sync with what we find in the 'Playlists' folder:

Example:

If someone creates / changes a Playlist in Vanilla Music, a copy of it would be stored as /sdcard/Playlists/$name.m3u8

If we detect a changed (or new) file in /sdcard/Playlists, we would just import it on the spot.

mase76 commented 7 years ago

The principles of this ideas are good. However if we keep a playlist in the playlist folder, the result is, that the same playlist appears twice in the system. Not a big problem. A copy of the vanilla playlist in the system is welcome. You can use it with the alarm clock in some roms. When you allow some sorting in the queue, which can be saved as m3u to the playlist folder, it would be fine. When syncing of the playlists folder doesn't take so much load, it would be ok. Manual, periodic or permanent sync? Or just when the app starts?

andiandi13 commented 7 years ago

I think a good way to implement it would be to do the same as included folder for media scanner. First we choose the main folder where our playlists are stored, and then when vanilla detect new playlists it automatically import them AND we can launch a manuall scan also.

oleastre commented 6 years ago

Searching for a solution around m3u files and android, I just found a m3u parser for android. This could maybe be helpful instead of implementing your own solution. https://github.com/iheartradio/open-m3u8

adrian-bl commented 6 years ago

Started to work on this via eba6acabc2e9a4215c8053d32f5f700f3a782eda

We now have a better way to be notified on playlists changes.

adrian-bl commented 6 years ago

The latest nightly, which includes https://github.com/vanilla-music/vanilla/commit/4715bd3c3135b8325ebc593f24066e3c44ce5ec3, does now write playlists to /storage/Playlists.

Imports (and deletions) are not yet implemented, but we are getting there.

andiandi13 commented 6 years ago

Oh yes what a good news, I'm going to try it thanks !

adrian-bl commented 6 years ago

Note that you must modify a playlist to trigger the export (changing the order of a track is enough)

andiandi13 commented 6 years ago

It works great. I'll edit the wiki when I've time

adrian-bl commented 6 years ago

I pushed b957b49e72e19a1e52d026e396571dd117f5f50e which drives this forward: Note that i changed the file extension from M3U8 to M3U - so the .m3u8 variants written by older versions will be 'orphaned'.

The newest version of the code attempts to sync playlists: that is: it picks up changes to the M3U done by other applications and re-imports the playlist.

Not sure if i feel confident enough to enable the sync by default (it currently is) - i'd love to get some feedback on how well it works (and how many playlists it destroyed ;-) )

andiandi13 commented 6 years ago

@adrian-bl First, I want to say thanks for that new automatic import of playlist it's amazing ! Remember when I say it's so inconvenient to flush all the media library to refresh playlist ? I'm glad it's not the case anymore and it's really quick.

Also it means that Vanilla isn't linked with system playlist anymore as I noticed that if you delete the .m3u from the /playlist folder, it disappear in Vanilla. Some might say it's dangerous or whatever but I'd disagree, it's so much easier to manage Vanilla's playlist, and only Vanilla's.

So I tried a bit and I've some things to point out :

1) I think you really need to change the .m3u file structure. It's and extended m3u with all the #EXTINF infos. However, when you try to import it in Android database, e.g with BlackPlayer, with Playlist Backup app, it completely fails. Android (or these apps maybe) want a simple structure like :

/storage/emulated/0/music.mp3
/storage/emulated/0/folder/song.mp3

So I think you should make it simple so that we can manage them easily in other apps and then export them again to simple m3u (BTW simple m3us are well imported by vanilla right now)

2) As Vanilla automatically import m3u from Playlist folder, don't you think you should put them into a Vanilla Playlists folder instead to avoid apps to put useless playlist by accident but also to recognize where do Vanillas Playlist come from (and avoid deleting the folder thinking it's just a generic folder for example).

3) As import of playlist in the right folder is automatic and works great, don't you think you also should add an import button into Vanilla to copy m3u from any folder to Vanilla playlist folder ? (very easy thing to code and avoiding to lose time copying m3u everytime to the right folder)

For now that's all, I haven't encountered issues at all with playlists and I'm quite happy that this side of Vanilla is finally improving tbh.

adrian-bl commented 6 years ago

@andiandi13 Thanks a lot for your feedback!

Also it means that Vanilla isn't linked with system playlist anymore as I noticed that if you delete the .m3u from the /playlist folder, it disappear in Vanilla.

Yes: We use Androids FileObserver to pick up filesystem changes using inotify

Some might say it's dangerous or whatever but I'd disagree, it's so much easier to manage Vanilla's playlist, and only Vanilla's.

It should be fine if the code doesn't have any bugs, but doing this kind of 'fuzzy syncing' is always tricky. However: We always write a backup file before purging our own internal copy of a playlist.

I think you really need to change the .m3u file structure....

Oh, that's sad: I'd expect almost all software to understand M3U (tested it with AIMP), but i removed writing EXTINF for now via b6fc1b8e317c80efb689d1c7afe1eae9ef61cfdb as it doesn't add much value anyway.

  1. As Vanilla automatically import m3u from...

I was thinking about that - but i'm not sure. I haven't seen any apps yet which clutter the Playlist folder - and Playlist is kind of a standard system directory. We could maybe make it a config option?

  1. As import of playlist in the right folder is automatic ...

We could register ourselfs in the system to handle M3U files - or maybe support opening them in the file browser.

andiandi13 commented 6 years ago

Oh, that's sad: I'd expect almost all software to understand M3U (tested it with AIMP), but i removed writing EXTINF for now via b6fc1b8 as it doesn't add much value anyway.

Great ! Now I confirm it can be imported in any player.

I was thinking about that - but i'm not sure. I haven't seen any apps yet which clutter the Playlist folder - and Playlist is kind of a standard system directory. We could maybe make it a config option?

Yes you're right an option to change default playlist folder would be a good choice then.

We could register ourselfs in the system to handle M3U files - or maybe support opening them in the file browser

Hmm I guess supporting them in the browser is ok. It'd copy the m3u to vanilla's playlist folder (Playlist or other if we add an option to change it). But when I think more, it's not a big deal to copy m3u files into another folder in a file explorer, so it's definitely not a priority.

Also I've a few more suggestions/questions :

1) As you recently added song duration into the code, it would be cool to add it also for the playlists (at right of the name of each playlist in library view, or inside it idk).

2) Something new on #372 ?

3) Or if it's too hard to make an SQL view for playlist, maybe we could add sort option for the queue ! Why ? Because I can play all songs from a playlist (so enqueue them) and then sort the queue. It's a workaround to avoid changing playlist view. Unless it's the same work for both things...

LivInTheLookingGlass commented 6 years ago

To confirm, do these changes only observe /Playlist? Or do they also look for m3us that are in /Music?

It is much better, to me at least, if it also scans /Music. This allows people to use Syncthing to sync their music library, like I do now.

adrian-bl commented 6 years ago

It only checks /sdcard/Playlists. Doing a recursive check of the Music folder is not feasible (recursive inotify is a pain, very high chance of naming collisions).

This allows people to use Syncthing to sync their music library, like I do now.

I also use syncthing to synchronize my music collection, but one could just add the Playlists folder to syncthing.

LivInTheLookingGlass commented 6 years ago

Then it has to be separate. Can you just have it scan the top of Music? That would be a reasonable restriction, IMO.

andiandi13 commented 6 years ago

@adrian-bl I think you should keep the Playlist folder as a default folder but add a path chooser for custom path as we said before.

adrian-bl commented 6 years ago

Implemented this in https://github.com/vanilla-music/vanilla/commit/4e3c440f803684a94a239f1f715311cdb9be40fa

We default to /sdcard/Playlists but the user can choose a different folder

andiandi13 commented 6 years ago

@adrian-bl Thanks it work perfectly !

What about the 3 questions/suggestions in my previous message ?

adrian-bl commented 6 years ago

Ah sorry, missed them:

1: Displaying the total duration of the playlist would need more work: The SQL query only shows this for single songs right now. It might be possible to get this for playlists (and artists + albums) with good performance. Would need to do some testing

2 & 3: Sorry, never had time to work on this - should not be impossible to do -- but just takes some time to implement it.

andiandi13 commented 6 years ago
  1. Ok, it's not a priority but it would be nice to know if you can listen an entire playlist for a trip for example !

2/3. I see, I've always found odd that playlists had a separate view (not SQL I guess). Example :just recently, I realized that it's not convenient when you enter inside a playlist and play a song, that you don't have the queue at the bottom with control, you have to exit the playlist.

andiandi13 commented 6 years ago

@adrian-bl Something bother me with playlists :

Vanilla allow to add duplicate songs inside the same playlist, so we may re-add a song by accident 2 or more times.

In my case, I often copy new tracks to a specific folder, and then add that folder to a the corresponding playlist with a longpress. If I had 20 songs in the folder and I copied 12 songs, I'm now with a playlist with 52 songs with duplicates, it's a nightmare.

I think it would be pretty easy to add an option to disable duplicates songs into the same playlist (by forbidding duplicate file path) ?

mase76 commented 6 years ago

How about some sorting options for the queue? Then we could easy create playlists within vanilla.

andiandi13 commented 6 years ago

@mase76 That's what I just suggested in a previous post, but as an alternative of adding sort options inside playlist.

IMO, it wouldn't help that much for playlists creation because you have to add songs one by one in the queue.

AaronBWagner commented 6 years ago

New to Vanilla Music ... I had been having no end of trouble trying to get my Android phone to play .m3u playlists that I created on my computer. Just installed the latest nightly build of Vanilla Music, put the .m3u files in /sdcard/Playlist and it works! Thank you! I love this app.

andiandi13 commented 6 years ago

@adrian-bl : Is it possible to add a 'delete from device' option when we're inside a playlist ?

The delete button only delete the entry from the playlist as you know, and its really unconvenient when we clean some songs from a playlists and we have to find them in the device, one by one...

(Sorry for all those questions on playlists these days, but I really seek improvement ^^)

adrian-bl commented 6 years ago

Yes, that should be trivial - do you mind to open a bug for this?