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

Python c api update #1552

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

Conversation

JonathanESantos
Copy link
Contributor

@JonathanESantos JonathanESantos commented Mar 30, 2021

Fixes #40, #917

Description of the changes:

  • The public C API is modified:
    • k4a_depth_mode_t removed from public C API (retained in k4ainternal/modes.h) and replaced with function k4a_device_get_depth_mode() which returns a k4a_depth_mode_info_t that holds the equivalent mode enum value and additional info about that depth mode.
    • k4a_color_resolution_t removed from public C API (retained in k4ainternal/modes.h) and replaced with function k4a_device_get_color_mode() which returns a k4a_color_mode_info_t that holds the equivalent resolution enum value and additional info about that color resolution mode.
    • k4a_fps_t removed from public C API (retained in k4ainternal/modes.h) and replaced with function k4a_device_get_fps_mode() which returns a k4a_fps_mode_info_t that holds the equivalent fps enum value and additional info about that fps mode.
    • Added a function k4a_device_get_info() which returns some information about the attached device, including a list of capabilities that it supports (depth, color, IMU, etc.).
    • All functions that accepted as input a k4a_depth_mode_t, k4a_color_resolution_t, or k4a_fps_t have been modified to accept the input as an int. The value to pass in should be the corresponding mode_id field of the structures k4a_depth_mode_info_t, k4a_color_mode_info_t, or k4a_fps_mode_info_t, respectively.
  • The corresponding changes have been applied to the C# API.
  • The corresponding changes have been applied to the C++ API.
  • The corresponding changes have been applied to the Python API.
  • The k4aviewer support for microphone recording is conditionally compiled based on a compiler DEFINE, which by default is set NOT to compile microphone support into the k4aviewer.

Before submitting a Pull Request:

I tested changes on:

  • Windows
  • Linux
  • C API was tested with automated unit tests and functional tests.
  • C# API was tested with automated unit tests and functional tests.
  • C++ API is untested.
  • Python API is tested with new automated unit tests and functional tests added.
  • k4arecorder is manually tested for all allowed combinations of color, depth, and fps modes.
  • k4aviewer is manually tested for all allowed combinations of color, depth, and fps modes, including playback.

JonathanESantos and others added 30 commits January 28, 2021 12:00
TODO: Test the following functions:
k4a_calibration_3d_to_3d
k4a_calibration_2d_to_3d
k4a_calibration_3d_to_2d
k4a_calibration_2d_to_2d
k4a_calibration_color_2d_to_depth_2d
k4a_transformation_create
k4a_transformation_destroy
k4a_transformation_depth_image_to_color_camera
k4a_transformation_depth_image_to_color_camera_custom
k4a_transformation_color_image_to_depth_camera
k4a_transformation_depth_image_to_point_cloud
… for the transformation functions. The build_wheel.ps1 script will create a win-amd64 only python wheel.
… folder. By doing this, when a user "import k4a", only the exported symbols will be visible. Everything else will be under a _bindings symbol.
…o that it aligns more with k4a.h and k4atypes.h.
…e. This seems redundant since the subpackage _bindings already has an underscore.
…nd image functions and handles.

Adding test files.
… Calibration classes. Fixing shallow copy and deep copy of Image objects.
Fixing minor bugs in the Transformation class, and fixing issues with the unit tests regarding starting and stopping the imu and getting imu samples.
…to generate the html documentation for the Python API.
… in the example code image_transformations.py.
…tion using doxygen.

Fixing several bugs:
1. The build_wheel.ps1 had extra arguments that breaks the building of the wheel.
2. The Transformation class had some parameters renamed but the renamed vars were not changed in the functions accordingly.
3. The Transformation class removed an unneeded param in depth_image_to_color_camera_custom but did not remove it in the test code.
…and Linux. Adding a bash script to automate generating the wheel file in Ubuntu (Linux).
…ll try to load. In Windows, the library names must be k4a.dll and depthengine.dll. In Linux, the library names must be libk4a.so and libdepthengine.so.

If the libraries have different names, the user should create symlinks and point it to the actual library using relative addressing.
…sts. Also renaming the tests so that they can be filtered into unit, functional_fast, functional, and perf tests.
AntonClaytonBursch and others added 24 commits March 29, 2021 10:41
…ll, which makes it more obvious what the intent is rather than a simple cast.
…t users have direct access to the bitmap of capabilities.
…r file to consolidate the multiple definitions in various files into one file.
…n compilation unit and static library. This static library should only be used internally; external tools should use the k4a API.

There is one notable place where an external tool uses this backdoor approach to get mode information: the recorder and playback. This is because the information saved in a matroska file consists only of the mode ids and not the full mode info struct. Upon playback, the mode info struct is reconstructed without the presence of a device by using this backdoor approach using k4ainternal/modes.h rather than through k4a/k4a.h. The consequence of this is that it is tied to Azure Kinect modes and will not work for other devices that have different modes. The optimal solution is to save the full mode info structs in the matroska file and reconstructed upon reading.
…o_t field was changed from uint32_t to a union, and when k4ainternal/modes.h implementation was put in its own library unit.
Merge branch 'develop' into python_c_api_update

