get_last_absgp returns None for the first stream

[TonalidadeHidrica]

The following is a snippet that reads first 30 packets from an ogg file.

    use lewton::inside_ogg::OggStreamReader;
    use std::io::BufReader;
    use std::fs::File;
    let reader = BufReader::new(File::open("some_ogg_file.ogg").unwrap());
    let mut reader = OggStreamReader::new(reader).unwrap();
    dbg!(reader.get_last_absgp());
    let mut i = 0;
    while let Some(_packet) = reader.read_dec_packet().unwrap() {
        dbg!(reader.get_last_absgp());
        i += 1;
        if i >= 30 {
            break;
        }
    }

It is reasonable that get_last_absgp returns None before reading any packet, but the program above actually output 23 Nones, followed by 7 reasonable values. Reading the code, I realized that cur_absgp is never updated for the first stream, but updated for the second and subsequent streams. Is this an intended behaviour, or just a bug?

[est31]

The code you link to is not the only place that updates the absolute granule position, it is also updated at page boundaries, which occur more often. https://github.com/RustAudio/lewton/blob/master/src/inside_ogg.rs#L198

Quoting the vorbis spec:

The granule position of a page represents the end PCM sample position of the last packet completed on that page.

Actually you bring up a good point, I might need to change the code that updates cur_absgp unconditionally on a new stream and only update it if it's the last packet finished on the page.

[TonalidadeHidrica]

Thanks. I'll learn more about ogg and Vorbis formats and consider if I can contribute.

[TonalidadeHidrica]

I now understood the structure of ogg/Vorbis and clearly figured out the reason. The "absolute granule position" is virtually the last index of decoded sample, defined per page, so we cannot determine the position until we finish parsing the first page. However, I don't think it's a good idea to update cur_absgp only when parsing the last packet in a page, simply because it'll be less useful.

Instead, I suggest that the decoder reads the entire packets in the first page where the first packet ends, and then calculate the appropriate cur_absgp after the first packet. This is not only useful but also enables to truncate the beginning samples when needed, as the Vorbis spec requires: (source)

A negative value indicates that output samples preceeding time zero should be discarded during decoding; this technique is used to allow sample-granularity editing of the stream start time of already-encoded Vorbis streams. The number of samples to be discarded must not exceed the overlap-add span of the first two audio packets.

By the way IMHO the name get_last_absgp should be rather like num_decoded_samples under the semantics of Vorbis parsing, which may be easier for user to understand the meaning. How do you think?

[TonalidadeHidrica]

I also realized that updating cur_absgp here is wrong, since the packets in the current page may not be fully decoded after parsing the first packet in the page.

[est31]

I suggest that the decoder reads the entire packets in the first page where the first packet ends,

This would likely need some ability to "peek" future packets in the Ogg crate. We are only interested about packets finishing on the same page the packet finishes. So we don't need complicated fragmented packet logic, all such packets would be inside the same page. Once we have that peeking ability we can use get_decoded_sample_count to get the number of samples.

This is not only useful but also enables to truncate the beginning samples when needed, as the Vorbis spec requires: (source)

Good idea, should be done!

I also realized that updating cur_absgp here is wrong, since the packets in the current page may not be fully decoded after parsing the first packet in the page.

Yes, that's what I said in my comment above

[TonalidadeHidrica]

This would likely need some ability to "peek" future packets in the Ogg crate.

I think this can still be resolved without "peeking" feature. Instead, the decoder can store the packets it obtained and use it later. Possible drawback is that the packets are unnecessarily moved from the Ogg reader to Vorbis decoder (which I actually think is small enough). Which do you think is the better way?

Yes, that's what I said in my comment above

Oh, excuse me.