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

ID3v2: URIs in APIC frames are not parsed #247

Open
FriederHannenheim opened this issue Aug 4, 2023 · 4 comments
Open

ID3v2: URIs in APIC frames are not parsed #247

FriederHannenheim opened this issue Aug 4, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@FriederHannenheim
Copy link
Contributor

ID3v2 allows for URIs to the image in the APIC tag. This is currently ignored and will not parse the image.
https://mutagen-specs.readthedocs.io/en/latest/id3/id3v2.4.0-frames.html#apic

I thought about implementing this functionality myself but the URI could point to a file or a image on the web. Since I don't think lofty should connect to the internet on it's own I would suggest to just return the URI.

@FriederHannenheim FriederHannenheim added the enhancement New feature or request label Aug 4, 2023
@Serial-ATA
Copy link
Owner

I've always put off supporting those, didn't think anyone even used them, especially with them being discouraged by both the ID3v2 and FLAC picture specs.

To support this Picture could be made into an enum to support both embedded and external images. The docs for the external variant should just instruct the caller to use Picture::from_reader after looking it up. We of course will not do that ourselves.

It'd be nice to only make a special case on AttachedPictureFrame, but if we're going to support it at all, it should also extend to FLAC pictures as well since it (sadly) took after ID3v2 as well.

I don't have any time to work on this at the moment. Would you be interested in working on this approach?

@uklotzde
Copy link
Contributor

uklotzde commented Aug 4, 2023

The URI is the actual metadata. Fetching any content referenced by an URI is out of scope.

@Serial-ATA
Copy link
Owner

The URI is the actual metadata. Fetching any content referenced by an URI is out of scope.

It may or may not be worth it to have a special case for the external images to make them nicer to work with. They work currently, but with MimeType::Unknown("-->") and the URI as a Vec<u8>, making it no more than a Vec -> String conversion.

@uklotzde
Copy link
Contributor

uklotzde commented Aug 4, 2023

I agree that mapping URI data from Vec<u8> to String might be more convenient. But this is the maximum lofty should try to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants