search

Serial-ATA / lofty-rs

Audio metadata library
Apache License 2.0
184 stars 34 forks source link

ParseOptions: Add `read_tags` #251

Closed Serial-ATA closed 1 month ago

Serial-ATA commented 11 months ago

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;
}
vincehi commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

I think I had the same problem as this person https://github.com/Serial-ATA/lofty-rs/issues/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 commented 2 months ago

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

vincehi commented 2 months ago

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