feat: listen to track events and additional updates to stream classes #57
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bbaldino
reviewed
Jul 13, 2023
src/media/local-stream.ts
Outdated
| if (this.inputTrack.id === newTrack.id) { | ||
| this._outputStream = this.inputStream; | ||
| } else if (this.inputTrack.id === this.outputTrack.id) { | ||
| this._outputStream = new MediaStream([newTrack]); |
There was a problem hiding this comment.
We have to preserve the output stream, since that's what callers will have a handle to. So we actually need to create a new stream that'll be used as the input stream, and move the current input/output track to that, and then add the new track to the existing outputstream. (Tweak the comment as well)
There was a problem hiding this comment.
I'd think enabling background blur or something should show that this doesn't work: the local rendering of the video wouldn't show the blur since the outputstream changes?
bbaldino
reviewed
Jul 13, 2023
src/media/local-stream.ts
Outdated
Comment on lines
102
to
115
| // input track is same as old output track, so separate the input stream into a new stream | ||
| if (this.inputTrack.id === this.outputTrack.id) { | ||
| this.inputStream = new MediaStream(this.inputStream); | ||
| } | ||
|
|
||
| this.outputStream.removeTrack(this.outputTrack); | ||
| this.outputStream.addTrack(newTrack); | ||
|
|
||
| // input track is same as new output track, so set the streams to be the same | ||
| if (this.inputTrack.id === this.outputTrack.id) { | ||
| this.inputStream = this.outputStream; | ||
| } | ||
|
|
||
| this[LocalStreamEventNames.OutputTrackChange].emit(newTrack); |
There was a problem hiding this comment.
I think be really verbose/descriptive with the comments here, the logic is pretty dense.
bbaldino
approved these changes
Jul 14, 2023
brycetham
added a commit
that referenced
this pull request
Jul 19, 2023
* feat: new stream classes initial commit (#49) * feat: new stream classes initial commit * refactor: originStream -> inputStream * feat: integrate new stream types (#50) * feat: replace LocalMicrophoneTrack with LocalMicrophoneStream * refactor: replace LocalDisplayTrack with LocalDisplayStream * refactor: replace LocalCameraTrack with LocalCameraStream * refactor: remove LocalTrack * refactor: rename originTrack -> inputTrack * feat: effects api (#52) * feat: add effects api * fix: add loadingeffects protection * test: localstream unit tests * chore: upgrade effects version * refactor: add methods and update events for stream classes (#55) * refactor: add support for wcme refactor * chore: minor updates to LocalStream and unit tests --------- Co-authored-by: Bryce Tham <[email protected]> * feat: limit appliable constraints (#56) * feat: limit appliable constraints * refactor: pick VideoDeviceConstraints from MediaTrackConstraints --------- Co-authored-by: Bryce Tham <[email protected]> * feat: listen to track events and additional updates to stream classes (#57) * feat: listen to track events and additional updates to stream classes * fix: preserve outputStream in changeOutputTrack * chore: change outputStream to readonly * chore: update unit tests and comments --------- Co-authored-by: Bryce Tham <[email protected]> * chore: update docs and errors in device management API * fix: stop outputTrack on LocalStream stop --------- Co-authored-by: bbaldino <[email protected]> Co-authored-by: Tyler McCarthy <[email protected]> Co-authored-by: Bryce Tham <[email protected]>
brycetham
added a commit
that referenced
this pull request
Jul 19, 2023
* feat: new stream classes initial commit (#49) * feat: new stream classes initial commit * refactor: originStream -> inputStream * feat: integrate new stream types (#50) * feat: replace LocalMicrophoneTrack with LocalMicrophoneStream * refactor: replace LocalDisplayTrack with LocalDisplayStream * refactor: replace LocalCameraTrack with LocalCameraStream * refactor: remove LocalTrack * refactor: rename originTrack -> inputTrack * feat: effects api (#52) * feat: add effects api * fix: add loadingeffects protection * test: localstream unit tests * chore: upgrade effects version * refactor: add methods and update events for stream classes (#55) * refactor: add support for wcme refactor * chore: minor updates to LocalStream and unit tests --------- Co-authored-by: Bryce Tham <[email protected]> * feat: limit appliable constraints (#56) * feat: limit appliable constraints * refactor: pick VideoDeviceConstraints from MediaTrackConstraints --------- Co-authored-by: Bryce Tham <[email protected]> * feat: listen to track events and additional updates to stream classes (#57) * feat: listen to track events and additional updates to stream classes * fix: preserve outputStream in changeOutputTrack * chore: change outputStream to readonly * chore: update unit tests and comments --------- Co-authored-by: Bryce Tham <[email protected]> * chore: update docs and errors in device management API * fix: stop outputTrack on LocalStream stop --------- Co-authored-by: bbaldino <[email protected]> Co-authored-by: Tyler McCarthy <[email protected]> Co-authored-by: Bryce Tham <[email protected]> BREAKING CHANGE: convert track-based classes into stream-based classes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes several additional changes to the stream classes, including listening to media stream track events in
Stream, adding more abstract methods, and updating howinputTrackandoutputTrackare used inLocalStream.