Skip to content
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

feature: play/pause and running promise modified #9

Merged
merged 3 commits into from
Dec 3, 2024

Conversation

JoaquinBCh
Copy link
Contributor

This PR adds the ability to pause and resume playback in the player.

  • Modified the play() method, which toggles between pausing and resuming playback:
  • On pause, it unsubscribes from the current audio and video tracks.
  • On resume, it resubscribes to the audio and video tracks.
  • Adjusted the handling of this.#running and the #runTrack tasks to allow resubscription after pausing.
  • Can now click either the Play/Pause button or directly on the player (canvas) to toggle playback.
    image

@englishm
Copy link
Owner

Thanks, @JoaquinBCh!

As mentioned on Discord, as much fun as it is to data mosh big buck bunny by pausing and playing and applying P frames to old state, I think we probably want something to ensure we restart playback on a keyframe.

bbb_datamosh.mp4

#3 added some code along these lines:

//At the start of decode , VideoDecoder seems to expect a key frame after configure() or flush()
if (this.#decoder.state == "configured") {
if (this.#waitingForKeyframe && !frame.sample.is_sync) {
console.warn("Skipping non-keyframe until a keyframe is found.")
return
}
// On arrival of a keyframe, allow decoding and stop waiting for a keyframe.
if (frame.sample.is_sync) {
this.#waitingForKeyframe = false
}
const chunk = new EncodedVideoChunk({
type: frame.sample.is_sync ? "key" : "delta",
data: frame.sample.data,
timestamp: frame.sample.dts / frame.track.timescale,
})
this.#decoder.decode(chunk)
}

But, I think because of how this shuffles around the tasks to actually run the tracks, that code isn't working in the same way anymore here.

Once we can sort out how to ensure a cleaner restart of playback here, I'm happy to merge this in.
Thanks again for the contribution!

@nicolaslevy
Copy link
Collaborator

Nice! Just in case, we have to test with ./dev/pub_multi_track to see "better" the issue with the Key frames

@JoaquinBCh
Copy link
Contributor Author

JoaquinBCh commented Nov 26, 2024

I set to true this.#waitingForKeyframe when pause

bbb_pause_play.webm.mp4

@englishm
Copy link
Owner

englishm commented Dec 3, 2024

Tested again, and this appears to be properly ensuring we always wait and reinitialize with keyframes now. Thanks!

@englishm englishm merged commit e3f0461 into englishm:main Dec 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants