Android Tango depth camera capture support.

Android Tango depth camera capture support.

3D pointcloud that is synchronized to color 3D space is projected to 2D
depth frame with the color 2D frame aspect ratio.

BUG=674440

[aleksandar.stojiljkovic, 2017-07-13 12:29:10]

Patchset #1 (id:1) has been deleted

[aleksandar.stojiljkovic, 2017-07-13 22:10:12]

Patchset #1 (id:20001) has been deleted

[aleksandar.stojiljkovic, 2017-07-14 12:00:37]

Patchset #1 (id:40001) has been deleted

[aleksandar.stojiljkovic, 2017-07-14 13:19:24]

aleksandar.stojiljkovic@intel.com changed reviewers:
+ mcasas@chromium.org

[aleksandar.stojiljkovic, 2017-07-14 13:19:25]

mcasas@ PTAL.
Thanks.

[mcasas, 2017-07-15 00:12:37]

This CL is pointing in the right direction but is large-ish,
I'd say keep this CL as reference and start getting it 
reviewed in chunks? In particular the //third_party needs
to be splintered off, and you can get reviewed and landed
the refactoring in video_capture_device_android already.

Also, use Gerrit? :-)

https://codereview.chromium.org/2983473002/diff/60001/media/capture/video/and...
File
media/capture/video/android/java/src/org/chromium/media/VideoCaptureTango.java
(right):

https://codereview.chromium.org/2983473002/diff/60001/media/capture/video/and...
media/capture/video/android/java/src/org/chromium/media/VideoCaptureTango.java:181:
} catch (TangoInvalidException e) {
I think you could bundle these three and Log() will
print enough information to discern them.

https://codereview.chromium.org/2983473002/diff/60001/media/capture/video/and...
File media/capture/video/android/video_capture_device_android.cc (right):

https://codereview.chromium.org/2983473002/diff/60001/media/capture/video/and...
media/capture/video/android/video_capture_device_android.cc:403: if
(!got_first_frame_) {
if (got_first_frame)
  return;

https://codereview.chromium.org/2983473002/diff/60001/media/capture/video/and...
File media/capture/video/android/video_capture_device_android.h (right):

https://codereview.chromium.org/2983473002/diff/60001/media/capture/video/and...
media/capture/video/android/video_capture_device_android.h:117: protected:
Can we make a CL just for these small extractions? Thx

https://codereview.chromium.org/2983473002/diff/60001/media/capture/video/and...
File media/capture/video/android/video_capture_device_tango_android.cc (right):

https://codereview.chromium.org/2983473002/diff/60001/media/capture/video/and...
media/capture/video/android/video_capture_device_tango_android.cc:39: const
base::TimeDelta capture_time = base::TimeDelta::FromSecondsD(timestamp);
Move this to the vicinity of l.174? Try to allocate temp variables
close to where they'll be used.

https://codereview.chromium.org/2983473002/diff/60001/media/capture/video/and...
media/capture/video/android/video_capture_device_tango_android.cc:49:
std::unique_ptr<uint8_t> buffer(new uint8_t[buffer_length]);
Since we keep ownership of |buffer|, it might be good
to hold on to it and just alloc once / resize it here if needed.

https://codereview.chromium.org/2983473002/diff/60001/media/capture/video/and...
File media/capture/video/android/video_capture_device_tango_android.h (right):

https://codereview.chromium.org/2983473002/diff/60001/media/capture/video/and...
media/capture/video/android/video_capture_device_tango_android.h:1: // Copyright
(c) 2017 The Chromium Authors. All rights reserved.
No (c) anymore, and 2017 in VideoCaptureTango.java.

https://codereview.chromium.org/2983473002/diff/60001/media/capture/video/and...
File media/capture/video/android/video_capture_device_tango_android_unittest.cc
(right):

https://codereview.chromium.org/2983473002/diff/60001/media/capture/video/and...
media/capture/video/android/video_capture_device_tango_android_unittest.cc:29:
model == "Lenovo PB2-690M");
Usually these name lists are moved out as static arrays:

static const  char* const kKnownTangoModelNames[] = 
  "TheOne",
  "TheOther"
};

static bool IsTangoDevice() {
  const std::string model = base::SysInfo::HardwareModelName();
  for (const auto& known_model : kKnownTangoModelNames) {
    if (model == known_model)
      return true;
  }
  return false;
}

https://codereview.chromium.org/2983473002/diff/60001/third_party/TangoSDK/RE...
File third_party/TangoSDK/README.chromium (right):

