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

Use metadata_god for getting song metadata instead of on_audio_query #101

Merged
merged 2 commits into from
Jun 19, 2023

Conversation

FriederHannenheim
Copy link
Contributor

@FriederHannenheim FriederHannenheim commented Jun 6, 2023

This implements #99. And fixes #100, #82 and #70. The debug build I tried recognizes my entire library correctly, with year tags for FLAC and Id3v2.4 showing the right value.

This currently uses my own fork of metadata_god which uses lofty-rs for getting metadata. Here's the pull request into metadata_god KRTirtho/metadata_god#17.

Sadly lofty-rs does not support the album artist tag, which is replaced by the artist instead. (I don't think it's a huge thing but there might be some Use-Case for album artist which I am not seeing)

This is now fixed. Album Artist is supported

The downsides:

  • Library updates tend to take a bit longer now, I have a library of 1200 songs and it took about one and a half minute for the initial scan
  • Album Covers are not resized. With on_audio_query you could request a image size for the album cover while you get the full size image with metadata_god

Both of these issues can be improved. This is a draft pull request just to get opinions in. I think the local_music_fetcher_impl can be rewritten to make library updates faster but didn't want to do it yet

@moritz-weber
Copy link
Owner

moritz-weber commented Jun 10, 2023

Thank you for your contribution. The missing album artist tag would have been a dealbreaker for me, so it's great that this issue is fixed now. My use case is that I just don't like to have entries like "Artist A feat. Artist B" in my list of artists ;)

I'll try this out over the next couple of days to see if we can merge your version. The longer load times are ok for me, personally. I'm not sure how the album covers affect the app performance, but I'm sure that there is a way to resize them manually.

EDIT:
Just tried it, and there are many issues with my library. I have empty album titles, empty artists (for albums and songs), empty song titles, and unrecognized album covers. On a second look, also wrong song durations and missing album years.

@moritz-weber
Copy link
Owner

I found a few patterns for the issues here. Since this is your fork of metadata_god, I'm not really sure where to open an issue, so I'll just sum up my findings here.

  • song title, album title, artist name are broken if they contain a / (unfortunate for AC/DC)
  • the year seems to be null for songs with special characters; examples in my library:
    • Devil’s Night
    • Façade
    • Breathe Into Me (Remix Acústica)
  • missing duration: no idea
  • missing artwork: no idea

All my files are MP3 with different versions of ID3 tags. I couldn't find a clear correlation between the version and the two missing features.

@FriederHannenheim
Copy link
Contributor Author

Hmm weird. Can you please go into specifics on the files / tags you're having problems with? My library consists of mostly id3v2.3 and id3v2.4 and some opus tags and I have seen none of the issues you're describing.

@FriederHannenheim
Copy link
Contributor Author

To debug this I tagged Thunderstruck by AC/DC with id3v1, id3v2.3, id3v2.4 and opus all with a different album name so the data wouldn't be merged. None of them had this issue. Maybe it is an encoding error. Could you please try to extract the id3 tag of a file with this issue in a hex editor and paste it here so I can debug it?

@FriederHannenheim
Copy link
Contributor Author

Okay I just reinstalled the app and deleted all data and I got this issue too. Give me some time to debug this

@FriederHannenheim
Copy link
Contributor Author

Ok here's what's happening:
When I was developing this I was using the metadata_god package I had checked out locally. This meant there where binaries built for android in the directory.
Before I committed my changes I changed the metadata_god package source to my repo in the pubspec.yaml. My repo contains no binaries. I built it again to be sure that it works and the app built and opened fine so I assumed that it was okay.
What I didn't know was that metadata_god will download the missing binaries from the github releases. That meant the old version, which doesn't include my changes was being used. That's why all those issues occurred.

Now I tired getting my own binary into the app but I'm not really sure how it works and I don't know how flutter builds packages so I have given up for now. I have sent an email to the metadata_god maintainer and hope he can help me.

I will get back to you once I've figured out how to properly test Mucke with my changes.

@KRTirtho
Copy link

Hey, @FriederHannenheim sorry for the trouble you've gone through

The reason why metadata_god download binary from is that pub.dev still doesn't support native assets. And the macOS/iOS xcframeworks are static libs thus they weigh a lot. The maximum upload size of pub.dev is 100MB but the native assets (.so/.dll/.xcframework files) adds up over 100MB thus I've to use a github actions pipeline to publish specific version of binaries in the releases and download + cache it for that version in the build phase of flutter app

So in all CMakeLists.txt and metadata_god.podspec replace https://github.com/KRTirtho/metadata_god with your repo URL
And dispatch/run this github action: https://github.com/<your-user-name>/metadata_god/actions/workflows/create-release.yml which will use melos to bump version based on conventional commits and create a new tag with that new version

After it run: https://github.com/<your-user-name>/metadata_god/actions/workflows/publish-release.yml which will build and create a new release publish using latest tag and will upload the binaries to that release

PS: Replace with actual username

@FriederHannenheim
Copy link
Contributor Author

@moritz-weber It's working now. Please test again. Delete all App data of the old app and then just run flutter run

@moritz-weber
Copy link
Owner

This looks really, really good. I don't see any of the issues from before and the loading time is moderate as well for me (around 2 min for 3400 songs). And since unchanged songs are skipped in consecutive scans, it should be quite fast, if someone adds a couple of albums to the library.

Thanks for this amazing work. I'll test drive it for a few days and try to finish a release by next Tuesday.

@FriederHannenheim
Copy link
Contributor Author

@KRTirtho Thanks for your help

@moritz-weber moritz-weber merged commit d3b4361 into moritz-weber:master Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ID Tag Track No with leading "0" fails
3 participants