fix: correct Source trait semantics and span tracking bugs #831
Conversation
Clarify Source::current_span_len() returns total span length (not remaining), while size_hint() returns remaining samples. Fix multiple bugs in span boundary detection, seeking, and iterator implementations. Opportunistic fixes: - fix division by zero, off-by-one error, and zero case handling - prevent counter overflows - optimize vector allocations Mid-span seeking: best-effort tracking after try_seek() to detect span boundaries when seeking lands mid-span. Fixes #691
|
I did not fix the decoders because I had already done so with the infamous #786. |
roderickvd
force-pushed
the
fix/span-fixes-clean
branch
from
16c90ba to
7538971
Compare
src/source/position.rs
Outdated
|
|
||
| // Detect span boundaries by TWO mechanisms: | ||
| // 1. Reached end of span (by length) - handles same-parameter consecutive spans | ||
| // 2. Parameters changed - handles mid-stream parameter changes |
There was a problem hiding this comment.
2 breaks the contract right? You are only allowed to change parameters at the end of a span. Or is 2 here in case of seeking? Should be removed from Source and allowed only on FixedSource, its seems really complex and costly to get right.
There was a problem hiding this comment.
You are right. I can't remember why I did this. I will remove it unless I start to remember.
Have you had the chance to review the rest of the code? No problem if you haven't, but if you have, then I'll take this as hint for a final commit and get it merged.
There was a problem hiding this comment.
will see if I can review it this weekend, gotta pace myself unfortunately
There was a problem hiding this comment.
I remembered: it was for seeking, indeed. I think I found a way to make it work at acceptable cost.
Parameter updates (channels/sample_rate) occur only at span boundaries or during post-seek detection. Reset counters and enter a detection mode on try_seek (Duration::ZERO is treated as start-of-span).
Use detect_span_boundary and reset_seek_span_tracking to detect span boundaries and parameter changes.
|
Verifying span boundary behavior in the other sources, latest commits I also added:
w.r.t. the first, I wonder how many people use that, given it only ever worked correctly on mono sources. |
|
I want to take a final look at a couple of other sources, that may need to deal with mid-stream endings as well. In the meantime, feel free to review what we've got already, as I might apply the same patterns that we've already got. |
Clarify Source::current_span_len() returns total span length (not remaining), while size_hint() returns remaining samples of the entire iterator. Fix multiple bugs in span boundary detection, seeking, and iterator implementations.
Opportunistic fixes:
Mid-span seeking: best-effort tracking after try_seek() to detect span boundaries when seeking lands mid-span.
Fixes #691