Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Direct metadata parsing #128

Closed
OxygenCobalt opened this issue May 5, 2022 · 17 comments
Closed

Direct metadata parsing #128

OxygenCobalt opened this issue May 5, 2022 · 17 comments
Labels
enhancement New feature or request music Related to music loading

Comments

@OxygenCobalt
Copy link
Owner

OxygenCobalt commented May 5, 2022

This issue is about the addition of an option that makes the music loader rely on the ExoPlayer metadata engine. Previously, I've rejected this feature as there were two main limitations with it:

  1. I wanted to rely on ExoPlayer for audio metadata if I were to extract it directly, however it's implementation is absurdly slow.
  2. Using external libraries was a no-go, as I'd either have to go native or use something like JAudioTagger which isn't really designed with android's filesystem limitations in mind.

Recently however, I've discovered that I could parallelize ExoPlayer's metadata extraction routine, which reduces manual extraction times from ~58ms per song (1000 songs -> 1 minute, 10000 songs -> 9 minutes) to only ~16ms (1000 songs -> 17 seconds, 10000 songs -> 3 minutes), which is a massive improvement.

Adding this would fix an uncountable amount of OEM-specific bugs, albeit the cost of a longer music loading process. Since the latter is still somewhat unfriendly, this will be an option and not a default feature.

Currently, this is blocked by #72, as the current hack we use to reload music will cause any setting for this feature not to be written.

@OxygenCobalt OxygenCobalt added enhancement New feature or request music Related to music loading labels May 5, 2022
@OxygenCobalt

This comment was marked as outdated.

OxygenCobalt added a commit that referenced this issue May 29, 2022
Add an ExoPlayer-based metadata backend.

This backend finally allows Auxio to parse metadata internally, which
has been a highly requested feature given how many unfixable MediaStore
issues it would fix.

However, this is not fully ready for production yet. The loading
process becomes much larger with manual parsing, the current
implementation still allocates picture metadata (terrible for
efficiency), and there is no good indicator in the app to keep
track of music loading.

As a result, this backend is disabled until it can be fully integrated
into Auxio's architecture.
@OxygenCobalt
Copy link
Owner Author

OxygenCobalt commented Jun 2, 2022

If anyone wants to try out an extremely experimental APK with this new system, I've attached a debug APK below.

Auxio_Ignore_MediaStore_Tags.zip

No guarantees that there are no strange bugs with the new system.

@2ndemosthenes
Copy link

Works fine for me, also I think ExoPlayer supports opus files but Auxio doesn't show them.

@OxygenCobalt
Copy link
Owner Author

OxygenCobalt commented Jun 4, 2022

What Android version are you running @2ndemosthenes? I know OPUS support is flaky on older versions of android.

@2ndemosthenes
Copy link

What Android version are you running @2ndemosthenes? I know OPUS support is flaky on older versions of android.

I'm on android 8, and yes most music players can't find opus files on my phone, only a handful of them support it.
I just thought the change to ExoPlayer may change things.

@OxygenCobalt
Copy link
Owner Author

OxygenCobalt commented Jun 4, 2022

Yeah, might be device-specific. I would imagine that the music players that do work don't load music from the media database (which isn't picking up your OPUS files) but rather using their own system which I can't really replicate.

@OxygenCobalt
Copy link
Owner Author

OxygenCobalt commented Jun 10, 2022

I've been able to implement a really cool thing with the new parser: Original date support. If you have an album like "Famous Album (Remastered)" that was originally released in 2000 but was re-released in 2020, you could tag the file to have an "original date" tag (TORY/TDRC on ID3v2 and ORIGINALDATE on vorbis) of 2000, and "date" tag of 2020. While Auxio currently reads the date as 2020, with the ExoPlayer parser, the date will now be 2000.

The only reason I implemented this was from reading https://news.ycombinator.com/item?id=31659799 and thinking it was really useful. If you want me to add support for more niche tags like these, please let me know.

@ghost
Copy link

ghost commented Jun 12, 2022

@OxygenCobalt, maybe you could add support for "Release Type" tags in order to separate studio/live albums, eps, and singles inside each artists' page.

Though, I'm a bit conflicted as to which tags would be used for such a feature. MusicBrainz (Picard) defines this specific type of tag here, but it isn't a native/official ID3 frame. Using the grouping tag (also defined for Picard here) might be more convenient, but its use isn't very standardized between applications (some use it for box sets, others for release groups, and iTunes uses it in a specific manner I haven't researched yet).

@OxygenCobalt
Copy link
Owner Author

OxygenCobalt commented Jun 12, 2022

Huh, that actually sounds cool @rhynec, as I have some EPs that I would like to categorize. Some things though:

  • How is the MusicBrainz/Vorbis RELEASETYPE tag formatted? GRP1 is out of the question, as it is actually an iTunes extension that basically has no standard format.
  • Should EPs, Singles, and Albums still be grouped the same? My idea is to separate them in the artists menu, but in the "Albums" tab things become much more vague. I would imagine that I add a little indicator next to the album item in order to signify what kind of release it is.
  • In the case that ExoPlayer parsing is disabled, this tag cannot be extracted, so I would need to gracefully degrade into the current album UI. This is pretty easy though.
  • Do all songs in an album apply this tag? Instead of aggregating songs, Auxio picks the song with the earliest year to build the album, which might cause issues if only one song has the tag applied.

I'll try to implement this some time once I can get an MVP of this system out (and I can make the necessary UI reworks)

@OxygenCobalt
Copy link
Owner Author

OxygenCobalt commented Jun 12, 2022

