feat(mp4): resolve overlapping fragments during defragmentation - #560
Conversation
0a356b8 to
037a5b1
Compare
037a5b1 to
0e17771
Compare
0e17771 to
8b58d14
Compare
|
Review of the second commit ( The overlap semantics hold up well. The one thing worth fixing before merge VerificationBuilt the branch, Beyond the included tests:
I also worked through whether synthetic tfdt-gap padding can ever be counted 1. Quadratic blowup on crafted overlapping inputTwo loops each rescan every fragment record per overlap:
With n fragments each overlapping the previous by one sample, every fragment
A clean 4x per doubling against a linear baseline — about 175x on the same Both are fixable without any behaviour change:
I tried both: 8.4 MB drops from 8.27 s to 60 ms, i.e. linear and matching Nit
|
8b58d14 to
1f01977
Compare
|
Re-checked Most of it is fixed. The The backwards break does not cover every shapeThe break at A file whose cut point descends one sample at a time hits exactly that: n
A clean 4x per doubling, with 85% of the time in Suggested fixKeep a
That is correct for the same reason your current break is: I tried it on top of your commit: the descending shape drops from 4.23 s to |
A fragment whose tfdt re-declares an earlier decode time supersedes the earlier samples of its track (a retransmission): the fragment appearing later in the file wins, whole re-sent fragments disappear, superseded tails are trimmed at sample granularity, and the timeline continues from the declaring fragment, whether or not the re-sent bytes are identical. Non-overlapping input converts byte-identically to before. Ambiguous overlaps keep failing closed instead of being guessed at: an overlap that starts inside a sample (unless it only shrinks earlier tfdt-gap padding), any abandoned time range that no surviving later fragment declares again (whether the superseded fragment was trimmed or dropped whole, and with voided declarations not counting as coverage), and overlapping files whose fragments use absolute base data offsets are rejected, so no declared content is ever silently dropped. The payload size bound applies to the surviving samples, so re-sent declarations larger than the input file do not reject a file that resolves cleanly. The work is linear in the number of fragments. Coverage checking merges each track's surviving declaration windows once and binary searches them. dropTrackSamplesFrom walked track.frags backwards until a record ended at or below the cut; records voided by earlier cuts sit above later live records in collection order and never satisfy that test once the cut walks below them, so each later cut rescanned the whole voided tail: n one-sample fragments followed by n-1 fragments each rewinding one sample took O(n^2). A parallel live slice keeps the records with kept > 0; their sample ranges tile the samples in order, so a cut pops the suffix with firstSample >= keep and trims at most the record straddling keep. The firstSample+kept <= keep guard stays so a cut landing exactly on a record boundary inside gap padding does not advance its cutoff. Gap padding is tracked as a stack indexed by sample so truncation pops instead of scanning. Each record and padding entry leaves once, making every cut amortized O(1).
1f01977 to
e25e1cf
Compare
|
Thanks for the update. Looks good! |
2 participants
Problem
Fragmented files from segmented capture or upload pipelines can re-declare
time ranges: a fragment is re-sent after a partial write, or a restart
re-declares part of the previous timeline with a backward tfdt. #557 rejects
such files outright, which makes the converter useless for exactly the files
that most need converting to a clean progressive form.
Fix
Overlap resolution as the default behavior, with no new API surface: a
fragment appearing later in the file whose tfdt re-declares an earlier decode
time wins (retransmission semantics). Whole re-sent fragments disappear,
superseded tails are trimmed at sample granularity, and the timeline
continues from the surviving declaration.
The guarantee that keeps this fail-closed in spirit: no declared time range
is ever silently dropped. Every abandoned range must be re-declared by
surviving fragments — a shorter resend, a transitively voided declaration, or
wrong-order concatenation (corruption rather than retransmission) all reject
with clear errors. Cuts landing inside a sample reject too, unless they only
shrink duration padding that an earlier tfdt gap introduced. Inputs that #557
already accepted convert byte-identically, since resolution only activates on
backward tfdts.
The declared-payload bound from #557 now applies after resolution, so a
legitimately re-sent fragment does not count twice against the input size.
Tests
Full resend, superseded tail, reset chains (covered resolves, shortening
rejects), byte-identical resend, multi-track with one overlapping track
(other track byte-verbatim), gap-padding shrink, mid-sample cuts, abandoned
ranges, wrong-order concatenation, and the payload-bound interaction. All
fragmented testdata files convert byte-identically to #557 output.
go test ./...,go vet,gofmt,golangci-lintpass.