lofty-rs icon indicating copy to clipboard operation
lofty-rs copied to clipboard

ParseOptions: ignore `Picture` setting

Open hinto-janai opened this issue 1 year ago • 6 comments

Consider the following situation:

You have 10,000 audio files you want to probe, totaling to around 500 albums:

for file in files {
    let probe  = lofty::Probe::new(file)
    let tagged = probe.guess_file_type()?.read()?;

    // Do stuff with TaggedFile.
}

Currently, this fully scans every file, including the Picture bytes, meaning: every single file allocates a Vec for the picture bytes, even if we have already scanned them, or don't want them in the first place (e.g, we only want the core tag metadata).

I'd like to avoid allocating bytes that I already have, so my proposal:

for file in files {
    let options = lofty::ParseOptions::new().read_picture(false); // This doesn't exist.
    let probe   = lofty::Probe::new(file).options(options);
    let tagged  = probe.guess_file_type()?.read()?;

    // Do stuff with TaggedFile (that doesn't have a Picture).
}

After testing around, this cuts probing time in half for files with non-trivial picture data (1200x1200+).

I'm willing to submit a PR for this, but I was wondering if this ParseOptions::read_picture() API was the correct way forward, or if this should be done another way. It would add another branch each file parse since every version of read_from() would have this added:

if parse_options.read_picture {
    // Push picture bytes.
}

hinto-janai avatar Apr 12 '23 16:04 hinto-janai