https://codereview.chromium.org/2983473002/diff/60001/third_party/TangoSDK/RE...
third_party/TangoSDK/README.chromium:1: Name: Tango SDK java files
Landing a new //third_party/ is a bit tougher than this, including
licensing review etc:
https://chromium.googlesource.com/chromium/src.git/+/master/docs/adding_to_th...

[aleksandar.stojiljkovic, 2017-07-15 17:48:15]

On 2017/07/15 00:12:37, mcasas wrote:
> This CL is pointing in the right direction but is large-ish,
> I'd say keep this CL as reference and start getting it 
> reviewed in chunks? In particular the //third_party needs
> to be splintered off, and you can get reviewed and landed
> the refactoring in video_capture_device_android already.
> 
> Also, use Gerrit? :-)
> 

Thanks for review.
Nice, and the plan is good.

Split patches:

https://chromium-review.googlesource.com/c/573040/ VideoCaptureDeviceAndroid:
extract common code to methods.
https://chromium-review.googlesource.com/c/573060/ VideoCaptureDeviceTest: skip
the test for Tango devices

I'll try to get rid of 3rd party jar file now - it is small as it doesn't do
much but connects to service and load native library. It is also closed source.
Might make more sense to do the same thing (connecting to service and dlsym to
native library) in C++ and drop the third party jar file.

[aleksandar.stojiljkovic, 2017-07-24 08:16:59]

On 2017/07/15 17:48:15, aleksandar.stojiljkovic wrote:
> On 2017/07/15 00:12:37, mcasas wrote:
> > This CL is pointing in the right direction but is large-ish,
> > I'd say keep this CL as reference and start getting it 
> > reviewed in chunks? In particular the //third_party needs
> > to be splintered off, and you can get reviewed and landed
> > the refactoring in video_capture_device_android already.
> > 
> > Also, use Gerrit? :-)
> > 
> 
> Thanks for review.
> Nice, and the plan is good.
> 
> Split patches:
> 
> https://chromium-review.googlesource.com/c/573040/ VideoCaptureDeviceAndroid:
> extract common code to methods.
> https://chromium-review.googlesource.com/c/573060/ VideoCaptureDeviceTest:
skip
> the test for Tango devices
> 
> I'll try to get rid of 3rd party jar file now - it is small as it doesn't do
> much but connects to service and load native library. It is also closed
source.
> Might make more sense to do the same thing (connecting to service and dlsym to
> native library) in C++ and drop the third party jar file.

mcasas@chromium.org,
The patch #2 removes third party requirement of Tango Java  API. Instead, it
dlsym-s to needed C++ API symbols in libtango_client_api that is already on the
Tango device (Tango Core).
Using C API is forward looking as Java API doesn't allow access to RGB&Fisheye
frames pixel data - only Tango C API does [1].
Also, less code that is not unit tested.

https://developers.google.com/tango/apis/c/reference/group/camera#group___cam...
 
I'll continue pushing the split parts of it out to separate parches but PTAL
meanwhile. Thanks.

https://codereview.chromium.org/2983473002/diff/80001/media/capture/video/and...
File media/capture/video/android/tango_api.cc (right):

https://codereview.chromium.org/2983473002/diff/80001/media/capture/video/and...
media/capture/video/android/tango_api.cc:14: std::unique_ptr<TangoApi>
TangoApi::loadAndCreate() {
Note that WebAR code (I copied the native Tango initialization approach from)
uses multiple fallback locations [1] for locating libtango_client_api.so, and
here I use only one.
I'll extend it later - this one works fine with tango development kit tablet and
Lenovo Phab 2 Pro. 

https://github.com/googlevr/chromium-webar/blob/9e2420ffb2870a7cc8095ad444082...

[mcasas, 2017-07-26 01:48:07]

On 2017/07/24 08:16:59, aleksandar.stojiljkovic wrote:
> On 2017/07/15 17:48:15, aleksandar.stojiljkovic wrote:
> > On 2017/07/15 00:12:37, mcasas wrote:
> > > This CL is pointing in the right direction but is large-ish,
> > > I'd say keep this CL as reference and start getting it 
> > > reviewed in chunks? In particular the //third_party needs
> > > to be splintered off, and you can get reviewed and landed
> > > the refactoring in video_capture_device_android already.
> > > 
> > > Also, use Gerrit? :-)
> > > 
> > 
> > Thanks for review.
> > Nice, and the plan is good.
> > 
> > Split patches:
> > 
> > https://chromium-review.googlesource.com/c/573040/
VideoCaptureDeviceAndroid:
> > extract common code to methods.
> > https://chromium-review.googlesource.com/c/573060/ VideoCaptureDeviceTest:
> skip
> > the test for Tango devices
> > 
> > I'll try to get rid of 3rd party jar file now - it is small as it doesn't do
> > much but connects to service and load native library. It is also closed
> source.
> > Might make more sense to do the same thing (connecting to service and dlsym
to
> > native library) in C++ and drop the third party jar file.
> 
> mailto:mcasas@chromium.org,
> The patch #2 removes third party requirement of Tango Java  API. Instead, it
> dlsym-s to needed C++ API symbols in libtango_client_api that is already on
the
> Tango device (Tango Core).
> Using C API is forward looking as Java API doesn't allow access to RGB&Fisheye
> frames pixel data - only Tango C API does [1].
> Also, less code that is not unit tested.

Chromium calls these classes _glue.{cc,h} (see e.g. the late avfoundation_glue
in https://codereview.chromium.org/24615005/).  Make sure only used symbols
are extracted and that they are cached, so dlsym() is called only once, and that
where possibly, (contained) static handles are used.

> 
>
https://developers.google.com/tango/apis/c/reference/group/camera#group___cam...
>  
> I'll continue pushing the split parts of it out to separate parches but PTAL
> meanwhile. Thanks.
> 
>
https://codereview.chromium.org/2983473002/diff/80001/media/capture/video/and...
> File media/capture/video/android/tango_api.cc (right):
> 
>
https://codereview.chromium.org/2983473002/diff/80001/media/capture/video/and...
> media/capture/video/android/tango_api.cc:14: std::unique_ptr<TangoApi>
> TangoApi::loadAndCreate() {
> Note that WebAR code (I copied the native Tango initialization approach from)
> uses multiple fallback locations [1] for locating libtango_client_api.so, and
> here I use only one.
> I'll extend it later - this one works fine with tango development kit tablet
and
> Lenovo Phab 2 Pro. 
> 
>
https://github.com/googlevr/chromium-webar/blob/9e2420ffb2870a7cc8095ad444082...

[aleksandar.stojiljkovic, 2017-08-07 15:29:28]

On 2017/07/26 01:48:07, mcasas wrote:
> On 2017/07/24 08:16:59, aleksandar.stojiljkovic wrote:
> > On 2017/07/15 17:48:15, aleksandar.stojiljkovic wrote:
> > > On 2017/07/15 00:12:37, mcasas wrote:
> > > > This CL is pointing in the right direction but is large-ish,
> > > > I'd say keep this CL as reference and start getting it 
> > > > reviewed in chunks? In particular the //third_party needs
> > > > to be splintered off, and you can get reviewed and landed
> > > > the refactoring in video_capture_device_android already.
> > > > 
> > > > Also, use Gerrit? :-)
> > > > 
> > > 
> > > Thanks for review.
> > > Nice, and the plan is good.
> > > 
> > > Split patches:
> > > 
> > > https://chromium-review.googlesource.com/c/573040/
> VideoCaptureDeviceAndroid:
> > > extract common code to methods.
> > > https://chromium-review.googlesource.com/c/573060/ VideoCaptureDeviceTest:
> > skip
> > > the test for Tango devices
> > > 
> > > I'll try to get rid of 3rd party jar file now - it is small as it doesn't
do
> > > much but connects to service and load native library. It is also closed
> > source.
> > > Might make more sense to do the same thing (connecting to service and
dlsym
> to
> > > native library) in C++ and drop the third party jar file.
> > 
> > mailto:mcasas@chromium.org,
> > The patch #2 removes third party requirement of Tango Java  API. Instead, it
> > dlsym-s to needed C++ API symbols in libtango_client_api that is already on
> the
> > Tango device (Tango Core).
> > Using C API is forward looking as Java API doesn't allow access to
RGB&Fisheye
> > frames pixel data - only Tango C API does [1].
> > Also, less code that is not unit tested.
> 
> Chromium calls these classes _glue.{cc,h} (see e.g. the late avfoundation_glue
> in https://codereview.chromium.org/24615005/).  Make sure only used symbols
> are extracted and that they are cached, so dlsym() is called only once, and
that
> where possibly, (contained) static handles are used.
> 

mcasas@,
Done - Patch Set 3 : rename tango_api files to tango_client_api_glue.

Replaced lib and api member variable with static methods in API glue class (like
in the referred patch) makes it cleaner.
Thanks.