Skip to content

Commit 890c0fd

Browse files
committed
feat(studio): audition a preset from the playhead while paused
Hovering a preset writes it to the running graph — which is silent while the transport is paused. So the whole affordance only worked mid-playback: a paused author hovering the shelf heard nothing at all and had no way to know the feature existed. Hovering now starts playback from wherever the playhead sits, and leaving stops it and returns the playhead to exactly where it was found. Browsing the shelf is not an edit and must not cost the author their place. Two guards, each with a test: - **A transport the author started is left alone**, in both directions. Stopping their playback because they passed over a preset would be the panel taking a decision nobody offered it. - **Every path that ends an audition stops the transport** — leaving, applying, and the panel unmounting. A click means "keep this", not "and carry on playing from wherever the audition reached". Order matters at both ends: the chain goes into the graph before playback starts, or the first moment heard is the un-auditioned mix; and playback stops before the chain reverts, so the last thing heard is the preset rather than a frame of the old chain coming back. `playbackRequest` on the player store follows `requestedSeekTime`: the panel cannot reach `useTimelinePlayer`, which is a single instance owned by the shell. It carries a nonce because two hovers in a row both want play, and without one the second is indistinguishable from the first already having been served. Falsified: not returning the playhead, and hijacking a transport the author started, each fail a test. studio 3716 passing, 18 todo.
1 parent bbaa6b1 commit 890c0fd

6 files changed

Lines changed: 187 additions & 7 deletions

File tree

packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,45 @@ describe("AudioFxGroup dynamic carve", () => {
435435
* nobody asked to level, through a channel that does not persist: audible,
436436
* absent from the document, and gone on the next reload.
437437
*/
438+
describe("auditioning starts the transport when it has to", () => {
439+
const store = () => usePlayerStore.getState();
440+
441+
const hoverPreset = (host: HTMLElement) => {
442+
act(() => byTextButton(host, "Presets")?.click());
443+
act(() => host.querySelector<HTMLElement>(".hf-fx-preset-item")?.focus());
444+
};
445+
const leaveShelf = (host: HTMLElement) =>
446+
act(() => {
447+
host
448+
.querySelector(".hf-fx-preset-menu")
449+
?.dispatchEvent(new FocusEvent("focusout", { bubbles: true }));
450+
});
451+
452+
it("plays from the playhead, then puts it back exactly where it was", () => {
453+
// Browsing the shelf must not cost the author their place: hovering is not
454+
// an edit, so the playhead it borrows has to be returned.
455+
act(() => usePlayerStore.setState({ isPlaying: false, currentTime: 42 }));
456+
const { host } = mount({ "fx-chain": CHAIN });
457+
hoverPreset(host);
458+
expect(store().playbackRequest?.playing).toBe(true);
459+
460+
leaveShelf(host);
461+
expect(store().playbackRequest?.playing).toBe(false);
462+
expect(store().playbackRequest?.returnTo).toBe(42);
463+
});
464+
465+
it("leaves a transport the author started alone", () => {
466+
// Stopping their playback because they passed over a preset would be the
467+
// panel taking a decision nobody offered it.
468+
act(() => usePlayerStore.setState({ isPlaying: true, currentTime: 12 }));
469+
const { host } = mount({ "fx-chain": CHAIN });
470+
const before = store().playbackRequest?.nonce ?? 0;
471+
hoverPreset(host);
472+
leaveShelf(host);
473+
expect(store().playbackRequest?.nonce ?? 0).toBe(before);
474+
});
475+
});
476+
438477
it("drops a levelling measurement that lands after the pointer has gone", async () => {
439478
const { release, decoded } = stubGatedDecode();
440479
const { host, onSetAttributeLive } = mount({ "fx-chain": CHAIN });

packages/studio/src/components/editor/propertyPanelAudioFxGroup.tsx

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,40 @@ export function AudioFxGroup({
574574
}
575575
};
576576

577+
/**
578+
* Where the playhead was when an audition started the transport, so leaving
579+
* can put it back. Null means this audition did not start playback — the
580+
* transport was already running and must be left alone.
581+
*/
582+
const auditionReturn = useRef<number | null>(null);
583+
584+
/**
585+
* Start playback for an audition, and stop it again on the way out.
586+
*
587+
* An audition writes the preset to the running graph, which is silent while
588+
* the transport is paused — so a paused author hovering a preset heard
589+
* nothing at all, and the whole affordance only worked mid-playback. Hovering
590+
* now plays from the playhead, and leaving stops and rewinds to exactly where
591+
* it started: browsing the shelf must not cost the author their place.
592+
*
593+
* Already playing, this does nothing in either direction. The author started
594+
* that, and stopping their transport because they passed over a preset would
595+
* be the panel taking a decision that was not offered to it.
596+
*/
597+
const auditionTransport = (on: boolean): void => {
598+
const store = usePlayerStore.getState();
599+
if (on) {
600+
if (store.isPlaying || auditionReturn.current !== null) return;
601+
auditionReturn.current = store.currentTime;
602+
store.requestPlayback(true);
603+
return;
604+
}
605+
const returnTo = auditionReturn.current;
606+
if (returnTo === null) return;
607+
auditionReturn.current = null;
608+
store.requestPlayback(false, returnTo);
609+
};
610+
577611
const [auditioningLevel, setAuditioningLevel] = useState(false);
578612
/**
579613
* Bumped on every enter and leave, so a measurement can tell whether the
@@ -808,6 +842,7 @@ export function AudioFxGroup({
808842
next.nodes.length ? serializeAudioFxChain(next) : null,
809843
)
810844
}
845+
onAuditionTransport={auditionTransport}
811846
onChainPreview={(next) =>
812847
// Live writes skip the preview refresh entirely, so dragging a knob no
813848
// longer reloads the composition and restarts playback on every pixel.

packages/studio/src/components/editor/propertyPanelFxSection.test.tsx

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ function mount(overrides: Partial<Parameters<typeof FxSection>[0]> = {}) {
9090
onRemoveNodeAutomation={overrides.onRemoveNodeAutomation}
9191
onAutomatePreset={overrides.onAutomatePreset}
9292
onRemovePresetAutomation={overrides.onRemovePresetAutomation}
93+
onAuditionTransport={overrides.onAuditionTransport}
9394
automatedPresets={overrides.automatedPresets}
9495
onLevel={overrides.onLevel}
9596
onRemoveLevel={overrides.onRemoveLevel}
@@ -695,6 +696,52 @@ describe("FxSection chain", () => {
695696
expect(item?.querySelector(".hf-fx-preset-name")?.textContent).toBe("Telephone");
696697
});
697698

699+
describe("auditioning while the transport is paused", () => {
700+
it("starts playback so a paused author can hear the preset at all", () => {
701+
// The audition is written to the running graph, which is silent while the
702+
// transport is paused — so without this, hovering a preset did nothing
703+
// whatsoever unless the author happened to be mid-playback.
704+
const onAuditionTransport = vi.fn();
705+
const { host } = mount({ chain: chainOf("peaking"), onAuditionTransport });
706+
click(byText(host, "button", "Presets"));
707+
act(() => (presetButton(host, "telephone") as HTMLElement | null)?.focus());
708+
expect(onAuditionTransport).toHaveBeenLastCalledWith(true);
709+
});
710+
711+
it("stops it again on the way out", () => {
712+
const onAuditionTransport = vi.fn();
713+
const { host } = mount({ chain: chainOf("peaking"), onAuditionTransport });
714+
click(byText(host, "button", "Presets"));
715+
act(() => (presetButton(host, "telephone") as HTMLElement | null)?.focus());
716+
act(() => {
717+
host
718+
.querySelector(".hf-fx-preset-menu")
719+
?.dispatchEvent(new FocusEvent("focusout", { bubbles: true }));
720+
});
721+
expect(onAuditionTransport).toHaveBeenLastCalledWith(false);
722+
});
723+
724+
it("stops it when the preset is applied, rather than playing on", () => {
725+
// The click means "keep this", not "and carry on playing from wherever
726+
// the audition reached".
727+
const onAuditionTransport = vi.fn();
728+
const { host } = mount({ chain: { version: 1, nodes: [] }, onAuditionTransport });
729+
click(byText(host, "button", "Presets"));
730+
act(() => (presetButton(host, "telephone") as HTMLElement | null)?.focus());
731+
click(presetButton(host, "telephone"));
732+
expect(onAuditionTransport).toHaveBeenLastCalledWith(false);
733+
});
734+
735+
it("stops it if the panel goes away mid-audition", () => {
736+
const onAuditionTransport = vi.fn();
737+
const { host, root } = mount({ chain: chainOf("peaking"), onAuditionTransport });
738+
click(byText(host, "button", "Presets"));
739+
act(() => (presetButton(host, "telephone") as HTMLElement | null)?.focus());
740+
act(() => root.unmount());
741+
expect(onAuditionTransport).toHaveBeenLastCalledWith(false);
742+
});
743+
});
744+
698745
describe("getting back out of a menu", () => {
699746
/** Escape, from inside the section, the way a keystroke really arrives. */
700747
const escape = (host: HTMLElement) =>

packages/studio/src/components/editor/propertyPanelFxSection.tsx

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,14 @@ export interface FxSectionProps {
111111
onAuditionLevel?(on: boolean): void;
112112
/** Whether that measurement is running, so the button can say so. */
113113
auditioningLevel?: boolean;
114+
/**
115+
* Start the transport for an audition, and stop it on the way out.
116+
*
117+
* An audition is written to the running graph, which is silent while the
118+
* transport is paused — so without this, hovering a preset does nothing at all
119+
* for a paused author.
120+
*/
121+
onAuditionTransport?(on: boolean): void;
114122
/** Structural edits and gesture-end writes; this is the one that persists. */
115123
onChainChange(chain: HfAudioFxChain): void;
116124
/** Continuous updates while a control is being dragged. */
@@ -156,6 +164,7 @@ export function FxSection({
156164
onAutomatePreset,
157165
onRemovePresetAutomation,
158166
automatedPresets,
167+
onAuditionTransport,
159168
}: FxSectionProps) {
160169
const presetAutomated = automatedPresets ?? new Set<string>();
161170
// Falls back to the persisting write when no preview handler is supplied, which
@@ -228,12 +237,18 @@ export function FxSection({
228237
if (make) {
229238
auditionBase.current ??= chain;
230239
onChainPreview(make(auditionBase.current));
240+
// After the chain is in the graph, not before: starting the transport
241+
// first plays a moment of the un-auditioned mix.
242+
onAuditionTransport?.(true);
231243
} else if (auditionBase.current) {
244+
// Stop before reverting, for the mirror of that reason — the last thing
245+
// heard should be the preset, not a frame of the chain coming back.
246+
onAuditionTransport?.(false);
232247
onChainPreview(auditionBase.current);
233248
auditionBase.current = null;
234249
}
235250
},
236-
[chain, onChainPreview],
251+
[chain, onChainPreview, onAuditionTransport],
237252
);
238253

239254
/**
@@ -253,9 +268,14 @@ export function FxSection({
253268
// Leaving by any route other than the pointer — the element deselected, the
254269
// panel closed — would otherwise leave the audition playing over a chain the
255270
// document does not have.
271+
const transportRef = useRef(onAuditionTransport);
272+
transportRef.current = onAuditionTransport;
256273
useEffect(
257274
() => () => {
258-
if (auditionBase.current) previewRef.current?.(auditionBase.current);
275+
if (auditionBase.current) {
276+
transportRef.current?.(false);
277+
previewRef.current?.(auditionBase.current);
278+
}
259279
},
260280
[],
261281
);
@@ -272,13 +292,14 @@ export function FxSection({
272292
// old chain back over the write that just landed is a race the author
273293
// hears as the preset arriving and then leaving again.
274294
auditionBase.current = null;
295+
onAuditionTransport?.(false);
275296
mutate(next.nodes);
276297
// Land on the first node the preset wrote, so the author can hear what
277298
// arrived and immediately see what it is made of.
278299
setOpenNode(next.nodes.findIndex((n) => n.fromPreset === preset.id));
279300
setPicking(false);
280301
},
281-
[chain, mutate],
302+
[chain, mutate, onAuditionTransport],
282303
);
283304

284305
/**
@@ -322,21 +343,23 @@ export function FxSection({
322343
const addJob = useCallback(
323344
(job: HfAudioFxJob) => {
324345
auditionBase.current = null;
346+
onAuditionTransport?.(false);
325347
mutate(withJob(chain, job).nodes);
326348
setOpenNode(chain.nodes.length);
327349
setAdding(false);
328350
},
329-
[chain, mutate, withJob],
351+
[chain, mutate, withJob, onAuditionTransport],
330352
);
331353

332354
const addEffect = useCallback(
333355
(type: string) => {
334356
auditionBase.current = null;
357+
onAuditionTransport?.(false);
335358
mutate(withEffect(chain, type).nodes);
336359
setOpenNode(chain.nodes.length);
337360
setAdding(false);
338361
},
339-
[chain, mutate, withEffect],
362+
[chain, mutate, withEffect, onAuditionTransport],
340363
);
341364

342365
const updateNode = useCallback(
@@ -458,11 +481,12 @@ export function FxSection({
458481

459482
const addEq = useCallback(() => {
460483
auditionBase.current = null;
484+
onAuditionTransport?.(false);
461485
const { chain: next, eqId } = addAudioEq(chain);
462486
mutate(next.nodes);
463487
setOpenEq(eqId);
464488
setAdding(false);
465-
}, [chain, mutate]);
489+
}, [chain, mutate, onAuditionTransport]);
466490

467491
// Dragging a fader is heard immediately and written once on release, the same
468492
// split every other control in the rack uses.

packages/studio/src/player/hooks/useTimelinePlayer.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,8 +383,20 @@ export function useTimelinePlayer() {
383383
seek(state.requestedSeekTime);
384384
usePlayerStore.getState().clearSeekRequest();
385385
}
386+
// Play or stop from outside the loop — the FX rack auditioning a preset
387+
// while paused, which is silent otherwise. `returnTo` puts the playhead
388+
// back where the request found it: hovering is not an edit.
389+
const request = state.playbackRequest;
390+
if (request && request.nonce !== prev.playbackRequest?.nonce) {
391+
if (request.playing) play();
392+
else {
393+
pause();
394+
if (request.returnTo !== null) seek(request.returnTo);
395+
}
396+
usePlayerStore.getState().clearPlaybackRequest();
397+
}
386398
});
387-
}, [seek]);
399+
}, [seek, play, pause]);
388400
const { playbackKeyDownRef, playbackKeyUpRef, attachIframeShortcutListeners, togglePlay } =
389401
usePlaybackKeyboard({
390402
iframeRef,

packages/studio/src/player/store/playerStore.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,22 @@ interface PlayerState extends KeyframeSlice, AutomationSelectionSlice {
163163
requestSeek: (time: number) => void;
164164
clearSeekRequest: () => void;
165165

166+
/**
167+
* Request the transport start or stop from outside the player loop.
168+
*
169+
* The FX rack auditions a preset by writing it to the running graph, which is
170+
* silent while the transport is paused — so hovering one has to start
171+
* playback, and leaving has to put the playhead back where it was. Hovering is
172+
* not an edit and must not cost the author their place.
173+
*
174+
* A nonce rather than a bare boolean: two hovers in a row both want play, and
175+
* without it the second request is indistinguishable from the first having
176+
* already been served.
177+
*/
178+
playbackRequest: { playing: boolean; returnTo: number | null; nonce: number } | null;
179+
requestPlayback: (playing: boolean, returnTo?: number | null) => void;
180+
clearPlaybackRequest: () => void;
181+
166182
/**
167183
* Request the timeline to scroll a clip into view (e.g. clicking an
168184
* already-added asset card in the sidebar). Consumed and cleared by
@@ -336,6 +352,13 @@ export const usePlayerStore = create<PlayerState>((set, get) => ({
336352
requestSeek: (time) => set({ requestedSeekTime: time }),
337353
clearSeekRequest: () => set({ requestedSeekTime: null }),
338354

355+
playbackRequest: null,
356+
requestPlayback: (playing, returnTo = null) =>
357+
set((s) => ({
358+
playbackRequest: { playing, returnTo, nonce: (s.playbackRequest?.nonce ?? 0) + 1 },
359+
})),
360+
clearPlaybackRequest: () => set({ playbackRequest: null }),
361+
339362
clipRevealRequest: null,
340363
requestClipReveal: (elementId) =>
341364
set((s) => ({

0 commit comments

Comments
 (0)