librealsense: refactor to allow use of cuda, and prepare for python integration #92997

freezeboy · 2020-07-12T12:02:30Z

Motivation for this change

librealsense can be compiled to use CUDA to improve some algorithms.
This patch adds two conditional parametes to add python support and cuda
support

also adds two variations (like tensorflow does):

pkgs.librealsense (original, uses config.cudaSupport dynamically)
pkgs.librealsenseWithCuda - forces cudaSupport
pkgs.librealsenseWithoutCuda - forces no cudaSupport

and for python:

pkgs.python3Packages.pyrealsense2
pkgs.python3Packages.pyrealsense2WithCuda
pkgs.python3Packages.pyrealsense2WithoutCuda

Things done

freezeboy · 2020-07-12T18:58:06Z

Cuda versions are currently subject to this bug: IntelRealSense/librealsense#6573 (comment)
I don't plan to apply the patch as another PR is proposing an update to 2.36

brian-dawn

I don't have an easy way to test these changes but it looks good to me!

brian-dawn · 2020-07-13T12:51:31Z

@freezeboy thanks for the changes. Since I don't use cuda with librealsense would you be interested in becoming a maintainer for the librealsense package?

freezeboy · 2020-07-13T16:30:12Z

Yeah, no problem on my side even if I don't use it a lot either I can help you

jonringer · 2020-07-13T18:19:17Z

pkgs/top-level/all-packages.nix

