Tracking on created streams #73
Conversation
…all opened stream. All streams created by this object can be released ar once when wanted.
| * @param constraints constraints the media stream constraints to get s valid media stream to decode from. | ||
| * @private For private use. | ||
| */ | ||
| private async getUserMedia(constraints: MediaStreamConstraints): Promise<MediaStream> { |
There was a problem hiding this comment.
I like this approach. However, I have 2 scenarios to discuss.
Scenario #1.
Let's imagine a situation when I call to getUserMedia with one set of constraints. It's a first call, so the streamTracker cache is empty and I receive the correct results directly from the navigator.mediaDevices. Then after some time I make another call to getUserMedia with a different set of constraints. In this case my new constraints are ignored and getUserMedia will return cached results that are not valid for the new constraints. Do you agree with that?
Scenario #2.
How to handle situations when user media may change from time to time? For example, when a USB web camera is being connected or disconnected from PC? As far as I understand, currently all media results are cached and cache update is not available?
There was a problem hiding this comment.
You are completely right, i didn't take this scenario in my mind because i only had one device. I will provide better fix for it. Maybe always returning new stream and closing streams thats are not in use anymore to prevent memory leak. I will have some time this weekend hopefuly i will be able to take a look in it.
There was a problem hiding this comment.
Thank you so much! What you are doing is very welcome and valuable for the community! Perhaps always returning a new stream is a good approach. Maybe you will have better ideas. I don't )) But maybe we need some time to think over about it.
There was a problem hiding this comment.
Main issue is because we have a bunch of promise and await. I didn't want to make big changes in code so this solution was looking the simplest to me without touching everywhere. There is better solution for sure. This is kind of quick fix because is releasing all streams when component get destroyed.
There was a problem hiding this comment.
Yes, I like your approach and it looks simple. I'm afraid that this may break existing users who depend on multi camera setups.
There was a problem hiding this comment.
Stream is not cached anymore after creating. It is only tracked so we can close all of them later. It does not differ much from previous functionality.
…acks on all created streams, allowing us to release all of them when needed.
|
@werthdavid Can you please approve and merge this PR? |
|
@werthdavid thank you so much. Will you publish a new release yourself or should we make a PR to |
|
PR would be much appreciated, thank you!! |
|
@werthdavid Sorry, the PR will not work. Because P.S. I reviewed the following PRs that can also be merged:
Also added a couple PRs of mine: Please, have a look when you have time for that. Thank you! |
|
Can you check if v0.0.10 works as expected? Thanks for all the work! |
|
Thank you so much! I started to test. I will inform about the results. |
|
@werthdavid I made the tests. I could successfully run the Also, I found out, that latest version of https://github.com/zxing-js/library is v0.19.0, however it has not been published at npm. Is there any reason for that? So, I made the tests with v0.18.0. If you publish v.0.19.0, then I can test it, too. I also linked the locally built I believe you may merge the PR above, and then publish a new |
|
|
Thank you so much! The ngx-scanner version 3.3.0 from npm works like a charm! |
|
Good stuff! |
Fix for zxing-js/ngx-scanner#425