Skip to content

Commit

Permalink
🐛 [Sound] Fix broken pause after speed change on FireFox
Browse files Browse the repository at this point in the history
  • Loading branch information
beefchimi committed Dec 29, 2023
1 parent 396cfb1 commit 4b56fcf
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
11 changes: 7 additions & 4 deletions src/Sound.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,17 @@ export class Sound extends EmittenCommon<SoundEventMap> {
this.#timestamp = Math.max(currentTime, tokens.minStartTime);
this.#elapsedSnapshot = this.#progress.elapsed;

// TODO: Not transitioning to new `speed` for now...
// this will be complicated given our `progress` calculations.
/*
linearRamp(
this.#source.playbackRate,
{from: oldSpeed, to: newSpeed},
{from: currentTime, to: currentTime},
// TODO: Not transitioning to new `speed` for now...
// this will be complicated given our `progress` calculations.
// {from: currentTime, to: currentTime + this.#fadeSec},
{from: currentTime, to: currentTime + this.#fadeSec},
);
*/

this.#source.playbackRate.value = newSpeed;
}

get loop() {
Expand Down
11 changes: 9 additions & 2 deletions src/tests/Sound.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,13 @@ describe('Sound component', () => {
expect(testSound.speed).toBe(tokens.maxSpeed);
});

it('sets value on `playbackRate`', async () => {
// TODO: Not using `linearRamp` at the moment because:
// 1. We are not usign "transitions" yet on `speed` change.
// 2. FireFox seems to have a problem pausing after a
// `speed` change when using `linearRamp`. Likely due to
// either the same `to/from > currentTime` or no call to
// `cancelScheduledValues()` before setting `tokens.pauseSpeed`.
it.skip('sets value on `playbackRate`', async () => {
const oldValue = testSound.speed;
const newValue = 2.2;

Expand All @@ -103,7 +109,8 @@ describe('Sound component', () => {
expect(spySourceRamp).toBeCalledWith(newValue, currentTime);
});

it('does not set value on playbackRate if paused', async () => {
// TODO: Skipping for same reasons as above.
it.skip('does not set value on playbackRate if paused', async () => {
const {currentTime} = defaultContext;
// TODO: This test should be changed to directly check that
// the `AudioParam` has the expected `tokens` value.
Expand Down

0 comments on commit 4b56fcf

Please sign in to comment.