+    cudaSupport = true;
+  };
+
+  librealsenseWithoutCuda = callPackage ../development/libraries/librealsense {


I believe the current convention is just to have no suffix

Suggested change

librealsenseWithoutCuda = callPackage ../development/libraries/librealsense {

librealsense = callPackage ../development/libraries/librealsense {

the "default" version uses config to switch, whereas this one forces to always "not" use cuda

pkgs/development/libraries/librealsense/default.nix

pkgs/top-level/python-packages.nix

jonringer · 2020-07-13T18:22:42Z

pkgs/top-level/python-packages.nix

+    pythonPackages = self;
+  });
+
+  pyrealsense2WithoutCuda = toPythonModule (pkgs.librealsenseWithoutCuda.override {


Suggested change

pyrealsense2WithoutCuda = toPythonModule (pkgs.librealsenseWithoutCuda.override {

pyrealsense2 = toPythonModule (pkgs.librealsenseWithoutCuda.override {

pkgs/top-level/python-packages.nix

tpwrules · 2020-07-20T22:11:22Z

I made my own pyrealsense2 PR (mentioned above) without knowing this one existed.

Unfortunately, Intel changed the way they install the Python bindings with 2.36.0. Now that nixpkgs has moved to that version, this PR no longer builds using nixpkgs-review -pr 92997. You are welcome to take the patches I made in my PR to make yours work with 2.36.0. I don't have a way to test with CUDA, so it doesn't make sense to do it the other way.

I have a further patch to enable T265 support that will require consultation with the maintainers due to licensing issues. Not sure the best way to do that, but regardless I was going to wait until Python support was merged.

freezeboy · 2020-07-21T07:29:18Z

I'll look at your method as I didn't check this update problem

freezeboy · 2020-07-21T07:41:15Z

About the examples, I don't particularly care about building them as I don't need them, aren't they big ? should I add a flag to let the user remove them or it is not worth it ?

freezeboy · 2020-07-21T12:00:26Z

rs-color, rs-depth and rs-distance cannot find some symbols from stdcpp library, e.g. :
librealsense2.so.2.36: undefined symbol: _ZNSt7__cxx1119basic_ostringstreamIcSt11char_traitsIcESaIcEEC1Ev

Do you know how this package is named in nix ?

tpwrules · 2020-07-21T15:14:43Z

Following up on our conversation on IRC:

You said you got the undefined symbol error resolved?
The compiled command line example binaries sum to under 2 megabytes and are useful for testing and troubleshooting. I think they should be unconditionally built as there's no real penalty.
Regarding the required Python packages, I believe you are correct, only python is required. Both setuptools and wheel can be removed. They were remnants of earlier experiments.

freezeboy · 2020-07-21T16:00:29Z

I added gcc.cc.lib as a buildInput to see if it is better
No problem then
Ok I cleanup

tpwrules · 2020-07-21T18:36:13Z

I reviewed the latest commit with nixpkgs-review and the examples build and run fine. The Python module is built fine, but it doesn't appear to Python. I believe this is because python should be moved to propagatedBuildInputs so toPythonModule picks the module up.

After doing nix-shell -p nixpkgs-review --run "nixpkgs-review pr 92997 -p python38Packages.pyrealsense2", you should be able to print out any attached RealSense devices using the examples with results/python38Packages.pyrealsense2/bin/rs-enumerate-devices or with Python using python3 -c 'import pyrealsense2 as rs; print(list(rs.context().query_devices()))' (if there are no attached devices but it otherwise worked correctly, it will just print an empty list []).

freezeboy · 2020-07-21T18:39:22Z

what do you mean it doesn't appear to python, it looks like it is ok on my side ..

I usually use :
nix-shell -p nixpkgs-review --run "nixpkgs-review pr 92997 -p 'python38.withPackages (ps: with ps; [ pyrealsense2])'"

tpwrules · 2020-07-21T19:02:45Z

We discussed on IRC and there was a misunderstanding on my end.

I'm happy with this PR and think it should be merged. I'll close mine out.

pkgs/development/libraries/librealsense/default.nix

…t and python bindings

jonringer

diff LGTM, but only did a quick glance:

https://github.com/NixOS/nixpkgs/pull/92997
8 packages built:
librealsense librealsenseWithCuda python27Packages.pyrealsense2 python27Packages.pyrealsense2WithCuda python37Packages.pyrealsense2 python37Packages.pyrealsense2WithCuda python38Packages.pyrealsense2 python38Packages.pyrealsense2WithCuda

jonringer · 2020-08-13T17:29:03Z

ping me tomorrow if no one merges

nixos-discourse · 2020-08-14T07:12:45Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/202

freezeboy · 2020-08-14T13:15:47Z

well ping @jonringer

jonringer

https://github.com/NixOS/nixpkgs/pull/92997
8 packages built:
librealsense librealsenseWithCuda python27Packages.pyrealsense2 python27Packages.pyrealsense2WithCuda python37Packages.pyrealsense2 python37Packages.pyrealsense2WithCuda python38Packages.pyrealsense2 python38Packages.pyrealsense2WithCuda

ofborg bot added the 8.has: package (new) label Jul 12, 2020

ofborg bot requested a review from brian-dawn July 12, 2020 12:10

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Jul 12, 2020

freezeboy requested review from FRidh and jonringer as code owners July 12, 2020 17:02

ofborg bot added 6.topic: python 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 and removed 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Jul 12, 2020

freezeboy force-pushed the realsense-cuda branch from ce414c0 to e9044f5 Compare July 12, 2020 20:56

brian-dawn approved these changes Jul 13, 2020

View reviewed changes

jonringer reviewed Jul 13, 2020

View reviewed changes

ofborg bot requested a review from brian-dawn July 14, 2020 08:20

brian-dawn approved these changes Jul 17, 2020

View reviewed changes

brian-dawn mentioned this pull request Jul 20, 2020

librealsense: add pyrealsense2 Python bindings + command line tools #93552

Closed

10 tasks

freezeboy force-pushed the realsense-cuda branch from c609ab0 to beb565d Compare July 21, 2020 11:55

ofborg bot requested a review from brian-dawn July 21, 2020 16:28

freezeboy force-pushed the realsense-cuda branch from 26a41a4 to 89c930e Compare July 21, 2020 19:42

brian-dawn reviewed Jul 22, 2020

View reviewed changes

pkgs/development/libraries/librealsense/default.nix Outdated Show resolved Hide resolved

brian-dawn approved these changes Jul 22, 2020

View reviewed changes

freezeboy force-pushed the realsense-cuda branch from 89c930e to e6b76fc Compare July 22, 2020 12:55

ofborg bot requested a review from brian-dawn July 22, 2020 13:09

brian-dawn approved these changes Jul 22, 2020

View reviewed changes

librealsense,python3Packages.pyrealsense2: refactor to add cudaSuppor…

7841a59

…t and python bindings

freezeboy force-pushed the realsense-cuda branch from e6b76fc to 7841a59 Compare August 13, 2020 07:35

ofborg bot requested a review from brian-dawn August 13, 2020 12:10

brian-dawn approved these changes Aug 13, 2020

View reviewed changes

jonringer approved these changes Aug 13, 2020

View reviewed changes

jonringer approved these changes Aug 14, 2020

View reviewed changes

jonringer merged commit ff8e182 into NixOS:master Aug 14, 2020

freezeboy deleted the realsense-cuda branch August 20, 2020 07:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

librealsense: refactor to allow use of cuda, and prepare for python integration #92997

librealsense: refactor to allow use of cuda, and prepare for python integration #92997

freezeboy commented Jul 12, 2020 •

edited

freezeboy commented Jul 12, 2020

brian-dawn left a comment

brian-dawn commented Jul 13, 2020 •

edited

freezeboy commented Jul 13, 2020

jonringer Jul 13, 2020

freezeboy Jul 14, 2020

jonringer Jul 13, 2020

tpwrules commented Jul 20, 2020

freezeboy commented Jul 21, 2020

freezeboy commented Jul 21, 2020

freezeboy commented Jul 21, 2020

tpwrules commented Jul 21, 2020

freezeboy commented Jul 21, 2020

tpwrules commented Jul 21, 2020

freezeboy commented Jul 21, 2020 •

edited

tpwrules commented Jul 21, 2020

jonringer left a comment

jonringer commented Aug 13, 2020

nixos-discourse commented Aug 14, 2020

freezeboy commented Aug 14, 2020

jonringer left a comment

	librealsenseWithoutCuda = callPackage ../development/libraries/librealsense {
	librealsense = callPackage ../development/libraries/librealsense {

	pyrealsense2WithoutCuda = toPythonModule (pkgs.librealsenseWithoutCuda.override {
	pyrealsense2 = toPythonModule (pkgs.librealsenseWithoutCuda.override {

librealsense: refactor to allow use of cuda, and prepare for python integration #92997

librealsense: refactor to allow use of cuda, and prepare for python integration #92997

Conversation

freezeboy commented Jul 12, 2020 • edited

Motivation for this change

Things done

freezeboy commented Jul 12, 2020

brian-dawn left a comment

Choose a reason for hiding this comment

brian-dawn commented Jul 13, 2020 • edited

freezeboy commented Jul 13, 2020

jonringer Jul 13, 2020

Choose a reason for hiding this comment

freezeboy Jul 14, 2020

Choose a reason for hiding this comment

jonringer Jul 13, 2020

Choose a reason for hiding this comment

tpwrules commented Jul 20, 2020

freezeboy commented Jul 21, 2020

freezeboy commented Jul 21, 2020

freezeboy commented Jul 21, 2020

tpwrules commented Jul 21, 2020

freezeboy commented Jul 21, 2020

tpwrules commented Jul 21, 2020

freezeboy commented Jul 21, 2020 • edited

tpwrules commented Jul 21, 2020

jonringer left a comment

Choose a reason for hiding this comment

jonringer commented Aug 13, 2020

nixos-discourse commented Aug 14, 2020

freezeboy commented Aug 14, 2020

jonringer left a comment

Choose a reason for hiding this comment

freezeboy commented Jul 12, 2020 •

edited

brian-dawn commented Jul 13, 2020 •

edited

freezeboy commented Jul 21, 2020 •

edited