Rachel030219 / Poweramp-LRC-Plugin

An app to implement local .lrc file support for Poweramp.
Other
80 stars 4 forks source link

[BUG] Standalone mode does not look at music files recursively (i.e. subfolders) #25

Closed fei0316 closed 4 years ago

fei0316 commented 4 years ago

Describe the bug In the new standalone mode, music and lyrics files places in subdirectories are not considered. As I place music in structures like /sdcard/Music/Genre/Album/Name.m4a, the new mode is almost unusable for me, as I would need to add all folders for my albums one by one for it to work. The old method, on the other hand, has not caused instability for me and has worked well.

To Reproduce Steps to reproduce the behavior:

  1. Go to 'Manage folders' option
  2. Add the directory 'Music'
  3. Play a song with .lrc file
  4. Lyrics does not show
  5. Add the directory for that particular album
  6. Play the song again
  7. Lyrics shows

Device info:

Rachel030219 commented 4 years ago

Sorry for the inconvenience caused. This is an intended behavior, but I didn't consider the situation in which songs are stored in separated album folders. To solve the problem, the old method of searching for lyric files (in the selected folders) will be reintroduced in the next version, as a new experimental feature.

fei0316 commented 4 years ago

Thanks for bringing the old method back! However, I am curious whether the new method supports nested folder. I have seen many file management apps that asks me to give permission to the root folder when accessing an USB drive, and all sub-directories can also be accessed. I suppose you decided to use this new method because it brings certain benefits (performance, privacy, etc.) I am wondering if something similar could be done for this app as well.

Rachel030219 commented 4 years ago

However, I am curious whether the new method supports nested folder.

Yes, and instead of the legacy method, the option of "include sub-folders" is what I plan to add.

I suppose you decided to use this new method because it brings certain benefits (performance, privacy, etc.) I am wondering if something similar could be done for this app as well.

Frankly speaking, the main reason I choose to deprecate the old method is that it does so many improper tricks to reach the file, and these tricks has brought unexpected bugs.

Actually, folder chosen through Android Storage Access Framework (SAF) grants access to all of its files and sub-folders. However, the main difference is that file managers could just list all of that, while this plugin has to parse the path delivered by Poweramp, which looks like: primary/Music/A/B/C.mp3 . And that brings the problem, so as the unreliable and unstable old method.

That's why the standalone mode has been invented, to avoid all these tricky stuff, and only reads the file names while parsing the path, which is really a easy thing compared with parsing the whole path, also a performance-friendly thing since looping through all the sub-folders is no longer necessary. However, it neglects the sub-folders, and the outcome does not seem as good as I expected.

That's why I'll reintroduce the old method. Nonetheless, this time I'll try to find a more compromised way, at least after a version using standalone method, there is no more need to know what primary actually is, and the app only loops through these selected folders (instead of the whole storage, unless the user has chosen to do that). Considering the complexity of Android devices, whether it will work out on more devices than the old method is still unknown, but anyway I think that it is worth trying.

fei0316 commented 4 years ago

Thanks for the detailed explanation into how the app works. I think you could reintroduce the old method as an opt-in feature until standalone mode can completely replace the old method, so that no one will need to downgrade. With that said, I have a few ideas, although I don't know if they are feasible or make any sense :) I assumed the problem you're facing is mostly on the unreliable detection of the path of primary.

Although I also develop Android apps, I didn't have much experience in file storage. Hope my suggestions helps!

Rachel030219 commented 4 years ago

Thanks for the detailed explanation into how the app works. I think you could reintroduce the old method as an opt-in feature until standalone mode can completely replace the old method, so that no one will need to downgrade. With that said, I have a few ideas, although I don't know if they are feasible or make any sense :) I assumed the problem you're facing is mostly on the unreliable detection of the path of primary.

  • Loop through all subfolders when selecting a new folder, recording the filenames and paths in database. Then when Poweramp gives you a full path, you could just look up the filename for the path in your database. But the problem would be duplicate songs in different folders and the need to rescan after adding songs. Also it might be a good idea to prevent people from selecting the root folder and the app going through all the WhatsApp/WeChat voice messages, lol
  • Similar to the first method, but this time you try to match parts of the path of Poweramp. e.g. Poweramp gives you /storage/emulated/0/Music/Genre/Album/01.Name.mp3, and the user selects the Music folder: Genre/Album/01.Name.mp3. Then you could try to "find/match" the Poweramp-given path in the selected folder, i.e. "guessing" the user might have selected Album as the root folder, then Genre, then Music. You could also integrate that in the setup process (e.g. ask the user to play a song in Poweramp, match it and store the result)

Although I also develop Android apps, I didn't have much experience in file storage. Hope my suggestions helps!

Your idea does hit the main point and has reminded me, pushing me a step forward in the way of finding a better solution. I have assessed the second way previously actually, but before I was able to give it a try, the API change in Android 11 and the low reliability of the old method has driven me to deprecate the old method. Since there has been a new way to store all folders to look for (and less possibility that root folder is selected) with "include sub-folders" to be implemented in the next version, it is worth trying. Thanks!

fei0316 commented 4 years ago

I built the app with the newest source code a few days ago, and I think it is more or less working like it did before. However I am still in the process of adding lyrics to my music collection so I have not tested it extensively (and I didn't try the embed feature).