Skip to content
This repository was archived by the owner on Feb 1, 2022. It is now read-only.

Improve examples a bit#154

Open
kenchris wants to merge 1 commit into
w3c:gh-pagesfrom
kenchris:gh-pages
Open

Improve examples a bit#154
kenchris wants to merge 1 commit into
w3c:gh-pagesfrom
kenchris:gh-pages

Conversation

@kenchris
Copy link
Copy Markdown

@kenchris kenchris commented Feb 22, 2017

Move to ES2017 and make the examples a bit easier to understand


Preview | Diff

@anssiko
Copy link
Copy Markdown
Member

anssiko commented Feb 22, 2017

Marked as non-substantive for IPR from ash-nazg.

@anssiko
Copy link
Copy Markdown
Member

anssiko commented Feb 22, 2017

Thanks @kenchris! (Fixes #57.)

PTAL @astojilj @huningxin.

@anssiko
Copy link
Copy Markdown
Member

anssiko commented Feb 22, 2017

(No need to worry about the build failure, it is a harmless warning of empty <span>s caused by MathJax integration used for formulas.)

Copy link
Copy Markdown
Contributor

@astojilj astojilj left a comment

Choose a reason for hiding this comment

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

@kenchris Thanks for help on this.

Comment thread index.html
body.appendChild(colorEl);
body.appendChild(depthEl);

// Assume that all our video inputs are depth stream capable.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can use videoKind to avoid the need for this assumptions - usually there is an integrated camera and this approach (via navigator.mediaDevices.enumerateDevices()) didn't work. I guess counting the MediaDeviceInfo that are videoinput with the same groupId would give a closer match but it still looks like a hack.

We shouldn't need to use enumerateDevices - first use videoKind to get depth stream, then get groupId from the depthStream's track and then use videoKind: color + groupId to get corresponding color device.
I didn't try this yet - it depends on experimental MediaStreamTrack.getSettings() - https://crbug.com/681824. Maybe it is already available through MediaTrackCapabilities though.

The implementation for videoKind has recently been added to Chromium: https://crrev.com/2664673002/.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Can you try to modify the example and test it there (I dont have the hardware), then I will update the example

Comment thread index.html
depthVideo.play();
}
);
const stream = await navigator.mediaDevices.getUserMedia(constraints);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like await. Is it a problem that it looks different compared to original approach in main spec?
https://w3c.github.io/mediacapture-main/#examples

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I would say no. This spec is newer and thus will use newer platform features.

Comment thread index.html
}).catch(function (reason) {
// handle gUM error here
});
</aside>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This approach is going to be deleted soon - reconstruction from split components is not supported.
We need to base the example on floats - like in https://software.intel.com/en-us/blogs/2017/02/10/depth-camera-capture-in-html5.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Let's change it to your demo as example instead then.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I can try to help simplify that as well but I cannot test here

@kenchris
Copy link
Copy Markdown
Author

Rebased, will have to look at this later... comments are of course always welcome

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants