feat(bundles): let a preset play from a position in its timeline - #286
Merged
Merged
Conversation
`PresetHandle.play()` could only start at the top, so a host app driving a `.pulsar` preset had no way to seek — the capability existed one layer down (the composers already take a sound `start`) but was never exposed. Adds `play(fromMs = 0)` across every SDK, defaulting to today's behaviour. A non-zero seek moves audio and haptics together: - the pattern is re-anchored — discrete events before the seek dropped and the rest rebased, both continuous envelopes re-anchored on their interpolated value at that instant; - an envelope whose points all sit before the seek HOLDS its last value rather than emptying, because the engines build the continuous channel only when amplitude and frequency are both non-empty — emptying either one silences both; - the audio seeks to the matching position in the file, keeping an authored `offset` as lead-in until the seek passes it. Each non-zero seek re-parses (the engines can only start a parsed pattern from zero); `fromMs = 0` still reuses the cached parse, so ordinary playback costs exactly what it did before. Because re-parsing is now something an app does repeatedly while scrubbing, iOS also releases the CoreHaptics audio resource it replaces — `registerAudioResource` had no counterpart, so every re-parse leaked one. Android already released its previous sound player. Covers the re-anchoring with unit tests on all four surfaces (swift-testing, JUnit, kotlin.test, jest).
…he others KMP's `PresetHandle` hard-coded `hasAudio = false` and never looked at `preset.audio`, on the grounds that synced audio "needs platform temp-file extraction". Everything else was already in place: `AudioRef` is modelled in the manifest DTO, `PatternComposerHandle.parsePatternWithSound` exists, and both platform composers implement it — the iOS one already slices its audio window exactly like the Swift SDK, and the Android one delegates to a composer that seeks its MediaPlayer. So the only missing piece was the extraction itself: a `writeBundleMedia` expect/actual putting the archive's audio in a platform cache directory (Android `cacheDir`, iOS caches), which is what the Swift and Kotlin bundle loaders already do. With it, a KMP preset plays its audio and honours `play(fromMs)` on both platforms. Also: - `SoundData` gains `hapticChannels`, mirroring the Android SDK, so bundle audio can be marked plain music — otherwise an `.ogg` bundle track would take the coupled path and mute Pulsar's own haptics. - The iOS composer releases the CoreHaptics audio resource it replaces, the same leak just fixed in the Swift SDK.
…ating it Review feedback: React Native had its own copy of the re-anchoring logic in `src/patternSeek.ts`. It was only used on one path — `loadBundleSync()` with no binary, where the pattern lives in JS and goes over the generic `PatternComposer_parsePattern(data)` call, which had no offset to pass. The binary-backed path already delegated to native `LoadedBundle.play(id, fromMs)`. A second implementation of the same rule is a liability, so the offset moves down to where it belongs: `parsePattern(hapticsData, fromMs)` and `parsePatternWithSound(..., fromMs)` on the composers themselves. The composer is what can only start at zero, so it is what should compensate. Consequences: - `src/patternSeek.ts` and its test are gone; `createBundle.ts` sends the authored pattern plus `fromMs` and lets native re-anchor it, on both paths. - `PresetHandle` no longer does seek arithmetic on any platform — it forwards `fromMs` — so `PatternSeek` now has exactly one caller per SDK. - `PatternSeek.soundWindow` takes the authored `start`/`duration` too, since the composer applies it to sounds that already carry a trim window. `fromMs` defaults to 0 everywhere, so every existing caller is untouched; the one exception is the RN ObjC bridge, because a Swift default argument still changes the generated selector.
…es carry the rest Two pieces of review feedback. Comments: the seek work leaned on narrative block comments where naming would do. Extracting `holdingLastValue` carries the envelope rule in its name, and `seekIntoFile` / `leadIn` / `playsToEndOfFile` / `alreadyParsedHere` / `seekedPattern` retire the rest. `duration(of:)` became `lastTimestamp(of:)`, which is what it actually returns, and `value`/`valueAt` became `interpolatedValue`/`interpolatedValueAt`. What survives is the one fact a reader cannot derive locally: emptying an envelope silences both continuous channels, because the composer builds that line only when the amplitude and frequency curves are each non-empty. Docs: `play(fromMs)` was only in BUNDLES.md. It now appears wherever the bundle API is described — the five SDK pages on the docs site, and the iOS, Android, KMP and Flutter bundle READMEs — alongside the updated `parsePattern` / `parsePatternWithSound` signatures. The KMP page's caution that synced bundle audio "is not wired yet" is gone, since it now is.
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.
What
PresetHandle.play()could only start a bundle preset at the top, so an app driving a.pulsarpreset had no way to seek. The capability already existed one layer down — the composers take a soundstart, andSoundDatacarriesstartMs— but nothing above them passed one.This adds
play(fromMs = 0)to every SDK, defaulting to today's behaviour so nothing existing changes.play()/play(fromMs: Double)play(fromMs: Long = 0)play({double fromMs = 0})play(fromMs?: number)How a seek works
Audio and haptics have to move together, so a non-zero
fromMsdoes two things.Haptics — re-anchor the pattern. Discrete events before the seek are dropped and the rest rebased to zero; each continuous envelope is re-anchored on its interpolated value at that instant. The subtle part: an envelope whose points all sit before the seek holds that last value instead of emptying. The engines build the continuous channel only when the amplitude and frequency curves are both non-empty, so emptying either one silences both. (This is the same rule
PulsarApp/src/haptics/patternSeek.tslearned the hard way; the native version is a port of it.)Audio — seek into the file. A sound offset by
offsetms sits at file positiont - offsetwhen the haptics are att, so the seek first eats into the lead-in and only then advances into the file.Each non-zero seek re-parses, because the engines can only start a parsed pattern from zero.
fromMs = 0still reuses the cached parse, so plain playback costs exactly what it did before — a preset only ever played from the start is parsed once, as always.Second commit: KMP now carries bundle audio
Review feedback on the first draft: the docs said "KMP is haptics-only, so
fromMsseeks the haptics alone", which is a caveat that shouldn't merge if it's fixable. It was.KMP's
PresetHandlehard-codedhasAudio = falseand never readpreset.audio, on the grounds that synced audio "needs platform temp-file extraction". But everything else was already there —AudioRefis in the manifest DTO,PatternComposerHandle.parsePatternWithSoundexists, and both platform composers implement it (the iOS one already slices its audio window exactly like the Swift SDK; the Android one delegates to a composer that seeks itsMediaPlayer).So the whole gap was the extraction: a
writeBundleMediaexpect/actual that puts the archive's audio into a platform cache dir (AndroidcacheDir, iOS caches), which is precisely what the Swift and Kotlin loaders already do. A KMP preset now plays its audio and honoursplay(fromMs)on both platforms, and thebundle/README.md"Limits" section is gone.Two riders:
SoundDatagainshapticChannels, mirroring the Android SDK. Without it a.oggbundle track would take the audio-coupled path and mute Pulsar's own haptics; the Android loader already sets itfalsefor bundle audio.Third commit: the composers own the seek, RN stops duplicating it
More review feedback, and a fair hit: React Native had its own copy of the re-anchoring rule in
src/patternSeek.ts.It was only reachable on one path. The binary-backed bundle already delegated (
Pulsar_playBundlePreset(token, id, fromMs)→ nativeLoadedBundle.play(id, fromMs)); the JS copy existed solely forloadBundleSync()with no binary, where the pattern lives in JS and goes over the genericPatternComposer_parsePattern(data)call — which had no offset to pass. Rather than letplay(fromMs)silently no-op there, I had duplicated the maths. Wrong call.The offset now lives on the composers instead:
parsePattern(hapticsData, fromMs)andparsePatternWithSound(..., fromMs). The composer is the thing that can only start a parsed pattern at zero, so it is the thing that should compensate. That means:src/patternSeek.tsand its test are deleted.createBundle.tssends the authored pattern plusfromMson both paths and lets native re-anchor.PresetHandlestops doing seek arithmetic on Swift, Kotlin and KMP — it just forwardsfromMs.PatternSeekis down to one caller per SDK.PatternSeek.soundWindownow takes the authoredstart/duration, since the composer applies the seek to sounds that may already carry a trim window.fromMsdefaults to0on every signature, so existing callers are untouched — with one unavoidable exception: a Swift default argument still changes the generated ObjC selector, so the RN bridge's twoparsePattern*calls had to be updated. Nothing outside the repo calls those.Fourth commit: docs, and naming instead of commentary
play(fromMs)was documented only inBUNDLES.md. It now appears everywhere the bundle API is described — the five SDK pages on the docs site plus the iOS, Android, KMP and Flutter bundle READMEs — along with the updatedparsePattern/parsePatternWithSoundsignatures. Headings are unchanged so existing anchor links still resolve. The KMP page's:::cautionsaying synced bundle audio "is not wired yet" is deleted, since it now is. Verified by building the docs site: 41 pages,Complete!.The same commit trades narrative comments for names:
holdingLastValuecarries the envelope rule,duration(of:)becamelastTimestamp(of:)(what it returns), andseekIntoFile/leadIn/playsToEndOfFile/alreadyParsedHere/seekedPatternretire the rest. One fact stays as a comment because it cannot be derived locally: emptying an envelope silences both continuous channels, since the composer builds that line only when amplitude and frequency are each non-empty.It also fixes something that slipped in with the previous commit — the Kotlin composer briefly took
hapticsData0/sound0shadow parameters, which renames a public parameter and would break named-argument callers. Real names restored, locals renamed instead.Drive-by fix
Re-parsing is now something an app does repeatedly while scrubbing, which exposed a leak: iOS
registerAudioResourcehad no counterpart, so everyparsePatternWithSoundregistered a CoreHaptics audio resource that was never released.PatternComposernow releases the resource it replaces, alongside the temp audio file it already cleaned up. Android already released its previousAudioHapticPlayer.Not done:
CHHapticAdvancedPatternPlayer.seek(toOffset:)Worth recording, since it's the obvious question. iOS does have a real seek — but only on
CHHapticAdvancedPatternPlayer, and bundle players are built withengine.makePlayer(with:)→ a plainCHHapticPatternPlayer. Using it wouldn't remove this work: Android has no equivalent (aVibrationEffectis precomputed and can't be seeked), and more importantly the audio wouldn't follow — this repo already concluded there is no runtime seek on a registered audio resource, which is whymakeAudioEventslices to a temp file forstart/durationin the first place. A sounded preset would still have to re-parse.It would pay off for a haptics-only preset on iOS, where an advanced player plus
seekavoids the re-parse entirely. Left out of this PR deliberately — happy to add it as a follow-up.Tests
New
PatternSeekunit tests on all four surfaces — swift-testing, JUnit, kotlin.test and jest — covering the rebase, the interpolated re-anchor, the hold-last-value case, empty envelopes, and the audio offset/window arithmetic.xcodebuild test -scheme Pulsar-Package— both CI steps green./gradlew :Pulsar:testDebugUnitTest— green (10 new):library:compileAndroidMain+:library:iosSimulatorArm64Test— green (6 new)flutter analyze+flutter test— greentsc, eslint on the changed files — greenExercised end to end on an iOS simulator through PulsarApp's Argent suite (see the linked PR): the Audio Sync demo seeks, and the full 23-flow suite passes.
Companion
App + cross-SDK docs: software-mansion-labs/pulsar-private#187