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

Issues with ID3 ordering of APIC frames #436

Open
phw opened this issue Dec 19, 2019 · 6 comments
Open

Issues with ID3 ordering of APIC frames #436

phw opened this issue Dec 19, 2019 · 6 comments

Comments

@phw
Copy link
Collaborator

phw commented Dec 19, 2019

The ID3 classes store frames sorted by some frame ID priority, then frame size and hash.

This causes some issues when storing APIC frames of different type. The user might expect certain image types, such as "Leaflet page" to be saved in a certain order. Further more images of type "Cover (front)" might be expedted first. In regards to sorting images of different types sorting them by data length does not make too much sense.

For the second issue I found actually a concrete example: Previews in the Nautilus file browser seem to always use the first APIC frame no matter the type. So files don't necessarily get shown with their actual cover but just with the smalles APIC frame.

At a minimum I propose to change the sorting to:

  • Keep same frame IDs together
  • For APIC frames, sort type 3 "Cover (front)" first

But I think it might be even better to generally leave APIC frames in the order they were provided. @lazka Thoughts?

@lazka
Copy link
Member

lazka commented Dec 19, 2019

The last change to this was afair f07a6f3

I'd prefer the other clients getting fixed, but I'm open to changes as long as they are not too intrusive.

@phw
Copy link
Collaborator Author

phw commented Dec 19, 2019

I see. I think the proper way to handle this is storing APIC last then, but don't reorder the APIC frames themselves.

I'd prefer the other clients getting fixed, but I'm open to changes as long as they are not too intrusive.

It's not just about broken clients but also about not re-ordering images. I'll try to come up with a not too intrusive patch :D

The last change to this was afair f07a6f3

That change is interesting, did not know some clients fail reading after APIC. Btw, there can also be the other issue of software failing if images are very close to the beginning. E.g. Windows Explorer fails to read tags from FLAC if the VComment block is not close enough at the start of the file, which can happen if the pictures are stored before the VComment. In Picard we added a workaround for this by assuring the pictures go behind the VComment. I have not reported this for mutagen, but would this be something you would consider for inclusion if I provide a mutagen patch?

@lazka
Copy link
Member

lazka commented Dec 19, 2019

I see. I think the proper way to handle this is storing APIC last then, but don't reorder the APIC frames themselves.

👍

It's not just about broken clients but also about not re-ordering images. I'll try to come up with a not too intrusive patch :D

I'd say clients depending on an order are broken clients. re patch, on a second thought, feel free to fix however you want (we could even have a priority property for the frames..)

That change is interesting, did not know some clients fail reading after APIC. Btw, there can also be the other issue of software failing if images are very close to the beginning.

I'm still not sure if its things just stopping at APIC because they can't support it, or memory requirements for APIC making it stop or they are only reading a fixed max size of the whole tag in general and then the parser can't continue at some point. Either way, having APIC last should cover 99% of those cases for real files I guess.

In Picard we added a workaround for this by assuring the pictures go behind the VComment. I have not reported this for mutagen, but would this be something you would consider for inclusion if I provide a mutagen patch?

Sounds good to me.

@ben-xo
Copy link

ben-xo commented Sep 3, 2020

I am also experiencing this issue. After updating tags on some files, i found that the Serato "wave preview" image was what iTunes started to interpret as the cover art. Ideally, ordering should be preserved.

I agree that iTunes is broken - in oh so many ways - but it's rather a big client to upset.

@phw
Copy link
Collaborator Author

phw commented Jun 19, 2023

This issue is very similar to #339

@okeefo
Copy link

okeefo commented Feb 28, 2024

I'm finding it frustrating that the APIC frames are re-orded by size, with the smallest as first in the list, no matter what order I add them. DJ media players like denon dj sc6000m only show the first work in the list. So, for me, that means, if i want multiple artwork I have to do this outside my application and use Mp3Tag or write this part in another language.

I would really like to have the option to store them in the order in which set them and retrieve them back in the same order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants