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

Add time seek similar as ov_time_seek #73

Open
mrDIMAS opened this issue Dec 28, 2019 · 12 comments
Open

Add time seek similar as ov_time_seek #73

mrDIMAS opened this issue Dec 28, 2019 · 12 comments

Comments

@mrDIMAS
Copy link

mrDIMAS commented Dec 28, 2019

Time seeking similar to ov_time_seek would be very useful. Or maybe it is already there?

@est31
Copy link
Member

est31 commented Dec 28, 2019

There is seeking support in lewton! The seek_absgp_pg function provides page-granularity seeking in terms of absolute granule position which is IIRC just the sample number, so you use the sample rate to convert to timestamps and back. As the function is in page granularity, you'll need to advance the decoder until you get to your desired position inside the page.

@mrDIMAS
Copy link
Author

mrDIMAS commented Dec 28, 2019

Hm, I tried it as seek_absgp_pg(0) and next calls to read_dec_packet_itl just returns zero-sized vectors, I tried it few times in a row (because first packet seems to be empty everytime) and all of them were empty.

@mrDIMAS
Copy link
Author

mrDIMAS commented Dec 28, 2019

As the function is in page granularity, you'll need to advance the decoder until you get to your desired position inside the page.

Maybe it not working because of this. I'm trying to "rewind" decoder to beginning of data after I read all the contents in source by just calling seek_absgp_pg(0). What does this: you'll need to advance the decoder mean exactly? How to do that?

@est31
Copy link
Member

est31 commented Dec 28, 2019

What does this: you'll need to advance the decoder mean exactly? How to do that?

I think currently if you use the ogg reader module, the only way is to call the packet decoding functions enough times while keeping count of the number of their samples.

@mrDIMAS
Copy link
Author

mrDIMAS commented Dec 28, 2019

Not sure if I get you right, do I need to track total amount of samples I read and then when I need to seek beginning I need to do something like (pseudocode)

while current_pos > 0 {
   current_pos -= current_packet_size
   seek_absgp_pg(current_pos)
}

Also when I do seek_absgp_pg(0) I get BadAudio(AudioIsHeader) error.

@est31
Copy link
Member

est31 commented Dec 28, 2019

No you need to track the number of samples read after you call seek_absgp_pg. Seeking always brings you to the page boundary, no matter where you were before. The seek_absgp_pg function brings you roughly into the area, but the sample precise fine tuning is up to you.

Also when I do seek_absgp_pg(0) I get BadAudio(AudioIsHeader) error.

Hmm yeah it makes sense that this happens. It might be a good idea to let lewton be responsible for this so maybe I should fix it. In the meantime, you can create a completely new OggStreamReader when you want to rewind completely.

@mrDIMAS
Copy link
Author

mrDIMAS commented Dec 28, 2019

Yeah, that would be awesome if it would be fixed on library side. New OggStreamReader instance is ok for now, but in future I want to be able to seek at any time pos I need. Simplest use case: set playback position for streaming sound after loaded a saved game, this is useful for long ambient sounds that eats too much memory when fully decoded.

I'm curious why seeking produces BadAudio(AudioIsHeader) error? Because internal reader was set to a header position, not a packet position?

@mrDIMAS
Copy link
Author

mrDIMAS commented Dec 28, 2019

Ok, this solution works (I wish rewind be less ugly)

pub struct OggDecoder {
    reader: Option<OggStreamReader<DataSource>>,
    samples: vec::IntoIter<i16>,
    channel_count: usize,
    sample_rate: usize,
}

fn i16_to_f32_sample(sample: i16) -> f32 {
    sample as f32 / 32767.0
}

impl Iterator for OggDecoder {
    type Item = f32;

    #[inline]
    fn next(&mut self) -> Option<Self::Item> {
        if let Some(sample) = self.samples.next() {
            Some(i16_to_f32_sample(sample))
        } else {
            self.samples = self.reader
                .as_mut()
                .unwrap()
                .read_dec_packet_itl()
                .ok()
                .and_then(|data| data)
                .unwrap_or_default()
                .into_iter();
            self.samples
                .next()
                .and_then(|v| Some(i16_to_f32_sample(v)))
        }
    }
}

impl OggDecoder {
    pub fn new(source: DataSource) -> Result<Self, SoundError> {
        let mut reader = OggStreamReader::new(source)?;
        Ok(Self {
            samples: reader.read_dec_packet_itl()?
                .ok_or(SoundError::UnsupportedFormat)?
                .into_iter(),
            channel_count: reader.ident_hdr.audio_channels as usize,
            sample_rate: reader.ident_hdr.audio_sample_rate as usize,
            reader: Some(reader),
        })
    }

    fn get_channel_count(&self) -> usize {
        self.channel_count
    }

    fn rewind(&mut self) -> Result<(), SoundError> {
        let mut source = self.reader
            .take()
            .unwrap()
            .into_inner()
            .into_inner();
        source.seek(SeekFrom::Start(0))?;
        *self = Self::new(source)?;
        Ok(())
    }

    fn get_sample_rate(&self) -> usize {
        self.sample_rate
    }
}

@est31
Copy link
Member

est31 commented Dec 28, 2019

Wonderful that you found a solution!

Note: you can avoid doing the conversion in i16_to_f32_sample by using the read_dec_packet_generic function and InterleavedSamples<f32> instead of read_dec_packet_itl

@mrDIMAS
Copy link
Author

mrDIMAS commented Dec 28, 2019

Great, thanks! Still API of OggStreamReader requires some improvements, my very first thought at beginning was - "wow, it is cool, I hope it will have similar API as libvorbis", but then I realized that is lacking high level functionality like seeking at time pos, which was a bit frustrating. Anyway, thanks for the great library!

@ghost
Copy link

ghost commented Dec 28, 2020

Should this function be included into the library's API instead of having the user implement it themselves?
If it should, I'll try to make a PR.

@TonalidadeHidrica
Copy link

TonalidadeHidrica commented Dec 28, 2020

@JackRedstonia In my opinion (I'm not an owner) It'd be extremely useful, but it's not as simple as it may seem. I've been working on it for while, but I don't have time recently so it's not clear I'll make it. If you have time go ahead, and I can help you time to time.

At least, I really want that future.

dvdsk added a commit to dvdsk/rodio that referenced this issue Apr 3, 2024
seek is broken, RustAudio/lewton#73.
We could work around it by:
 - using unsafe to create an instance of Self
 - use mem::swap to turn the &mut self into a mut self
 - take out the underlying Read+Seek
 - make a new self and seek

If this issue is fixed support use the implementation in
commit: 3bafe32
dvdsk added a commit to dvdsk/rodio that referenced this issue Apr 4, 2024
seek is broken, RustAudio/lewton#73.
We could work around it by:
 - using unsafe to create an instance of Self
 - use mem::swap to turn the &mut self into a mut self
 - take out the underlying Read+Seek
 - make a new self and seek

If this issue is fixed use the implementation in
commit: 3bafe32
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

3 participants