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

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

Open Boscop opened this issue 5 years ago • 12 comments

https://github.com/RustAudio/rodio/issues/293 next_frame seems to always return Some.

Boscop avatar Jun 17 '20 18:06 Boscop

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?

lieff avatar Jun 19 '20 18:06 lieff

@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.

Boscop avatar Jun 19 '20 20:06 Boscop

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?

lieff avatar Jun 19 '20 21:06 lieff

@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).

Boscop avatar Jun 20 '20 01:06 Boscop

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.

lieff avatar Jun 20 '20 09:06 lieff

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

Boscop avatar Jun 20 '20 20:06 Boscop

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.

lieff avatar Jun 20 '20 20:06 lieff

@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? https://github.com/RustAudio/rodio/issues/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.

Boscop avatar Jun 21 '20 13:06 Boscop

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 avatar Jun 21 '20 14:06 lieff

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 https://github.com/germangb/minimp3-rs/issues/21 as well.

lieff avatar Jun 21 '20 14:06 lieff

@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!

Boscop avatar Jun 21 '20 23:06 Boscop

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

lieff avatar Jun 22 '20 08:06 lieff