-
Notifications
You must be signed in to change notification settings - Fork 19
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
Album cover failed to be written into the file #42
Comments
The cover does not show up on macOS Finder, so I suspect it's a writer issue. :'( where do you recommend me to look up ...? Maybe iTunes and Finder on Mac simply use a different layout? |
I'll look at it on macOS later (actually I didn't do that previously, so thanks for info). |
So I turned to HEX editor to inspect the files. Strangely the ID3 tags iTunes give me is in v2.2. Unfortunately, iTunes has since removed the option allow users to switch between ID3 versions. This is what the library generates, from 0th position to the image data: This is what iTunes generates: I will poke around my library and find an ID3 v2.3 file that can display picture correctly, and try to figure it out from there. |
I've managed to "fix" a file generated by the library: Here are the unchanged bytes of the frame, annotated with comments from the source code:
Here are the bytes of an APIC frame from a song from my library (which pic can be seen correctly):
Comparing both I realized there are 2 diffferences:
And indeed when I manually change and delete these bytes the picture show up on Finder. I don't understand ID3 v2.3 enough to say how to fix this the right way; if you comment on it I am happy to submit a pull request for it. |
So I looked up ID3 v2.3 section 3.3, does this means iTunes simply don't support picture type and description in an APIC frame? Does it consider a valid fix if we switch to "iTunes compatible" frame format when we found the picType = |
Correction: the APIC frame understood by iTunes should be interpreted this way:
This should be correct because I can add whatever description into the frame and change the pic type without making the cover disappear from Finder. So the "iTunes compatible" frame format is simply an APIC frame that always encodes the description in ISO-8859-1. What's the recommend fix here? Do we want to simply force the encoding of the description to ISO-8859-1? Or we could do something sane like check a string and give a warning or whatnot? Tell me what to do and I will submit a pull request. |
Here is an alternative byte sequence that can be read by iTunes
However I doubt this is confirm to spec since the description doesn't even have to be 2n bytes long. |
Nice findings, Timothy! I think we can add another parameter when set APIC frame which tells what encoding to use (by default it will be writer.setFrame('APIC', {
type: 3,
data: coverArrayBuffer,
description: 'description',
useUnicodeEncoding: true
}); And add in readme info about the issue. By the way, I don't think many people use APIC description at all. |
Would you like to work on it or I'll do it? |
@egoroof Sure, I can submit a pull request with that. |
There are some quirks with APIC frame parsing on macOS iTunes and Finder. When the description is encoded in Unicode and is either an empty string or with an non-Western character as the last character, iTunes will fail to parse the picture from the frame. This fix workaround it by always use Western encoding when the description is empty, and only opt to Unicode encoding when the caller explicitly choose to with `useUnicodeEncoding: true`. README.md is updated accordingly with a warning.
There are some quirks with APIC frame parsing on macOS iTunes and Finder. When the description is encoded in Unicode and is either an empty string or with an non-Western character as the last character, iTunes will fail to parse the picture from the frame. This fix workaround it by always use Western encoding when the description is empty, and only opt to Unicode encoding when the caller explicitly choose to with `useUnicodeEncoding: true`. README.md is updated accordingly with a warning.
I am testing with a file like this.
The picture will be encoded into the file but with a few bytes missing from the beginning, therefore it will not be decoded correctly (this is a conclusion I found from extracting the encoded bytes with ID3 Reader).
This can be reproduced with the online demo. I couldn't see obvious issues by looking at the code for 10 min, so I decided to file an issue first. Will look into it later.
Thanks for the great work!
The text was updated successfully, but these errors were encountered: