Shubhra/ajs 137 add resampler to input#541
Conversation
|
| return typeof array === 'object' && !!(array as any)[READONLY_SYMBOL]; | ||
| } | ||
|
|
||
| export function resampleStream({ |
There was a problem hiding this comment.
We will probably need this in the plugins. That's why I kept it a separate utility.
There was a problem hiding this comment.
can you add a brief function spec to this function?
There was a problem hiding this comment.
Like a JS doc you mean?
There was a problem hiding this comment.
yeah, just a one or two line brief description would be fine
| } | ||
| }, | ||
| }); | ||
| return stream.pipeThrough(transformStream); |
There was a problem hiding this comment.
note that this will lock the input stream argument. When doing resource cleanup, we need to keep this in mind to avoid potential locking issue.
There was a problem hiding this comment.
this a good callout. I'll add a comment for the function.
There was a problem hiding this comment.
I wonder if we make it a class like "ResampledStream" and we expose method for unlock, cancel, just like deferred readable stream.
I think instead of using pipeThrough we should probably use manual getReader() to pipe the input stream to the transformStream just to make things consistent, because we might need to deal with bugs if using pipeThrough as we lose control of input stream's lock. What do you think?
There was a problem hiding this comment.
I haven't tried it yet, but feel free to try it.
| this.logger.debug({ track, publication, participant }, 'track subscribed'); | ||
| this.deferredStream.setSource(this.createStream(track)); | ||
| this.deferredStream.setSource( | ||
| resampleStream({ |
There was a problem hiding this comment.
^ same, we need to take care of un-pipeThrough whenever we are going to use it.
There was a problem hiding this comment.
What do you mean "un-pipeThrough"? If you call cancel on the output stream then it will signal it back down the chain?
There was a problem hiding this comment.
If we cancel the pipeThrough the source and destination stream will be cancelled as well and stop stream immediately, but in some situation, like deferred readable stream, when we"detach" source stream we still want to keep source stream "alive" and pipe to other stream later. Not sure if we'd ever want to "detach" source stream here tho
There was a problem hiding this comment.
This is a great point. Let me look through the code base and see what makes sense.
There was a problem hiding this comment.
when we"detach" source stream we still want to keep source stream "alive" and pipe to other stream later
you can do this via preventCancel/Abort? [source]
There was a problem hiding this comment.
readable = stream.pipeThrough({ writable, readable }[, { preventClose, preventAbort, preventCancel, signal }])
There was a problem hiding this comment.
I think we would set preventCancel to true in this case? 
There was a problem hiding this comment.
Yeah I think you're right. Going to test it with restaurant agent and handoff to make sure it works okay. Then I'll let you know.
There was a problem hiding this comment.
hand off worked correctly so I think we don't need it here.
2 participants
Re-sample the input based on user config