Repository navigation
fix(media): count grid ordinals as skipped when the sample stream ends early - #914
Merged
Merged
Conversation
…s early (#850) The worker's materialize() loop broke out when the decoder's sample generator finished before the grid did, so every remaining ordinal was in neither the materialized frames nor `skipped`. The loop now lives in a pure core helper, walkGrid(), which records each remaining ordinal as skipped unless the run was cancelled, and computes the ordinal in one place for visited, null and padded positions. walkGrid has vitest coverage for null samples, an early end, an empty stream and both cancel paths. The doc on VideoImportResult.skipped now states materialized + skipped.length === expected for an uncancelled run.
#850) Every browser case asserted an empty `skipped`, so the null-sample path was covered only by the walkGrid unit tests. A VP8 clip cut mid-GOP, built in the page with mediabunny, leaves three grid ordinals with no key frame to decode from; the spec asserts they land in `skipped` and that materialized plus skipped equals expected.
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
1 participant
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.
Fixes #850.
materialize()in the media worker broke out of its loop when the decoder's sample generator ended before the grid did, so every remaining ordinal was in neither the materialized frames norskipped.walkGrid()(frontend/media/src/grid-walk.ts), which records each remaining ordinal as skipped on an early end, unless the run was cancelled.VideoImportResult.skippednow documentsmaterialized + skipped.length === expectedfor an uncancelled run.nullsamples throughmaterialize()(a clip cut mid-GOP, so the leading frames have no key frame).Scope note: with mediabunny 1.61.3 the sample stream emits one item per requested timestamp and ends early only after
return()(a cancel), so the early-end path is not reachable in an uncancelled run today. The fix is protection against a future change, and only thewalkGridunit tests exercise it. The browser spec covers the null-sample path, which the old code already handled, so it passes on both old and new code.Verified locally: media vitest 24/24, lint, build, Chromium suite 24/24.