aadsm / jsmediatags

Media Tags Reader (ID3, MP4, FLAC)
Other
745 stars 128 forks source link

Not able to read FLAC tags from certain files. #90

Closed aadsm closed 3 years ago

aadsm commented 6 years ago

The example file and original report is in https://github.com/captbaritone/winamp2-js/issues/461.

enjikaka commented 6 years ago

Also stumbled upon this now!

enjikaka commented 6 years ago

Using dist/jsmediatags.js instead of dist/jsmediatags.min.js fixes the issue for me. :) Must be something destructive in creation of the minified version!

Sanctarium commented 5 years ago

Same problem. Is there any else way to solve it?

Borewit commented 4 years ago

Under the assumption you use it in the browser, music-metadata-browser will do the job.

In node.js use music-metadata.

AllAstronauts commented 3 years ago

If anyone cares, you can find the actual problem explained by reading the closed Media tags not working cybercase issue above this comment. The problem is that some FLAC tags are not TITLE, but Title or title instead.

jsmediatags is running a switch case on these with only ALL CAPS, so it's missing the FLACs that have them with all lowercase or first-letter-capitalized and the rest lower. I just edited it to add the necessary checks (pull requests have gone unanswered here for awhile so I'm not going to bother.)

jsmediatags/src/FLACTagReader.js - find the _parseData function and replace the case statements there with this:

switch (split[0]) { case "TITLE": case "Title": case "title":
title = split[1]; break; case "ARTIST": case "Artist": case "artist": artist = split[1]; break; case "ALBUM": case "Album": case "album": album = split[1]; break; case "TRACKNUMBER": case "Tracknumber": case "tracknumber": track = split[1]; break; case "GENRE": case "Genre": case "genre": genre = split[1]; break; }

You can see its merely appending two additional case lines for the Title and title options (and so on down the line)

Tested working with one of my FLACs that caused me to look into this.

Most of you probably just want this to work and are not looking to fork and compile, in which case just go to the dist directory, copy out the jsmediatags.js file (not the minified one) and make the edits there, its fairly near the top. Save and there you go or run it through your own online minifier of choice if you need/want a min version. Adventurous individuals can look at one of the lingering pull requests here for adding missing tag fields to FLACs - since you are already editing the file you may wish to insert those changes in as well - equally as easy as this edit.

And thanks to @aadsm hope things are well for you. The music-metadata-browser option someone posted above is much more expansive, but also comes with a lot more dependencies and a different use-flow. This js is pleasantly lean and fits my use case quite well.

aadsm commented 3 years ago

@AllAstronauts oh wow, I didn't receive any mail of the past PRs on this repo 😧. I did receive a mail for this comment though so decided to check those claims about unanswered pr's 😅 , and realized I had no idea about them 😬. Please do a PR if you're still interested and I'll merge it in and will release a new version. Tomorrow or Monday I'll go through the other open PRs. Thanks for bringing this to my attention!

AllAstronauts commented 3 years ago

lol - so that's why - crazy

I could do the pull requests, but you can see they are brain dead changes (so is that pull with the added FLAC fields).

A case change to upper on the var before the switch cases would be more practical and you can skip a few (all) of of those multiple case additions I made - I just smacked the thing to get it working - nothing elegant or thought out on my end :smile:

I'd just assume let you deal with these minor edits :smile:

Glad you are doing alright!

aadsm commented 3 years ago

Finally fixed in 3.9.5!