-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix: fix position jump on playback rate change #4268
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| private bufferNode: AudioBufferSourceNode | null = null | ||
| private playStartTime = 0 | ||
| private playedDuration = 0 | ||
| private playbackPosition = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed playedDuration to playbackPosition to clarify that this variable represents a coordinate (offset) within the audio buffer, rather than elapsed wall-clock time.
| this.bufferNode.connect(this.gainNode) | ||
|
|
||
| let currentPos = this.playedDuration * this._playbackRate | ||
| let currentPos = this.playbackPosition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since playbackPosition now denotes the absolute position in the buffer, it is used directly as the starting offset without any rate-based scaling.
| this.paused = true | ||
| this.bufferNode?.stop() | ||
| this.playedDuration += this.audioContext.currentTime - this.playStartTime | ||
| this.playbackPosition += (this.audioContext.currentTime - this.playStartTime) * this._playbackRate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upon pausing, the wall-clock time elapsed since the last play start is multiplied by the playbackRate to calculate the actual "distance" covered in the track, which is then accumulated into playbackPosition.
| const wasPlaying = !this.paused | ||
| if (wasPlaying) this._pause() | ||
| this._playbackRate = value | ||
| if (wasPlaying) this._play() | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By calling _pause() before a rate change, we "snapshot" the current position as a buffer offset. Resuming with _play() ensures continuity in time calculation, preventing cursor jumps.
| return this.paused | ||
| ? this.playbackPosition | ||
| : this.playbackPosition + (this.audioContext.currentTime - this.playStartTime) * this._playbackRate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During playback, it returns the fixed starting point (playbackPosition) plus the real-time distance (elapsed time * rate). This prevents past accumulated data from being corrupted by the current rate setting.
|
|
||
| if (wasPlaying) this._pause() | ||
| this.playedDuration = value / this._playbackRate | ||
| this.playbackPosition = value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user-defined seek target (value) is an absolute value in seconds, so it is assigned directly to playbackPosition regardless of the playback rate.
|
@katspaugh Could you review this PR? |
katspaugh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one, thank you!
Short description
This PR fixes a bug where currentTime returns an incorrect value when playbackRate is modified, causing the progress tracking to jump abnormally.
Implementation details
How to test it
Screenshots
Here are videos changing playback rate dynamically:
Before:
2025-12-21.062929.mp4
After:
2025-12-21.062805.mp4
Checklist