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

ParseOptions: Add read_tags #251

Closed
Serial-ATA opened this issue Sep 12, 2023 · 7 comments · Fixed by #406
Closed

ParseOptions: Add read_tags #251

Serial-ATA opened this issue Sep 12, 2023 · 7 comments · Fixed by #406
Labels
enhancement New feature or request good first issue These issues are a good way to get started with Lofty help wanted Extra attention is needed
Milestone

Comments

@Serial-ATA
Copy link
Owner

Summary

Unexpectedly, many projects pull in Lofty solely for its property reading. Each time it is used for this purpose, however, all of the tags are still being parsed unnecessarily.

Just as one can skip property reading with ParseOptions::read_properties(false), it may be worth adding ParseOptions::read_tags().

API design

impl ParseOptions {
    pub fn read_tags(&mut self, read_tags: bool) -> Self;
}
@Serial-ATA Serial-ATA added enhancement New feature or request help wanted Extra attention is needed good first issue These issues are a good way to get started with Lofty labels Sep 12, 2023
@vincehi
Copy link

vincehi commented Jun 8, 2024

hello, it would be great to be able to do ParseOptions::read_tags(false). Or wouldn't we have to use a [features] for this? Because in my case I wouldn't need the tags at all.

@Serial-ATA
Copy link
Owner Author

@vincehi Hello!

There wouldn't need to be a feature for this. Since read_properties already exists, this would act the same. When parsing, the tags will just be skipped and stored as their defaults (Id3v2Tag::default(), etc.).

@vincehi
Copy link

vincehi commented Jun 8, 2024

Does this seem complicated to you? I preferred to use taglib even though it also analyses tags even though I don't need it, but it doesn't crash on the file format (UTF-...). On the other hand, the length recovered is in milisecond on lofty, which is very convenient for me.

@Serial-ATA
Copy link
Owner Author

Serial-ATA commented Jun 8, 2024

Does this seem complicated to you?

No, it'd be a pretty quick feature to add. I just haven't bothered since I've been working on more requested features.

it doesn't crash on the file format (UTF-...).

Can you make issues for any crashes you have with Lofty? There haven't been any text encoding issues that I'm aware of.

@vincehi
Copy link

vincehi commented Jun 8, 2024

I think I had the same problem as this person #373. But if I understand correctly, it's on the read tags, not on the read properties. So if in future I could bypass the read tags that would be great.

@Serial-ATA
Copy link
Owner Author

Yeah, this would avoid the error in #373 entirely. I'll see about getting this in 0.21.0.

Serial-ATA added a commit that referenced this issue Jun 11, 2024
This makes it possible to use Lofty exclusively for its property reading, which many projects do at this point.

closes #251
@Serial-ATA Serial-ATA added this to the 0.21.0 milestone Jun 11, 2024
@vincehi
Copy link

vincehi commented Jun 12, 2024

Thank's for this option, it's realy fast 🚀
https://github.com/vincehi/pulp/releases

Serial-ATA added a commit that referenced this issue Jul 3, 2024
This makes it possible to use Lofty exclusively for its property reading, which many projects do at this point.

closes #251
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue These issues are a good way to get started with Lofty help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants