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

How to detect when decoder has reached end of audio file? #20

Open
Boscop opened this issue Jun 17, 2020 · 12 comments
Open

How to detect when decoder has reached end of audio file? #20

Boscop opened this issue Jun 17, 2020 · 12 comments

Comments

@Boscop
Copy link

Boscop commented Jun 17, 2020

RustAudio/rodio#293
next_frame seems to always return Some.

@lieff
Copy link

lieff commented Jun 19, 2020

next_frame seems to always return Some. So this is a minimp3 bug, not ours.

I have little experience in rust. Can you debug why

    pub fn next_frame(&mut self) -> Result<Frame, Error> {
        loop {
            // Keep our buffers full
            let bytes_read = if self.buffer.len() < REFILL_TRIGGER {
                Some(self.refill()?)
            } else {
                None
            };

            match self.decode_frame() {
                Ok(frame) => return Ok(frame),
                // Don't do anything if we didn't have enough data or we skipped data,
                // just let the loop spin around another time.
                Err(Error::InsufficientData) | Err(Error::SkippedData) => {
                    // If there are no more bytes to be read from the file, return EOF
                    if let Some(0) = bytes_read {
                        return Err(Error::Eof);
                    }
                }
                Err(e) => return Err(e),
            }
        }
    }

return Err(Error::Eof); is not triggered?
In decode_frame: is there non-zero self.buffer.len() in repeated end-frames?
If so, can you try update to latest minimp3.h?

@Boscop
Copy link
Author

Boscop commented Jun 19, 2020

@lieff When bytes_read is Some(0), decode_frame still gets called (with the previous frame's buffer content) and succeeds, so it seems like Ok(frame) is always returned (in the absence of errors).
I wouldn't call decode_frame in that case.

@lieff
Copy link

lieff commented Jun 19, 2020

Yes, I try to understand how it can happen. If Ok(frame) always returned

        if samples == 0 {
            if frame_info.frame_bytes > 0 {
                Err(Error::SkippedData)
            } else {
                Err(Error::InsufficientData)
            }
        } else {
            Ok(frame)
        }

then samples should be non-zero. It can happen only if self.buffer.len() is non-zero, so buffer not fully consumed here:

        let current_len = self.buffer.len();
        self.buffer
            .truncate_front(current_len - frame_info.frame_bytes as usize);

Can you debug and confirm that?

Also minimp3-rs/examples/example.rs seems relies on Err(Error::Eof), does it work on your side?

@Boscop
Copy link
Author

Boscop commented Jun 20, 2020

@lieff I'm not using minimp3 directly, I'm using it through rodio..
But there is no reason to call decode_frame when bytes_read is Some(0).

@lieff
Copy link

lieff commented Jun 20, 2020

But there is no reason to call decode_frame when bytes_read is Some(0).

    async fn refill_future(&mut self) -> Result<usize, io::Error> {
        use tokio::io::AsyncReadExt;

        let mut dat: [u8; MAX_SAMPLES_PER_FRAME * 5] = [0; MAX_SAMPLES_PER_FRAME * 5];
        let read_bytes = self.reader.read(&mut dat).await?;
        self.buffer.extend(dat[..read_bytes].iter());

        Ok(read_bytes)
    }

Some(0) do not means buffer is empty, Only in combination if with current buffer we have Err(Error::InsufficientData) | Err(Error::SkippedData) and we can`t read anything further (Some(0)) means Err(Error::Eof); which looks correct in code.

If minimp3-rs/examples/example.rs works with your mp3 file then it's not minimp3-rs bug after all. If you share your mp3 file - I can look at it too.

@Boscop
Copy link
Author

Boscop commented Jun 20, 2020

@lieff It happened with all mp3 files I tested..

@lieff
Copy link

lieff commented Jun 20, 2020

Not minimp3-rs bug then, can't reproduce issue with bundled mp3 test file and decoder.next_frame() successfully returns Err(Error::Eof) on eof.

@Boscop
Copy link
Author

Boscop commented Jun 21, 2020

@lieff Can you please try with this file? This file exhibits this behavior.

I think the bug is in this crate because the rodio code on top of this is very minimal and seems correct:

https://github.com/RustAudio/rodio/blob/6e2c022b5da913aedd459b78d3e9d06302c57594/src/decoder/mp3.rs#L65-L79


Btw, I have another issue, maybe this is related to this one?
RustAudio/rodio#295
Basically, even though I drain frames from the decoder iterator (to seek to a certain point in time) it doesn't advance the decoder. When I then play it with rodio, it plays from the start, as if I hadn't drained frames.

@lieff
Copy link

lieff commented Jun 21, 2020

No luck, can't reproduce:

use minimp3::{Decoder, Error, Frame};

use std::fs::File;

fn main() {
    let mut decoder =
        Decoder::new(File::open("afLb.mp3").unwrap());

    loop {
        match decoder.next_frame() {
            Ok(Frame {
                data,
                sample_rate,
                channels,
                ..
            }) => println!("Decoded {} samples", data.len() / channels),
            Err(Error::Eof) => { println!("EOF reached"); break },
            Err(e) => panic!("{:?}", e),
        }
    }
}

Outputs:

...
Decoded 1152 samples
Decoded 1152 samples
Decoded 1152 samples
Decoded 1152 samples
Decoded 1152 samples
EOF reached

@lieff
Copy link

lieff commented Jun 21, 2020

Btw, I have another issue, maybe this is related to this one?

That`s expected, minimp3-rs have filled internal buffer, so when you consume data from stream to skip frames, filled buffer coninued to decode. Also mp3 have bit-reservoir, so some frames may not decode after such seek.

Overall mp3 have complex seek procedure https://github.com/lieff/minimp3/blob/master/minimp3_ex.h#L707, minimp3-rs currently do not have one.
Here work which adds seeking decoder https://github.com/Cocalus/minimp3-rs,
may be it can help #21 as well.

@Boscop
Copy link
Author

Boscop commented Jun 21, 2020

@lieff I don't understand: Why does the decoder start over from the start again after I drained samples from it?
It behaves as if I hadn't drained any samples at all!

@lieff
Copy link

lieff commented Jun 22, 2020

I do not know, auto-rewind streams may be? Do you have minimal reproduction example?

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