# Conflicts:
#	examples/fastpointcloud/main.cpp
#	examples/green_screen/main.cpp
#	examples/k4arecord_custom_track/main.c
#	examples/opencv_compatibility/main.cpp
#	examples/streaming/main.c
#	examples/transformation/main.cpp
#	examples/undistort/main.cpp
#	examples/viewer/opengl/main.cpp
#	src/csharp/SDK/ColorModeInfo.cs
#	src/csharp/SDK/DepthModeInfo.cs
#	src/csharp/SDK/DeviceInfo.cs
#	src/csharp/SDK/FPSModeInfo.cs
#	src/csharp/Tests/UnitTests/DeviceFunctionTests.cs
#	src/python/k4a/build_wheel.csh
#	src/python/k4a/build_wheel.ps1
#	src/python/k4a/docs/building.md
#	src/python/k4a/docs/examples.md
#	src/python/k4a/docs/testing.md
#	src/python/k4a/examples/image_transformations.py
#	src/python/k4a/examples/simple_viewer.py
#	src/python/k4a/examples/the_basics.py
#	src/python/k4a/src/k4a/__init__.py
#	src/python/k4a/src/k4a/_bindings/calibration.py
#	src/python/k4a/src/k4a/_bindings/device.py
#	src/python/k4a/src/k4a/_bindings/image.py
#	src/python/k4a/src/k4a/_bindings/k4a.py
#	src/python/k4a/src/k4a/_bindings/k4atypes.py
#	src/python/k4a/src/k4a/_libs/README.md
#	src/python/k4a/tests/test_config.py
#	src/python/k4a/tests/test_functional_api_azurekinect.py
#	src/python/k4a/tests/test_functional_ctypes_azurekinect.py
#	src/python/k4a/tests/test_unit_k4atypes.py
#	tools/k4arecorder/main.cpp
#	tools/k4aviewer/k4adevicedockcontrol.cpp
…t so that in the logger callback, the context is passed in as the type of python object that the user passed in to the DLL. This makes it easier for the user since there is no casting required to get back the context that is passed in.
Copy link

@resuther-msft resuther-msft left a comment

Choose a reason for hiding this comment

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

  1. Was able to run and pass all tests except for 9 transformation tests and 3 Ctypes tests. Transformation tests failing because some tests are not defined. Ctypes tests failing assertions. (see attached test results)

  2. Script to build wheel should use script root for pip install commands to enable execution from anywhere.

  3. Several lines in file src/python/k4a/src/k4a/_bindings/k4a.py should have spaces added surrounding = to maintain consistency.
    akdk_PR1552_unit_tests_results.txt
    Lines: 166, 172, 178, 184, 190, 196, 202, 208, 218, 233, 242, 248, 254, 260, 266, 272, 284, 290, 296, 302, 308, 314, 326, 332, 338, 344, 356.

  4. Otherwise the python implementation looks good and all three examples run great.

@@ -1,5 +1,6 @@
from ._bindings.k4atypes import *
from ._bindings.device import Device
from ._bindings.device import logging_message_cb, set_logging_callback

Choose a reason for hiding this comment

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

Is there a reason you have lines 2 and 3 separate? ie. a reason not to combine the lines as "from ._bindings.device import Device, logging_message_cb, set_logging_callback"

# const k4a_depth_mode_t depth_mode,
# const k4a_color_resolution_t color_resolution,
# const uint32_t depth_mode_id,
# const uint3_t color_mode_id,

Choose a reason for hiding this comment

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

This should be uint32_t not uint3_t

camera_fps:EFramesPerSecond=EFramesPerSecond.FPS_5,
color_mode_id:int=0, # 720P
depth_mode_id:int=0, # OFF
fps_mode_id:int=0, # FPS_0

Choose a reason for hiding this comment

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

Is default FPS really Zero (not Five)?


# Just return success.
status = k4a.EBufferStatus.SUCCEEDED
return status

Choose a reason for hiding this comment

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

No newline at end of file


self.assertTrue(k4a.K4A_SUCCEEDED(status))
self.assertEqual(valid_int_flag.value, 1)

if __name__ == '__main__':
unittest.main()

Choose a reason for hiding this comment

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

No newline at the end of file


s = capability.__str__()
self.assertIsInstance(s, str)


if __name__ == '__main__':
unittest.main()

Choose a reason for hiding this comment

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

No newline at the end of file

@@ -0,0 +1,299 @@
# Changes to the Kinect SDK API surface

Choose a reason for hiding this comment

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

Should have additional formatting to make the document easier to read. ie using code blocks for code, enum definitions, and struct definitions.

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.

There is no programmatic way to obtain width/height from a resolution enum
5 participants