Also, something else I'm going to try implementing is full ISO-8601 date support. If you have special "YYYY-MM-DD" or even "YYYY-MM-DD HH-MM-SS" tags, Auxio would handle them correctly when sorting, instead of sorting them alphabetically like Auxio does now. Note that they'll probably still be displayed in the UI as a simple year.

@illdeletethis
Copy link

suggestion for displaying the different release types: one list as it is now, but different sizes of cover artwork

so album would extend all the way to the left screen edge, and into the empty space on top and bottom of the line.

ep and various other stuff at current size

single and demo smaller

@OxygenCobalt
Copy link
Owner Author

@illdeletethis I don't really want to make sizing inconsistent like that. Material Design prefers you to size components by 8dp increments, but there are no 8dp increments between the size of a song cover and the size of an album cover. I think I'll still separate them in the artist view, but not in the albums tab (I'll differentiate them somehow).

@ghost
Copy link

ghost commented Jun 15, 2022

Hey @OxygenCobalt, sorry for taking so long in getting back to you. This week's been crazy for me.

  • How is the MusicBrainz/Vorbis RELEASETYPE tag formatted? GRP1 is out of the question, as it is actually an iTunes extension that basically has no standard format.

I'm not too sure about ID3 frames, but for Vorbis it should be UTF-8 formatted text with the different options listed here. I'm not sure how the "secondary" types would be written into the tag by Picard, though. I'd guess they're either written in separate RELEASETYPE tags twice (which Vorbis allows), or as a single type written as "<Primary> + <Secondary>", like shown for this release. I think that it'd be better in your implementation to only read the first tag it finds, and not worry about additional RELEASETYPE tags. Also, it may be best to allow for any release type rather than look for the finite amount of types defined by MusicBrainz. This would give more flexibility to each user to group albums as they please (for example, bootlegs aren't defined as a release type by MusicBrainz, which is a tag I think many users would like to use).

  • Should EPs, Singles, and Albums still be grouped the same? My idea is to separate them in the artists menu, but in the "Albums" tab things become much more vague. I would imagine that I add a little indicator next to the album item in order to signify what kind of release it is.

I like this implementation. Though, maybe only show the release type inside each album's menu rather than cluttering up the listing with that information?

  • Do all songs in an album apply this tag? Instead of aggregating songs, Auxio picks the song with the earliest year to build the album, which might cause issues if only one song has the tag applied.

I would assume all songs would be tagged with the release type. At least, that's the way I'd do it since tagging an arbitrary song in an album with the release type wouldn't make much sense.

Also, something else I'm going to try implementing is full ISO-8601 date support.

That's very cool! There are bands like KGLW who have released up to five albums per year, so using the full date for sorting would be something I'd very much appreciate.

One more thing. Perhaps it'd be better to move the release type discussion into a separate issue for more visibility?

This was referenced Jun 15, 2022
@OxygenCobalt
Copy link
Owner Author

OxygenCobalt commented Jun 15, 2022

Okay, thanks for the formatting info @rhynec.

I'm not too sure about ID3 frames

For ID3v2, I'm just going to use the MusicBrainz tag, and then fall back to GRP1. I'll assume that it's formatted similarly.

I'd guess they're either written in separate RELEASETYPE tags twice (which Vorbis allows), or as a single type written as " + ".

think that it'd be better in your implementation to only read the first tag it finds, and not worry about additional RELEASETYPE tags.

In general, due to some files that have both ID3 and Vorbis tags, the source of truth for a particular tag will always be the last one Auxio finds. In the case that there are separate tags, I would simply reccomend using the + syntax. It's not like duplicate vorbis tags are that common, anyway, and I'd imagine using them would be quite the hassle in other taggers like Kid3/Mutagen given that they both use HashMaps.

I like this implementation. Though, maybe only show the release type inside each album's menu rather than cluttering up the listing with that information?

Also, it may be best to allow for any release type rather than look for the finite amount of types defined by MusicBrainz.

My rule regarding items is this:

  • If the tag is definite, as in it is short or represented by a string resource, it can be combined with other definite tags. This includes years, song counts, and the (finite) release types.
  • If the tag is indefinite, as in it can be any length, including pretty long, it must be on one line as to prevent the line from being ellipsized often. This is why I removed the "Artist / Album" formatting in a recent version, as it ellipsized too often for it to provide a good experience.

So, when it comes to my idea, it wouldn't clutter the list of albums too much since I would consider release types to be a definite tag. Now, this changes if the release type could be arbitrary. Then, it would have to be on one line, which I don't like.

Arbitrary release types would also make it much harder to group up albums in the Artist UI. Whereas I could have built-in string resources for "Albums", "EPs", "Singles", I can't do that with a flat tag like bootleg. Either I do something stupid to pluralize it, or I just leave it as-is.

I'll consider it some other time if I choose to extend the Release Type system. Perhaps I should just extend the spec depending on user requests. Currently though, I just want to deliver an MVP as it stands.

Anyway, this is going to be spun off into two issues: #158 for release types, #159 for the dates.

@OxygenCobalt

This comment was marked as outdated.

@OxygenCobalt
Copy link
Owner Author

OxygenCobalt commented Jul 7, 2022

Actually, this might not arrive in 2.5.0. I don't want to be overwhelmed with bugs from both #72 and this issue in a single release, so I think I'll try to bundle this (and all the new metadata additions) into a 2.6.0 release.

@OxygenCobalt
Copy link
Owner Author

Now that 2.5.0 has been released, I've re-enabled this feature in the developer branch. It should arrive in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request music Related to music loading
Projects
None yet
Development

No branches or pull requests

3 participants