Skip to content
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.

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

Video Reality – started #28

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from

Conversation

sarthaksrinivas
Copy link

Worked with @jv776 @DaoGatech to get video reality on web browsers

@jv776
Copy link
Contributor

jv776 commented Sep 7, 2016

Okay that actually makes a lot of sense. I'm not sure why I thought an iframe was necessary

@speigg
Copy link
Contributor

speigg commented Sep 7, 2016

The detectrtc.js library you found seems to be a bit heavy for our use case. Perhaps it's as simple as checking for the existence of navigator.mediaDevices.getUserMedia. That should work, right?

this.videoElement.width = this.iframeElement.width;
this.videoElement.height = this.iframeElement.height;
this.videoElement.width = '100%';
this.videoElement.height = '100%';
this.videoElement.controls = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure this code only runs in a DOM environment (wrap it in an if statement that checks for the existence of the document object)

@jv776
Copy link
Contributor

jv776 commented Sep 7, 2016

You're right, I can add a check that the document actually exists. I was thinking it might also be good to check if the DOM exists before registering the LiveVideoRealityLoader (that way Argon won't even try to use the LiveVideoRealityLoader in non-DOM environments).

@speigg
Copy link
Contributor

speigg commented Sep 7, 2016

I was thinking it might also be good to check if the DOM exists before registering the LiveVideoRealityLoader (that way Argon won't even try to use the LiveVideoRealityLoader in non-DOM environments).

No, don't do that. The LIveVideoRealityLoader is used in argon-app, which is a non-DOM environment.

@jv776
Copy link
Contributor

jv776 commented Sep 7, 2016

No, don't do that. The LIveVideoRealityLoader is used in argon-app, which is a non-DOM environment.

Okay. I'll just add the check to the constructor.

The detectrtc.js library you found seems to be a bit heavy for our use case. Perhaps it's as simple as checking for the existence of navigator.mediaDevices.getUserMedia. That should work, right?

I agree the DetectRTC library is pretty heavy and I'm not sure that I want to use it. The issue with testing the existence of getUserMedia is that it might exist but not actually provide any useful functionality. For example, on my desktop the function exists, but since there are no webcams or microphones connected to my desktop I can't get any MediaStream objects with getUserMedia.

So really there are two different things we need to test for: does the browser provide an implementation of getUserMedia, and, if it does, does the hardware actually have any cameras available. The first one is easy to test but the second one seems a little more complicated. There's also the issue that even if a webcam exists, the user might not give the app permission to use it, so getUserMedia will not return a MediaStream in that case either.

"typings": "index.d.ts",
"jspm": {
"format": "cjs",
"jspmPackage": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this jspmPackage flag removed?

@jv776
Copy link
Contributor

jv776 commented Sep 12, 2016

Why was this jspmPackage flag removed?

I think I updated my jspm installation at some point during development, which might also explain why the entire file was overwritten. I can add the flag back in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants