Chained ogg streams sometimes have dicontinuities after performing a decoder reset

[aschey]

I'm trying to play some internet radio streams and I've noticed that sometimes there's a slight glitch or pop when the next track starts and the ogg demuxer returns ResetRequired. I've forked symphonia-play (branched off dev-0.6) to be able to play HTTP streams and I can reproduce the issue with this.

It doesn't always happen - sometimes it's barely noticeable, but sometimes there's a pretty audible pop. This is the stream that I've been testing with: cargo run --bin=symphonia-play http://play.global.audio/radio1rock_low.ogg.

[aschey]

I think I see the issue. The end delay is only calculated if you call inspect_end_page, but that is only called from probe_stream_end which is only called if the stream is seekable (not the case for radio streams).

Seems like inspect_end_page will need to be called during read_page for non-seekable streams so the end delay can be calculated correctly.

[pdeljanov]

Hmm, that's interesting. Thanks for you analysis. Could you get me a small reproducible sample since debugging this will need one?

A command like this will use curl to stream the radio station to symphonia while also dumping it to a file. You can listen and Ctrl+c it once it repros.

curl -sL http://play.global.audio/radio1rock_low.ogg | tee radio1rock_low.ogg | ./target/debug/symphonia-play -

[aschey]

I think I found the fix for this in #407. I haven't been able to reproduce with those changes in place. If you'd still like to take a closer look, then I can work on getting a reproducible sample. Good idea to use tee like that; I hadn't thought of that.

[aschey]

I think I found another issue here, though this is more relevant for the start delay. I haven't been able to identify any audible glitches resulting from this, as opposed to the issue above.

In this loop, it reads all of the header packets until it finds a page that has a data packet. At this point, it's already finished calculating the timestamp and gapless info for the first set of data packets. Since it doesn't call probe_stream_start and probe_stream_end until later, the first set of data packets that were read above won't have the gapless info and timestamp adjustments set.

The easiest fix would be to call inspect_start_page after we've found the first data packet. That might make some of the logic in probe_stream_start redundant, though. I'm not sure if that's the best way to handle this.

[pdeljanov]

Hey @aschey, could you please provide some samples for both issues? I think I will need to do a deeper dive to fully understand the problem and determine a proper fix. My feeling at the moment is that calling those inspect_* functions may be too heavyweight a solution.

There's also a possibility a lot of this may change since in 0.6 we may need to change the timestamps to be i64 to support video decoding (see #347) and that opens the possibility of negative timestamps which makes dealing with encoder delay easier.

[aschey]

Sure, here's a snippet from the stream that reproduces the glitch. You can skip forward 250 seconds to get to the relevant bit. It needs to be ran from stdin since the issue only reproduces for non-seekable streams. cat ./radio1rock_low.ogg | ./target/debug/symphonia-play - -s 250.

I haven't found a file that produces any audible artifacts from the second issue. It's just something I noticed while stepping through the code with a debugger.

[pdeljanov]

I haven't forgotten about this one, and I understand the problem, but unfortunately I'm reaching the conclusion that I won't be able to fix it for a 0.5.x release in an acceptable manner.

I think I'll have better luck fixing this in 0.6 because I'm switching over to i64 timestamps which dramatically simplifies the timestamp handling for OGG and reduces the edge cases. I'm working on the switchover now, and once I have it working correctly over there I'll see if the solution can be ported back to 0.5.x.

In this loop, it reads all of the header packets until it finds a page that has a data packet. At this point, it's already finished calculating the timestamp and gapless info for the first set of data packets. Since it doesn't call probe_stream_start and probe_stream_end until later, the first set of data packets that were read above won't have the gapless info and timestamp adjustments set.

This one became a problem in #418. This one is solveable and I have a fix I'm currently testing. Unfortunately it makes solving the first problem even more difficult. :(

[pdeljanov]

All of these issues should now be fully fixed in the dev-0.6 branch. I ended up rewriting the entire OGG demuxer logic for timestamp and duration assignment. Any additional testing is appreciated!

I'll see what I can do for 0.5.x, but it's probably only going to be a partial fix. Luckily, I think an alpha release of 0.6 should be possible sometime this month.

[aschey]

Thanks! I can confirm the discontinuity seems to be gone with the sample I posted above.