Skip to content

@remotion/media: Bound unshifted audio decoding to the sequence end - #12683

Closed
alec-watts wants to merge 3 commits into
remotion-dev:mainfrom
alec-watts:codex/bound-audio-to-sequence-end
Closed

alec-watts wants to merge 3 commits into
remotion-dev:mainfrom
alec-watts:codex/bound-audio-to-sequence-end

Conversation

@alec-watts

@alec-watts alec-watts commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Short non-looping @remotion/media Audio and Video clips can decode source audio beyond their sequence end even though those chunks will never play.

Cap the existing iterator at the earlier of the media end and sequence end when no pitch shifting is active. Invalidate the audio iterator before the existing duration-change seek so extending a clip can decode its newly added span instead of retaining the exhausted iterator.

Loops retain their existing source bounds. Pitch-shifted audio also retains its existing bounds: the unconditional cap reproducibly fails the existing preview pitch regression (estimated 654 Hz versus expected 660 Hz), while unchanged upstream passes. The streaming pitch shifter consumes future samples, so safely bounding that path needs separate tail handling. No pitch assertion or tolerance is changed.

This applies to both Audio and Video's shared audio manager. Video-frame decoding is unchanged.

Validation

Based on upstream main 4725fadc9d4bab2779af5846554b18adff1613c6.

  • Full media Chromium suite with the audio fix and fixture-repair dependency: 193 tests passed across 54 files.
  • Added seven real-AAC/browser cases to the existing MediaPlayer suite: Audio/Video, source trims, 0.5×/1×/2× rates, a sub-100 ms clip, unchanged loop bounds, and extension → shrink → re-extension.
  • Removing the cap fails three boundary cases. Keeping the cap but removing duration invalidation fails both Audio and Video extension cases: the queue remains at 0.2 s instead of reaching 0.5 s.
  • Removing the pitch exception reproduces the existing pitch-preview failure; unchanged upstream and the final candidate pass it.
  • Core/Player/media dependency builds, media TypeScript, media lint and formatting, and git diff --check pass (existing lint warnings only).

The tests exercise actual decoding and queued audio, not physical-speaker output or an editor Undo UI. This PR makes no playback-FPS, retained-memory or universal performance claim.

Related downstream investigation: kino-ai/kino#4930. This upstream version adds the pitch exception discovered during broader validation. The separate paused-audio scheduling proposal #12609 is not required by this fix.

Why each file changes

File Reason
packages/media/src/audio-iterator-manager.ts Bound non-looping, unshifted decoding using the existing media and sequence endpoints.
packages/media/src/media-player.ts Invalidate the old audio iterator when sequence duration changes.
packages/media/src/test/media-player.test.ts Extend the existing real-codec fixture to guard bounds and duration edits for Audio and Video.
packages/example-videos/src/cached-example-media.ts Dependency #12685 stages cached fixtures at existing browser URLs.
.gitignore Dependency #12685 excludes generated public fixture copies.
turbo.json Dependency #12685 restores generated fixtures on cache hits.

Diff cleanup found no unrelated formatting, new production mechanism, unused exports or extra media fixtures.

Prepared with OpenAI Codex.

Fixture dependency and CI

Includes the separately reviewable fixture repair #12685 at 8b61316. Initial macOS CI failed because upstream main's asset migration left /bigbuckbunny.mp4 returning 404; the same existing test failed on unchanged main. The dependency repairs the existing downloader/public-directory setup and Turbo cached outputs. Maintainers can merge #12685 first; those changes will then drop out of this PR's diff.

The combined branch passes all 193 media Chromium tests. #12685 separately passes all 186 baseline media tests, and a full Turbo cache hit restores its eight public fixtures byte-for-byte. Fresh upstream CI is running on merge head 1aa96be.

@vercel

vercel Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
bugs Ready Ready Preview Oct 29, 2026 12:25am UTC
remotion Ready Ready Preview Oct 29, 2026 12:25am UTC

Request Review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Bounded non-looping audio decode — startAudioIterator now caps maximumTimestamp at Math.min(getMediaEndTimestamp(), getSequenceEndTimestamp()) when neither looping nor pitch-shifting, so short clips stop decoding source audio past the sequence end. onAudioChunk already caps scheduled duration at the same getSequenceEndTimestamp(), so this cannot truncate audible audio; it only avoids decoding chunks that would never be scheduled.
  • Pitch/loop exception — loop || currentToneFrequency !== 1 retains the old media-end bound. This matches exactly the condition under which pitchShiftAudioIterator engages (it passes through when toneFrequency === 1), so the streaming shifter's future-sample lookahead keeps its full input. The tradeoff is documented in the PR body and covered by the existing pitch regression.
  • Duration-change invalidation — setSequenceDurationInFrames now calls destroyIterator() before the discontinuity seek, matching the sibling setters (setPlaybackRate, setLoop, setTrimRange, …). This is the necessary half of the fix: after a duration change the retained iterator is exhausted at the old end, and the seek path would otherwise short-circuit via isAlreadyQueued and leave live extensions silent.
  • Tests — makeAudioPlayer gains pass-through options (durationInFrames, playbackRate, trimBefore, trimAfter, loop) and two real-codec cases assert the decode bound and the extension → shrink → re-extension reopening for both Audio and Video.

The cap is in the same units as getMediaEndTimestamp() (media seconds), uses the same endpoint the scheduler already enforces, and is recomputed only at iterator start — with the duration-change invalidation closing the growth window. No behavioral surface beyond the intended bound.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash | 𝕏

@samohovets

Copy link
Copy Markdown
Member

Thanks @alec-watts! Since this fork doesn't allow maintainer edits, we moved your change to a new PR with you credited as co-author. It's rebased on main, the fixture changes that already landed in #12685 are dropped, and a few comments are removed. Closing this one in favor of the new PR.

@samohovets samohovets self-assigned this Oct 29, 2026
@samohovets samohovets closed this Oct 29, 2026
DeryFerd pushed a commit to DeryFerd/remotion that referenced this pull request Oct 29, 2026
Short non-looping Audio and Video clips no longer decode source audio past
their sequence end. Extending a clip's duration recreates the audio iterator
so the newly added span is decoded.

Supersedes remotion-dev#12683.

Co-authored-by: Alec Watts <alecwatts1@gmail.com>

This branch was successfully deployed

2 active deployments
Preview – remotion — 1aa96be9 Deployed Oct 29, 2026 by vercel[bot]
Preview – bugs — 1aa96be9 Deployed Oct 29, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants