search

w3c / mediacapture-depth

the Media Capture Depth Stream Extensions specification
https://w3c.github.io/mediacapture-depth/
Other
24 stars 20 forks source link

Fix #110: Intrinsics and extrinsics #144

Closed astojilj closed 7 years ago

astojilj commented 7 years ago

Fix #110: Intrinsics and extrinsics

Principal point, distortion coefficients, depth to video transformation. Formulas for deprojection, transform and projection. TODO. shader example code.
Fix #110: Accessing Camera Calibrations

astojilj commented 7 years ago

@huningxin, @robman, @bbernhar, @fluffy, PTAL. Thanks.

astojilj commented 7 years ago

@huningxin, I know you are busy. Some feedback would be appreciated as I'm starting work on writing formulas for deprojection etc... Thanks.

huningxin commented 7 years ago

@astojilj , Thanks for your PR! It's my bad that I didn't comment here.

But I understand that we still have some opens in #110 and #135.

My suggestion would be only including the changes with consensus in this PR and leave the opens in following PRs. WDYT?

astojilj commented 7 years ago

@huningxin, @robman, @anssiko

But I understand that we still have some opens in #110 and #135.

135:

I don't see there is anything open in #135. All the comments and reporter confirm that this needs to be fixed and the PR here is a fix for it. in comment here @robman expresses agreement with my approach coded here in this PR.

110,

The only open there is @robman's proposal about which part of this PR should be v1 and which should be v2 scope. You also raised the question here about why to attempt to provide incomplete data only.

I don't think that PR should matter about v1/v2 scope split. Or in PR only we should put content that all the participants in issue page agree should be v1 and delete all v2 scope content?

huningxin commented 7 years ago

Then how about separate this PR into one for #135 and another one for #110. It would be more clear to me. Thanks!

astojilj commented 7 years ago

Yes, let me do so. Thanks.

astojilj commented 7 years ago

PR for #135 is split to https://github.com/w3c/mediacapture-depth/pull/146.

anssiko commented 7 years ago

Updated the title, since #135 was already fixed with https://github.com/w3c/mediacapture-depth/commit/e50bc08af1bc6e520d87dd66d6842191b322ac6c

astojilj commented 7 years ago

Updated. PTAL. Travis CI fails on acceptable "Warning: trimming empty " only.

astojilj commented 7 years ago

After peer review with Anssi:

astojilj commented 7 years ago

Updated the pull request by: 1) Removed device specific distortion models 2) added full formulas for depth deprojection, translating from depth to color coordinates and color projection with the optional distortion handling formular on both deptojection and projection. 3) transformation matrix was modeled using translation and quaternion for rotation - replaced them by simple 16 element array in the format suitable to be directly used in matrix x vector multiplication and with WebGL's uniformMatrix4fv.

Example code for color deprojection and projection I'll add in separate request - don't want to grow this one further.

anssiko commented 7 years ago

Thanks! I added some review comments.

In addition, you'll need to patch the build.js as follows to make MathJax happy with the complex formulas:

https://gist.github.com/anssiko/fd03fb67eb7b29dd1abecbac0ec43536

anssiko commented 7 years ago

Regarding checking the presence of dictionary members, we should actually check if the member is present, and refer to https://heycam.github.io/webidl/#dfn-present like this:

"If the <a>foobar</a> member is <a>present</a> in <a>baz dictionary</a>"

(Additionally, add appropriate <dfn>present</dfn> to Dependencies.

Related issue: https://github.com/heycam/webidl/issues/124

astojilj commented 7 years ago

present and not present in MediaTrackSettings dictionary in updated PR.

anssiko commented 7 years ago

Discussed with @astojilj and concluded "Synchronizing depth and color video rendering" section will be made non-normative guidance to web developers. Thus some of my earlier comments are not valid, since we should not use RFC terms in an informative section.

astojilj commented 7 years ago

PR updated. Section moved just before examples, Example vertex shader code added and links to it added around. @ds-hwang Kudos for the code - I used it many times last week.

astojilj commented 7 years ago

@anssiko PTAL

astojilj commented 7 years ago

PR Updated @anssiko PTAL.

Open issue:

Added this note:

Radial distortion coefficients and tangential distortion coefficients are used when there's need to deproject depth value to 3D space or to project 3D value to 2D video frame coordinates. See algorithm to map depth pixels to color pixels and Brown-Conrady distortion model implementation 3D point cloud rendering example GLSL shader.

Don't have a good reference to Brown-Conrady model . Wikipedia link?

anssiko commented 7 years ago

Thanks @astojilj! I'll land this now and do some editorial cleanup afterwards.