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

Tag reading failure for some M4A files #151

Closed
LudekStoklasa opened this issue Oct 4, 2018 · 8 comments

Comments

Projects
None yet
2 participants
@LudekStoklasa
Copy link

commented Oct 4, 2018

Hi, I wonder why this M4A file fails to read tag completely?

Tested also in iTunes, Windows Explorer, Windows Media Player, MediaMonkey, AIMP and all the apps read the tag just fine.

M4A related: #78, #74

@Borewit Borewit self-assigned this Oct 4, 2018

@Borewit Borewit added the bug label Oct 4, 2018

@Borewit

This comment has been minimized.

Copy link
Owner

commented Oct 4, 2018

Thanks for reporting the issue @LudekStoklasa, I will look into it.

@Borewit

This comment has been minimized.

Copy link
Owner

commented Oct 4, 2018

I find it interesting that those applications have no issues.
But is not just music-metadata, because these application (which are very reliable in my experience) are not able to extract metadata neither:

  1. foobar2000
  2. Mp3tag

I concur iTunes reads the tags just fine.

If I look into the file, it really looks to me the file contains an encoding mistake.

At position 63592 the length of the 'mdat' atom is stored, which is 14065434 (00 D6 9F 1A).
However I think the length should be 14065439 (+5), the 1A should be 1F.
I gave it try, then it will read the following 'mean' atom correctly and will parse just fine until the end of the file.

Under the assumption it is an encoding error, I don't directly see how I go around it.

@Borewit

This comment has been minimized.

Copy link
Owner

commented Oct 4, 2018

I found something, to be continued...

Borewit added a commit that referenced this issue Oct 4, 2018

Borewit added a commit that referenced this issue Oct 4, 2018

#151 Increased tolerance for Atom identifier (based on Four-CC identi…
…fier), allow zeroes (except first character) and dashed.

Borewit added a commit that referenced this issue Oct 4, 2018

Borewit added a commit that referenced this issue Oct 4, 2018

@Borewit

This comment has been minimized.

Copy link
Owner

commented Oct 4, 2018

Thanks to [this too]l(https://www.bento4.com/downloads/) I was able to figure out how the other indented structure looked like:

[ftyp] size=8+16
 major_brand = mp42
 minor_version = 0
 compatible_brand = mp42
 compatible_brand = isom
[free] size=8+63559
[mdat] size=8+14065426
[-...] size=8+67  <=== which is a dash with 0x00, 0x00, 0x00. Apparently that should be read as a valid ID. 
[moov] size=8+138376

Scary, it is almost if there are 2 parallel structures. In that dashed tag, there was another tag I could jump in as I did in the fix.

Thank you for this challenge, I enjoyed it.

Borewit added a commit that referenced this issue Oct 4, 2018

@Borewit

This comment has been minimized.

Copy link
Owner

commented Oct 4, 2018

Fixed in v3.1.2, enjoy.

@Borewit Borewit closed this Oct 4, 2018

@LudekStoklasa

This comment has been minimized.

Copy link
Author

commented Oct 4, 2018

Thanks so much for a quick fix!
I can confirm it is working now! ;-)

LudekStoklasa referenced this issue in mediamonkeyserver/mms Oct 4, 2018

@LudekStoklasa

This comment has been minimized.

Copy link
Author

commented Oct 5, 2018

BTW: Did you consider just skipping such a non-standard and unknown atoms? It seems that the other apps are doing it? This way they are able to read the tag even when there is an unknown atom written by a third party app?

@Borewit

This comment has been minimized.

Copy link
Owner

commented Oct 5, 2018

Unknown atoms are ignored.
Problems was that the atom header was considered to be invalid. More specific the FourCC identifier. Ofcourse I can accept any binary value, issue is, if the identifier contains garbage, it is an indication I am reading something which is not the FourCC, at the parser is completely out-of-sync. At this point, all of the following atoms cannot be read neither.
I am not sure the FourCC atom identifiers in your sample are valid, but they are tolerated by music-metadata after the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.