rico345100 / react-multimedia-capture

react-multimedia-capture is Multimedia capturing module via React, using HTML5 MediaDevice and MediaRecorder API
49 stars 14 forks source link

Made stopStream optional on stop #11

Closed Tseberechts closed 6 years ago

Tseberechts commented 6 years ago

For the same reason that I needed the stream to be passed on init, I don't want the stream to stop when I stop recording. I continuously show the stream, independently of recording.

You don't have to add this if you don't want to, but I like to use it this way.

I didn't have this problem before, because the stream didn't actually stop earlier.

rico345100 commented 6 years ago

Thank you for PR!

However I think the media stream should be stopped when the invoke stop method, literally.

I can understand you want to keep the media stream alive even it's stopped, because you already built your apps by this behavior, however this wasn't my intention.

There is already similar feature in it: Pause & Resume which is not stop the media stream, it just paused and resuming.

If you want to give the UI to the user to want to pressing stop, but not actually stop, you can just change the text of Pause Button, instead of actual Stop.

Stopping media record should be stop when they stopped, so this could be quite non-sense.

Feel free to show your thinking, I will keep listen your opinions!

Have a nice day!

Tseberechts commented 6 years ago

The recording should stop, but the stream should still be visible on the site.

You can view the stream that's coming from the webcam without it recording. This is for a video booth where you can see yourself and position yourself correctly, then start recording, stop recording (which is then uploaded to a server), next one can come to the video boot, position themselves correctly, start, stop, upload and so on.

Pause & Resume doesn't fulfill these conditions, as the video cannot be uploaded I then.

If the stream stops when the recording stops, we need to reload the page.

Still, you don't have to merge if you don't want to.

rico345100 commented 6 years ago

Thank you for detailed explanations!

Seems like your situation is enough to consider, I have to more think about this issue.

It's very late night here, so it's hard to think straight, I'll decide how to resolve this issue tomorrow!

Thanks!

rico345100 commented 6 years ago

Hello, I just merged your PR!

I just got back home and read your opinions, and I'm agree with the situations something like you faced.

Again, thanks for improve this module better!

I will update some more codes and publish to NPM soon!

Tseberechts commented 6 years ago

Thanks! You're very welcome!