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

Add AbortSignal to prepareStream and playStream #153

Merged
merged 4 commits into from
Feb 20, 2025
Merged

Conversation

longnguyen2004
Copy link
Collaborator

@longnguyen2004 longnguyen2004 commented Feb 19, 2025

Adding this because while working on https://github.com/Discord-RE/FFmpeg-input-premade, I came across some cases where ending the stream is a bit more complicated than just a single command.kill, and having a unified way to end processes would be better for other users.

AbortSignal is chosen because many other Node APIs and libraries are using it as the way to abort processes.

// Global variable
let controller = new AbortController();

const { command, output } = prepareStream(url, { ... }, controller.signal);
await playStream(output, streamer, { ... }, controller.signal);

// End everything
controller.abort();

Copy link

pkg-pr-new bot commented Feb 19, 2025

Open in Stackblitz

npm i https://pkg.pr.new/Discord-RE/Discord-video-stream/@dank074/discord-video-stream@153

commit: d0346d8

@dank074
Copy link
Member

dank074 commented Feb 19, 2025

Yes this looks good.

I like that AbortController will end both methods. I think that's better

@longnguyen2004 longnguyen2004 changed the title Export a stop function from prepareStream Add AbortSignal to prepareStream and playStream Feb 19, 2025
@longnguyen2004
Copy link
Collaborator Author

Seems to work fine. Also we should release 5.0.0 after this

@longnguyen2004 longnguyen2004 marked this pull request as ready for review February 19, 2025 16:39
Copy link
Member

@dank074 dank074 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Will release 5.0 tonight

@dank074 dank074 merged commit 6434c25 into master Feb 20, 2025
5 checks 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.

2 participants