From ebb6f1b602a0f5fb0d2c799db551d4a213c11602 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 24 Jun 2021 20:18:50 -0600 Subject: [PATCH] Add seeking and notes about clock desync --- res/css/views/audio_messages/_SeekBar.scss | 2 + .../views/audio_messages/AudioPlayer.tsx | 7 ++ .../views/audio_messages/SeekBar.tsx | 14 +++- src/voice/Playback.ts | 81 ++++++++++++++++--- src/voice/PlaybackClock.ts | 56 ++++++++++++- 5 files changed, 143 insertions(+), 17 deletions(-) diff --git a/res/css/views/audio_messages/_SeekBar.scss b/res/css/views/audio_messages/_SeekBar.scss index 23fc0bf5db..ca3f7aa562 100644 --- a/res/css/views/audio_messages/_SeekBar.scss +++ b/res/css/views/audio_messages/_SeekBar.scss @@ -31,6 +31,8 @@ limitations under the License. outline: none; // remove blue selection border position: relative; // for progress bar support later on + cursor: pointer; + &::-webkit-slider-thumb { appearance: none; // default style override diff --git a/src/components/views/audio_messages/AudioPlayer.tsx b/src/components/views/audio_messages/AudioPlayer.tsx index 336139e467..0767c814d8 100644 --- a/src/components/views/audio_messages/AudioPlayer.tsx +++ b/src/components/views/audio_messages/AudioPlayer.tsx @@ -41,6 +41,7 @@ interface IState { @replaceableComponent("views.audio_messages.AudioPlayer") export default class AudioPlayer extends React.PureComponent { private playPauseRef: RefObject = createRef(); + private seekRef: RefObject = createRef(); constructor(props: IProps) { super(props); @@ -66,6 +67,12 @@ export default class AudioPlayer extends React.PureComponent { if (ev.key === Key.SPACE) { ev.stopPropagation(); this.playPauseRef.current?.toggle(); + } else if (ev.key === Key.ARROW_LEFT) { + ev.stopPropagation(); + this.seekRef.current?.left(); + } else if (ev.key === Key.ARROW_RIGHT) { + ev.stopPropagation(); + this.seekRef.current?.right(); } }; diff --git a/src/components/views/audio_messages/SeekBar.tsx b/src/components/views/audio_messages/SeekBar.tsx index 2561321df3..09beed70b6 100644 --- a/src/components/views/audio_messages/SeekBar.tsx +++ b/src/components/views/audio_messages/SeekBar.tsx @@ -46,6 +46,8 @@ interface ISeekCSS extends CSSProperties { '--fillTo': number; } +const ARROW_SKIP_SECONDS = 5; // arbitrary + @replaceableComponent("views.audio_messages.SeekBar") export default class SeekBar extends React.PureComponent { // We use an animation frame request to avoid overly spamming prop updates, even if we aren't @@ -80,15 +82,21 @@ export default class SeekBar extends React.PureComponent { } public left() { - console.log("@@ LEFT"); + // noinspection JSIgnoredPromiseFromCall + this.props.playback.skipTo(this.props.playback.clockInfo.timeSeconds - ARROW_SKIP_SECONDS); } public right() { - console.log("@@ RIGHT"); + // noinspection JSIgnoredPromiseFromCall + this.props.playback.skipTo(this.props.playback.clockInfo.timeSeconds + ARROW_SKIP_SECONDS); } private onChange = (ev: ChangeEvent) => { - console.log('@@ CHANGE', ev.target.value); + // Thankfully, onChange is only called when the user changes the value, not when we + // change the value on the component. We can use this as a reliable "skip to X" function. + // + // noinspection JSIgnoredPromiseFromCall + this.props.playback.skipTo(Number(ev.target.value) * this.props.playback.clockInfo.durationSeconds); }; public render(): ReactNode { diff --git a/src/voice/Playback.ts b/src/voice/Playback.ts index f1f91310b2..6a120bf924 100644 --- a/src/voice/Playback.ts +++ b/src/voice/Playback.ts @@ -161,16 +161,9 @@ export class Playback extends EventEmitter implements IDestroyable { public async play() { // We can't restart a buffer source, so we need to create a new one if we hit the end if (this.state === PlaybackState.Stopped) { - if (this.source) { - this.source.disconnect(); - this.source.removeEventListener("ended", this.onPlaybackEnd); - } - - this.source = this.context.createBufferSource(); - this.source.connect(this.context.destination); - this.source.buffer = this.audioBuf; - this.source.start(); // start immediately - this.source.addEventListener("ended", this.onPlaybackEnd); + this.disconnectSource(); + this.makeNewSourceBuffer(); + this.source.start(); } // We use the context suspend/resume functions because it allows us to pause a source @@ -180,6 +173,18 @@ export class Playback extends EventEmitter implements IDestroyable { this.emit(PlaybackState.Playing); } + private disconnectSource() { + this.source?.disconnect(); + this.source?.removeEventListener("ended", this.onPlaybackEnd); + } + + private makeNewSourceBuffer() { + this.source = this.context.createBufferSource(); + this.source.buffer = this.audioBuf; + this.source.addEventListener("ended", this.onPlaybackEnd); + this.source.connect(this.context.destination); + } + public async pause() { await this.context.suspend(); this.emit(PlaybackState.Paused); @@ -194,4 +199,60 @@ export class Playback extends EventEmitter implements IDestroyable { if (this.isPlaying) await this.pause(); else await this.play(); } + + public async skipTo(timeSeconds: number) { + // Dev note: this function talks a lot about clock desyncs. There is a clock running + // independently to the audio context and buffer so that accurate human-perceptible + // time can be exposed. The PlaybackClock class has more information, but the short + // version is that we need to line up the useful time (clip position) with the context + // time, and avoid as many deviations as possible as otherwise the user could see the + // wrong time, and we stop playback at the wrong time, etc. + + timeSeconds = clamp(timeSeconds, 0, this.clock.durationSeconds); + + // Track playing state so we don't cause seeking to start playing the track. + const isPlaying = this.isPlaying; + + if (isPlaying) { + // Pause first so we can get an accurate measurement of time + await this.context.suspend(); + } + + // We can't simply tell the context/buffer to jump to a time, so we have to + // start a whole new buffer and start it from the new time offset. + const now = this.context.currentTime; + this.disconnectSource(); + this.makeNewSourceBuffer(); + + // We have to resync the clock because it can get confused about where we're + // at in the audio clip. + this.clock.syncTo(now, timeSeconds); + + // Always start the source to queue it up. We have to do this now (and pause + // quickly if we're not supposed to be playing) as otherwise the clock can desync + // when it comes time to the user hitting play. After a couple jumps, the user + // will have desynced the clock enough to be about 10-15 seconds off, while this + // keeps it as close to perfect as humans can perceive. + this.source.start(now, timeSeconds); + + // Dev note: it's critical that the code gap between `this.source.start()` and + // `this.pause()` is as small as possible: we do not want to delay *anything* + // as that could cause a clock desync, or a buggy feeling as a single note plays + // during seeking. + + if (isPlaying) { + // If we were playing before, continue the context so the clock doesn't desync. + await this.context.resume(); + } else { + // As mentioned above, we'll have to pause the clip if we weren't supposed to + // be playing it just yet. If we didn't have this, the audio clip plays but all + // the states will be wrong: clock won't advance, pause state doesn't match the + // blaring noise leaving the user's speakers, etc. + // + // Also as mentioned, if the code gap is small enough then this should be + // executed immediately after the start time, leaving no feasible time for the + // user's speakers to play any sound. + await this.pause(); + } + } } diff --git a/src/voice/PlaybackClock.ts b/src/voice/PlaybackClock.ts index 9c2d36923f..e3f41930de 100644 --- a/src/voice/PlaybackClock.ts +++ b/src/voice/PlaybackClock.ts @@ -18,7 +18,42 @@ import { SimpleObservable } from "matrix-widget-api"; import { IDestroyable } from "../utils/IDestroyable"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; -// Because keeping track of time is sufficiently complicated... +/** + * Tracks accurate human-perceptible time for an audio clip, as informed + * by managed playback. This clock is tightly coupled with the operation + * of the Playback class, making assumptions about how the provided + * AudioContext will be used (suspended/resumed to preserve time, etc). + * + * But why do we need a clock? The AudioContext exposes time information, + * and so does the audio buffer, but not in a way that is useful for humans + * to perceive. The audio buffer time is often lagged behind the context + * time due to internal processing delays of the audio API. Additionally, + * the context's time is tracked from when it was first initialized/started, + * not related to positioning within the clip. However, the context time + * is the most accurate time we can use to determine position within the + * clip if we're fast enough to track the pauses and stops. + * + * As a result, we track every play, pause, stop, and seek event from the + * Playback class (kinda: it calls us, which is close enough to the same + * thing). These events are then tracked on the AudioContext time scale, + * with assumptions that code execution will result in negligible desync + * of the clock, or at least no perceptible difference in time. It's + * extremely important that the calling code, and the clock's own code, + * is extremely fast between the event happening and the clock time being + * tracked - anything more than a dozen milliseconds is likely to stack up + * poorly, leading to clock desync. + * + * Clock desync can be dangerous for the stability of the playback controls: + * if the clock thinks the user is somewhere else in the clip, it could + * inform the playback of the wrong place in time, leading to dead air in + * the output or, if severe enough, a clock that won't stop running while + * the audio is paused/stopped. Other examples include the clip stopping at + * 90% time due to playback ending, the clip playing from the wrong spot + * relative to the time, and negative clock time. + * + * Note that the clip duration is fed to the clock: this is to ensure that + * we have the most accurate time possible to present. + */ export class PlaybackClock implements IDestroyable { private clipStart = 0; private stopped = true; @@ -41,6 +76,12 @@ export class PlaybackClock implements IDestroyable { } public get timeSeconds(): number { + // The modulo is to ensure that we're only looking at the most recent clip + // time, as the context is long-running and multiple plays might not be + // informed to us (if the control is looping, for example). By taking the + // remainder of the division operation, we're assuming that playback is + // incomplete or stopped, thus giving an accurate position within the active + // clip segment. return (this.context.currentTime - this.clipStart) % this.clipDuration; } @@ -49,7 +90,7 @@ export class PlaybackClock implements IDestroyable { } private checkTime = () => { - const now = this.timeSeconds; + const now = this.timeSeconds; // calculated dynamically if (this.lastCheck !== now) { this.observable.update([now, this.durationSeconds]); this.lastCheck = now; @@ -82,8 +123,9 @@ export class PlaybackClock implements IDestroyable { } if (!this.timerId) { - // case to number because the types are wrong - // 100ms interval to make sure the time is as accurate as possible + // cast to number because the types are wrong + // 100ms interval to make sure the time is as accurate as possible without + // being overly insane this.timerId = setInterval(this.checkTime, 100); } } @@ -92,6 +134,12 @@ export class PlaybackClock implements IDestroyable { this.stopped = true; } + public syncTo(contextTime: number, clipTime: number) { + this.clipStart = contextTime - clipTime; + this.stopped = false; // count as a mid-stream pause (if we were stopped) + this.checkTime(); + } + public destroy() { this.observable.close(); if (this.timerId) clearInterval(this.timerId);