Skip to content

Cuda + OpenGL on ARM#15658

Merged
alalek merged 2 commits intoopencv:3.4from
tolysz:patch-1
Oct 9, 2019
Merged

Cuda + OpenGL on ARM#15658
alalek merged 2 commits intoopencv:3.4from
tolysz:patch-1

Conversation

@tolysz
Copy link
Copy Markdown
Contributor

@tolysz tolysz commented Oct 7, 2019

This pull request changes

There might be multiple ways of getting OpenCV compile on Tegra (NVIDIA Jetson) platform, but mainly they modify CUDA(8,9,10...) source code, this one fixes it for all installations.
( https://devtalk.nvidia.com/default/topic/1007290/jetson-tx2/building-opencv-with-opengl-support-/post/5141945/#5141945 et al.).
This way is exactly the same as the one proposed but the code change happens in OpenCV.

opencv_contrib=

There might be multiple ways of getting OpenCV compile on Tegra (NVIDIA Jetson) platform, but mainly they modify CUDA(8,9,10...) source code, this one fixes it for all installations. 
( https://devtalk.nvidia.com/default/topic/1007290/jetson-tx2/building-opencv-with-opengl-support-/post/5141945/#5141945 et al.).
This way is exactly the same as the one proposed but the code change happens in OpenCV.
@alalek
Copy link
Copy Markdown
Member

alalek commented Oct 8, 2019

patch-1

Please use descriptive names next time.
This name conflicts with patch into opencv_contrib under the same "patch-1" but for different target branch (this is a CI feature for testing patches into multiple repositories at once).

# include "gl_core_3_1.hpp"
# ifdef HAVE_CUDA
# if defined(__arm__) || defined(__aarch64__)
# warning Workaround for CUDA on ARM
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To make robust solution please add option to bypass this workaround:

if (defined(__arm__) || defined(__aarch64__)) \
    && !defined(OPENCV_SKIP_CUDA_OPENGL_ARM_WORKAROUND)

which can be applied via C++ CMake flags.

Copy link
Copy Markdown
Contributor Author

@tolysz tolysz Oct 8, 2019

Choose a reason for hiding this comment

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

Should OPENCV_SKIP_CUDA_OPENGL_ARM_WORKAROUND workaround be enabled or disabled by default? (I will start with enabled).
The current change is derived from the patch for CUDAs to make OpenCV compile with it, and covers exactly the cases which CUDA currently rejects:

index 0f4aa17..e8c538c 100644
--- a/cuda_gl_interop.h
+++ b/cuda_gl_interop.h
@@ -59,13 +59,13 @@
 
 #else /* __APPLE__ */
 
-#if defined(__arm__) || defined(__aarch64__)
-#ifndef GL_VERSION
-#error Please include the appropriate gl headers before including cuda_gl_interop.h
-#endif
-#else
+//#if defined(__arm__) || defined(__aarch64__)
+//#ifndef GL_VERSION
+//#error Please include the appropriate gl headers before including cuda_gl_interop.h
+//#endif
+//#else
 #include <GL/gl.h>
-#endif
+//#endif
 
 #endif /* __APPLE__ */

re: patch-1 it is an artifact from github editor, will try to rename them before making PRs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for update!

I don't see information about fixed CUDA versions in linked forum thread, so this workaround should be enabled by default (but probably we should eliminate #warning from here or disable that message under another "*_MESSAGE" definition flag).

Please change "default" value of CL_VERSION to 0x1F02. I see this on my machine (Fedora):

/usr/include/GL/gl.h:#define GL_VERSION				0x1F02
/usr/include/GL/glcorearb.h:#define GL_VERSION                        0x1F02

I believe CMake changes are not necessary (option is very specific, lets keep it hidden).
Flag can be added via CMake for future fixed versions (if necessary - workaround is harmless without warning message and proper default value):

cmake ... -DOPENCV_EXTRA_FLAGS=-DOPENCV_SKIP_CUDA_OPENGL_ARM_WORKAROUND=1

@tolysz
Copy link
Copy Markdown
Contributor Author

tolysz commented Oct 8, 2019

done, but I am not sure if I extended cmake in the right way... I tested it both ways.

The link provided mentions: cuda8 + 9, I have cuda 10 + 10.1 (and can confirm it is still defined this way).
NVIDIA is probably using some other "secret" backend with Jetson.
@asmorkalov
Copy link
Copy Markdown
Contributor

The issue is still there with latest Jetpack and CUDA 10 I have on my Jetson Nano. @alalek Let's go ahead and merge the workaround. 👍

@alalek alalek merged commit 3fd36c1 into opencv:3.4 Oct 9, 2019
@tolysz tolysz deleted the patch-1 branch October 9, 2019 09:29
@alalek alalek mentioned this pull request Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants