Skip to content

Tracking on created streams #73

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 7, 2021
Merged

Tracking on created streams #73

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6cf647d
Fetching camera stream is moved in common method who will also track …
Oct 28, 2021
46f5bd4
getUserMedia() function always return new stream instance but keep tr…
Oct 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 35 additions & 6 deletions src/readers/BrowserCodeReader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
Reader,
Result,
} from '@zxing/library';
import { ErrorCorrectionLevelValues } from '@zxing/library/esm/core/qrcode/decoder/ErrorCorrectionLevel';
import { DecodeContinuouslyCallback } from '../common/DecodeContinuouslyCallback';
import { HTMLCanvasElementLuminanceSource } from '../common/HTMLCanvasElementLuminanceSource';
import { HTMLVisualMediaElement } from '../common/HTMLVisualMediaElement';
Expand Down Expand Up @@ -96,7 +95,9 @@ export class BrowserCodeReader {
} catch (err) {
// some browsers may not be compatible with ImageCapture
// so we are ignoring this for now.
// tslint:disable-next-line:no-console
console.error(err);
// tslint:disable-next-line:no-console
console.warn('Your browser may be not fully compatible with WebRTC and/or ImageCapture specs. Torch will not be available.');
return false;
}
Expand Down Expand Up @@ -414,6 +415,19 @@ export class BrowserCodeReader {
}
}

/**
* Stops all media streams that are created.
*/
public static releaseAllStreams() {
if (BrowserCodeReader.streamTracker.length !== 0) {
// tslint:disable-next-line:no-console
BrowserCodeReader.streamTracker.forEach((mediaStream) => {
mediaStream.getTracks().forEach((track) => track.stop());
});
}
BrowserCodeReader.streamTracker = [];
}

/**
* Waits for a video to load and then hits play on it.
* To accomplish that, it binds listeners and callbacks to the video element.
Expand Down Expand Up @@ -480,6 +494,12 @@ export class BrowserCodeReader {
return videoElement;
}

/**
* Keeps track to created media streams.
* @private there is no need this array to be accessible from outside.
*/
private static streamTracker: MediaStream[] = [];

/**
* Returns a Promise that resolves when the given image element loads.
*/
Expand Down Expand Up @@ -525,7 +545,7 @@ export class BrowserCodeReader {
* Standard method to dispose a media stream object.
*/
private static disposeMediaStream(stream: MediaStream) {
stream.getVideoTracks().forEach(x => x.stop());
stream.getVideoTracks().forEach((x) => x.stop());
stream = undefined;
}

Expand Down Expand Up @@ -633,8 +653,7 @@ export class BrowserCodeReader {

BrowserCodeReader.checkCallbackFnOrThrow(callbackFn);

const stream = await navigator.mediaDevices.getUserMedia(constraints);

const stream = await this.getUserMedia(constraints);
try {
return await this.decodeFromStream(stream, previewElem, callbackFn);
} catch (error) {
Expand Down Expand Up @@ -768,7 +787,6 @@ export class BrowserCodeReader {
}

const constraints: MediaStreamConstraints = { video: videoConstraints };

return await this.decodeFromConstraints(constraints, previewElem, callbackFn);
}

Expand Down Expand Up @@ -849,7 +867,7 @@ export class BrowserCodeReader {
videoSource?: string | HTMLVideoElement,
): Promise<Result> {

const stream = await navigator.mediaDevices.getUserMedia(constraints);
const stream = await this.getUserMedia(constraints);

return await this.decodeOnceFromStream(stream, videoSource);
}
Expand Down Expand Up @@ -1092,4 +1110,15 @@ export class BrowserCodeReader {

return this.decode(element);
}

/**
* Get MediaStream from browser to be used.
* @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> {
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I like your approach and it looks simple. I'm afraid that this may break existing users who depend on multi camera setups.

Copy link
Author

Choose a reason for hiding this comment

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

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.

const stream = await navigator.mediaDevices.getUserMedia(constraints);
BrowserCodeReader.streamTracker.push(stream);
return stream;
}
}