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

read_next_or_eof's API prevents buffer reuse #34

Open
DavidVentura opened this issue Apr 10, 2024 · 1 comment
Open

read_next_or_eof's API prevents buffer reuse #34

DavidVentura opened this issue Apr 10, 2024 · 1 comment

Comments

@DavidVentura
Copy link

Hi!
I'm trying to use claxon in an embedded environment and would like to reuse the decode buffer, but I'm not figuring out how to do this when I might have multiple frames to decode in a single buffer:

impl Decode for FlacDecoder {
    fn decode_sample(&mut self, buf: &[u8], out: &mut [i16]) -> Result<usize, anyhow::Error> {
        let mut fr = FrameReader::new(std::io::Cursor::new(buf));
        let mut c = 0;
        let mut v = std::mem::take(&mut self.dec_buf);
        loop {
            if let Ok(Some(block)) = fr.read_next_or_eof(v) {
                for (a, b) in block.stereo_samples() {
                    out[c + 0] = a as i16;
                    out[c + 1] = b as i16;
                    c += 2;
                }
                v = block.into_buffer();
            } else {
                break;
            }
        }
        // self.dec_buf = v;
        // can't do this, `v` was moved into  `read_next_or_eof` 
        // and both the `Err` and `Ok(None)` cases would lose the buffer
        Ok(c)
    }
}

I've made a branch with an API change which allows passing a mutable reference, but this breaks the FlacSamples and FlacIntoSamples iterators

@ruuda
Copy link
Owner

ruuda commented Apr 10, 2024

Yes, this is a limitation of the current design. For a future version, I think I would split it into a lower-level API that reads and decodes one block, with higher level iterator-like functions built on top. (For the audio data it’s not a big change, but especially for the metadata frames, right now we either have to read all of them and store them, or skip over them, there is no way to stream them and inspect only the interesting ones. For that I think a split into a lower-level API and a higher-level one makes sense.)

If you’re in a context where Vec::resize is fine though, I suppose allocating is acceptable, so maybe it’s okay to assign v = Vec::new() in the Err case?

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

No branches or pull requests

2 participants