Reconcile spliced sample counts in signed space; absorb one-sample rounding drift (fixes #68, #39) - #79
Merged
jwcullen merged 1 commit intoAug 5, 2026
Conversation
…ample rounding drift. The objects-to-3OA splice computes per-segment sample counts by floor()-ing double products of parsed ADM durations. When a segment boundary lands on an exact sample but the double product rounds just below it (e.g. 163.89 s * 48000 Hz = 7866719.99999999906...), the splice comes up one sample short, and the final reconciliation (fabs(num_samples_count - *total_samples_per_channel)) promotes the count through unsigned arithmetic, wraps to ~1.8e19, and CHECK-crashes with SIGABRT. - Reconcile in signed 64-bit space and return a Status instead of crashing (continuing the direction of fecb716). - Absorb a one-sample shortfall by splicing the still-unread remainder through the panner, so the output stays sample-exact. - Convert the per-segment and total-duration CHECKs on input-dependent values to Statuses. - Add a regression test: a 6005-sample mono Dolby-mode ADM BWF (00:00:00.12510 at 48 kHz) that previously crashed and must now produce sample-exact 3OA output. Fixes AOMediaCodec#68. Fixes AOMediaCodec#39. Related: AOMediaCodec#37 (the remaining CHECK from that report class now returns a clean error instead of aborting).
Contributor
Author
|
Thanks |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows up on #75's invitation to send fixes for the recently filed issues.
Root cause
As analyzed in #68 and independently measured in #39: the objects-to-3OA
splice (
wav_file_splicer.cc) computes per-segment sample counts byfloor()-ing double products of parsed ADM durations. When a segmentboundary falls on an exact sample but the double product rounds just below
it (e.g. 163.89 s × 48000 Hz = 7866719.99999999906…), the splice comes up
one sample short. The final reconciliation —
fabs(num_samples_count - *total_samples_per_channel)— then promotes theintcount through unsigned arithmetic, so the −1 shortfall wraps to~1.84467e+19 and the
CHECKaborts (SIGABRT). Whether a given input tripsthis depends only on which way the products round, which is why roughly
half of plausible authoring grids crash (boundary table in #68).
fecb716 moved the total sample count into integer space; the
per-segment products and the final reconciliation still ran through
doubles and unsigned promotion. This PR completes that arc:
Statusinstead ofcrashing.
through the panner, so the output stays sample-exact (no padding, the
real final sample).
CHECKs on input-dependentvalues to
Statuses (an encoder should not abort on valid input).path: a 6005-sample mono ADM BWF (
00:00:00.12510at 48 kHz, i.e. the5-decimal ADM timecode quantization of 0.125104166… s) that previously
crashed with Issue Check failed: fabs(num_samples_count - *total_samples_per_channel) <= kErrorTolerance #39's exact signature and must now produce sample-exact 3OA
output.
What this deliberately does not do: re-time the segmentation.
Per-segment lengths are computed exactly as before, so output for inputs
that already worked is unchanged (verified below).
Verification (Linux, at dd03679)
dd03679(SIGABRT,wav_file_splicer.cc:436, magnitude 1.84467e+19) and completes cleanlypost-fix (exit 0, valid
.iamf).(200-block and 1000-block, from the generator cited in wav_file_splicer CHECK failure (SIGABRT) on ~half of uniform object automation grids — unsigned underflow in sample reconciliation #68) produce
byte-identical
.iamfoutput pre/post fix.CHECK) andpasses with the fix, asserting the panned output is exactly
6005 × 16 ch × 3 B.
bazelisk test -c opt --test_output=errors iamf/...(the repo's CIsuite) is green on this branch across the full matrix — linux-amd64,
macos-arm64, macos-amd64, windows, plus the test-vector generator
(fork Actions run
30877230150);
//iamf/cli/adm_to_user_metadata/adm/tests:wav_file_splicer_testPASSED.
git-clang-format dd03679is idempotent on the change.On #37: fecb716 addressed that report's total-samples computation; this
change converts the remaining aborts in the same function to clean errors.
Contributor agreement: executed and on file since 2026-07-27 (per
CONTRIBUTING), as with #78.