search

Rachel030219 / Poweramp-LRC-Plugin

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

[BUG] Embedded Lyrics issues #26

Closed fei0316 closed 3 years ago

fei0316 commented 4 years ago

Describe the bug

  1. Reports error when music format is not mp3
  2. Does not consider .lrc files when lyrics tag is empty
  3. Does not consider LYRICS field, used by some music apps (MusicBee)

To Reproduce

  1. Play a music file that is not in mp3 format
  2. Play an mp3 file that does not have lyrics/unsyncedlryics tag, but has .lrc file next to it
  3. Put lyrics data in LYRICS field, instead of UNSCYNEDLYRICS field in mp3 file.

Device info:

Additional context Please see log for the first problem. I believe it can be solved by first checking if it's mp3 before proceeding. Alternatively, you could consider supporting file formats other than mp3. I found these libraries: https://github.com/ealva-com/ealvatag, http://www.jthink.net/jaudiotagger/ The last two (especially the last one) are probably expected behavior, but I believe fixing it would make the app usable to more people, thank you! 20201017-104615.log

Rachel030219 commented 4 years ago

Thanks! I'll check it out.

Rachel030219 commented 3 years ago

The first problem should've been fixed by switching to Jaudiotagger.

But the third problem might not get fixed until there is a better solution. I've written the "SYNCHRONISEDLYRICS" tag to one of my songs and tried to put the Jaudiotagger to work, which turned out that although AutomaTag and Poweramp both could detect the lyrics, the library does not recognize them,, even if I specified the field key "SYNCHRONISEDLYRICS".

As for the second one, it'll get fixed ASAP, which would behave like: when embedded lyrics is disabled, the plugin only looks for the external lyric file. Once enabled, the plugin parses embedded lyrics first, and tries to find external lyric file if embedded lyrics are not available.

fei0316 commented 3 years ago

I did more research on the third problem (should've done it before I report the issue - sorry!), and apparently no one uses the LYRICS field.

So there are two types of lyrics field in ID3 (MP3): USLT and SYLT.

USLT is what we refer to as UNSYNCEDLYRICS, which is what everyone (Poweramp, MusicBee, Mp3tag, foobar, mp3agic) uses. It was intended to be used for lyrics with no timestamp, but people just embed the contents of lrc files in this field, and many apps would recognize it (MusicBee does, and by default saves lrc-formatted synchronized lyrics in this field). I believe this is the original plan for this feature of this app also (save lrc content in the tag, so no more lrc files necessary).

SYLT is the "intended" approach for synchronized lyrics in ID3 tags, which uses a different formatting scheme with two fields: one for time and one for lyrics. Almost nothing supports this except MusicBee, and I think its implementation is wrong anyway, as it does not offer two separate fields for time and text, like SYLT Editor does. MusicBee's implementation is probably why it got me all confused.

Since apparently no one uses SYLT (I only "used" it as I misunderstood its meaning) I believe it is safe to just ignore it altogether, and focus on the original design: lrc contents in USLT.

With that out of the way, I think using FieldKey for Jaudiotagger is sufficient for our needs then.

Rachel030219 commented 3 years ago

No problem, and thanks for your explanation!

fei0316 commented 3 years ago

I just tested your latest code with MP3, FLAC, and AAC (M4A), and all seemed to be working as expected. Thanks!