Skip to content
This repository has been archived by the owner on Aug 1, 2019. It is now read-only.

Centralize vision_opencv repository #13

Closed
testkit opened this issue May 31, 2018 · 7 comments
Closed

Centralize vision_opencv repository #13

testkit opened this issue May 31, 2018 · 7 comments

Comments

@testkit
Copy link

testkit commented May 31, 2018

When do ROS2 cv_bridge porting, the question about where to host it has been raised and discussed at #12, the final decision is to manage the code in https://github.com/ros-perception/vision_opencv ros2 branch.
Sorry to bring this question to the table again, since we run into a conflict issue when try to add cv_bridge to coming bouncy release. In vision_opencv, there are two key packages, cv_bridge and image_geometry, but they are maintained in ros-perception/vision_opencv and ros2/vision_opencv respectively, which brings difficulties to add these two packages into bloom release system.
For coming bouncy release, we'll copy ROS2 cv_bridge implementation to this project, but it's not an idea way to maintain the code in both repositories moving forward.

There are two options to get it resolved, I list them as below with the order of my preference, pls. comment.

  • manage cv_bridge and image_geometry in this project, bouncy release from this project as well
  • manage cv_bridge and image_geometry in ros-perception/vision_opencv ros2 branch, bouncy release from that project

@dirk-thomas @clalancette @gaoethan @mikaelarguedas @vrabaud.

@mikaelarguedas
Copy link
Member

Thanks @testkit for bringing it up!

We discussed it internally a few weeks back and concluded that it should be centralized on https://github.com/ros-perception/vision_opencv as the ROS 2 fork was only temporary.
Ideally we would also keep the goal of "keeping the diff with the ROS1 version as small as possible", as this will allow for a lower maintenance burden and make patches trivially portable between the ROS1 and ROS2 versions.
This can be done in multiple ways:

  • submit all changes not specific to ROS 2 (and backward compatible) to the ROS 1 branch
  • avoid unnecessary changes if the ROS 1 maintainer is not willing to take them upstream (style changes for example).

I started the work of rebasing our changes of image_geometry on top of ros-perception/vision_opencv ros2 branch a few weeks back. I have not been able to use them on our CI system yet as:

  • cv_bridge has compilation warnings
  • the conversions.py test from cv_bridge fails on my machine

I opened ros-perception#219 with the image_geometry changes we currently use in ROS2.

Once that is merged, we can modify the ROS 2 CI to use the ros2 branch from https://github.com/ros-perception/vision_opencv and just skip the packages that we currently dont use and that are failings tests / having compiler warnings

@testkit
Copy link
Author

testkit commented Jun 1, 2018

@mikaelarguedas, that's great to have this issue considered and the solution is on the way already!
@gaoethan will help to check the two issues you mentioned above for cv_bridge.

@gaoethan
Copy link

gaoethan commented Jun 1, 2018

@mikaelarguedas thanks for your quick support, regarding the test of conversions.py you mentioned

the conversions.py test from cv_bridge fails on my machine

Generally, it contains 4 test cases in it and it takes relatively long time than the other to finish, because it's normal with my trial, could u please give your error log for more details ? thanks !

@mikaelarguedas
Copy link
Member

@gaoethan below the console output of the test.
Tested on Ubuntu bionic after installing manually python3-numpy and python3-opencv

10: Test command: /usr/bin/python3 "-u" "/root/ros2_ws/install/ament_cmake_test/share/ament_cmake_test/cmake/run_test.py" "/root/ros2_overlay_ws/build/cv_bridge/test_results/cv_bridge/conversions.py.xunit.xml" "--package-name" "cv_bridge" "--output-file" "/root/ros2_overlay_ws/build/cv_bridge/ament_cmake_pytest/conversions.py.txt" "--command" "/usr/bin/python3" "-u" "-m" "pytest" "/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/test/conversions.py" "-o" "cache_dir=/root/ros2_overlay_ws/build/cv_bridge/test/ament_cmake_pytest/conversions.py/.cache" "--junit-xml=/root/ros2_overlay_ws/build/cv_bridge/test_results/cv_bridge/conversions.py.xunit.xml" "--junit-prefix=cv_bridge"
10: Test timeout computed to be: 600
10: -- run_test.py: invoking following command in '/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge':
10:  - /usr/bin/python3 -u -m pytest /root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/test/conversions.py -o cache_dir=/root/ros2_overlay_ws/build/cv_bridge/test/ament_cmake_pytest/conversions.py/.cache --junit-xml=/root/ros2_overlay_ws/build/cv_bridge/test_results/cv_bridge/conversions.py.xunit.xml --junit-prefix=cv_bridge
10: ============================= test session starts ==============================
10: platform linux -- Python 3.6.5, pytest-3.6.0, py-1.5.3, pluggy-0.6.0
10: rootdir: /root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge, inifile:
10: plugins: rerunfailures-4.1, repeat-0.4.1, cov-2.5.1
10: collected 4 items
10: 
10: test/conversions.py .F..                                                 [100%]
10: 
10: =================================== FAILURES ===================================
10: ______________ TestConversions.test_encode_decode_cv2_compressed _______________
10: 
10: self = <conversions.TestConversions testMethod=test_encode_decode_cv2_compressed>
10: 
10:     def test_encode_decode_cv2_compressed(self):
10:         import numpy as np
10:         # from:
10:         # http://docs.opencv.org/2.4/modules/highgui/doc/reading_and_writing_images_and_video.html#Mat
10:         # imread(const string& filename, int flags)
10:         formats = ['jpg', 'jpeg', 'jpe', 'png', 'bmp', 'dib', 'ppm', 'pgm', 'pbm',
10:                    'jp2', 'sr', 'ras', 'tif', 'tiff']  # this formats rviz is not support
10:     
10:         cvb_en = CvBridge()
10:         cvb_de = CvBridge()
10:     
10:         for w in range(100, 800, 100):
10:             for h in range(100, 800, 100):
10:                 for f in formats:
10:                     for channels in ([], 1, 3):
10:                         if channels == []:
10:                             original = np.uint8(np.random.randint(0, 255, size=(h, w)))
10:                         else:
10:                             original = np.uint8(np.random.randint(0, 255, size=(h, w, channels)))
10: >                       compress_rosmsg = cvb_en.cv2_to_compressed_imgmsg(original, f)
10: 
10: test/conversions.py:78: 
10: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
10: 
10: self = <cv_bridge.core.CvBridge object at 0x7f9e8e836d68>
10: cvim = array([[215, 148, 142, ..., 116, 162, 242],
10:        [242,  60, 244, ...,  84, 114, 155],
10:        [ 50, 234,  48, ..., 16..., 147, 249, 159],
10:        [186, 233, 250, ..., 170, 116, 158],
10:        [157,  38,  84, ..., 184, 144, 137]], dtype=uint8)
10: dst_format = 'jp2'
10: 
10:     def cv2_to_compressed_imgmsg(self, cvim, dst_format='jpg'):
10:         """
10:             Convert an OpenCV :cpp:type:`cv::Mat` type to a ROS sensor_msgs::CompressedImage message.
10:     
10:             :param cvim:      An OpenCV :cpp:type:`cv::Mat`
10:             :param dst_format:  The format of the image data, one of the following strings:
10:     
10:             http://docs.opencv.org/2.4/modules/highgui/doc/reading_and_writing_images_and_video.html
10:             http://docs.opencv.org/2.4/modules/highgui/doc/reading_and_writing_images_and_video.html#Mat
10:             * imread(const string& filename, int flags)
10:                * bmp, dib
10:                * jpeg, jpg, jpe
10:                * jp2
10:                * png
10:                * pbm, pgm, ppm
10:                * sr, ras
10:                * tiff, tif
10:     
10:             :rtype:           A sensor_msgs.msg.CompressedImage message
10:             :raises CvBridgeError: when the ``cvim`` has a type that is incompatible with ``format``
10:     
10:     
10:             This function returns a sensor_msgs::Image message on success,
10:             or raises :exc:`cv_bridge.CvBridgeError` on failure.
10:             """
10:         import cv2
10:         import numpy as np
10:         if not isinstance(cvim, (np.ndarray, np.generic)):
10:             raise TypeError('Your input type is not a numpy array')
10:         cmprs_img_msg = sensor_msgs.msg.CompressedImage()
10:         cmprs_img_msg.format = dst_format
10:         ext_format = '.' + dst_format
10:         try:
10: >           cmprs_img_msg.data = np.array(cv2.imencode(ext_format, cvim)[1]).tostring()
10: E           cv2.error: /build/opencv-zcaJjh/opencv-3.2.0+dfsg/modules/imgcodecs/src/loadsave.cpp:682: error: (-2) could not find encoder for the specified extension in function imencode
10: 
10: ../../../../install/cv_bridge/lib/python3.6/site-packages/cv_bridge/core.py:240: error
10: ----------------------------- Captured stderr call -----------------------------
10: OpenCV Error: Unspecified error (could not find encoder for the specified extension) in imencode, file /build/opencv-zcaJjh/opencv-3.2.0+dfsg/modules/imgcodecs/src/loadsave.cpp, line 682
10:  generated xml file: /root/ros2_overlay_ws/build/cv_bridge/test_results/cv_bridge/conversions.py.xunit.xml 
10: ===================== 1 failed, 3 passed in 147.82 seconds =====================
10: -- run_test.py: return code 1
10: -- run_test.py: verify result file '/root/ros2_overlay_ws/build/cv_bridge/test_results/cv_bridge/conversions.py.xunit.xml'
10/11 Test #10: conversions.py ...................***Failed  148.77 sec

Build warnings:

CMake Warning at /usr/share/cmake-3.10/Modules/FindBoost.cmake:1626 (message):
  No header defined for python3; skipping header check
Call Stack (most recent call first):
  CMakeLists.txt:20 (find_package)


/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/cv_bridge.cpp: In function ‘cv_bridge::CvImageConstPtr cv_bridge::cvtColorForDisplay(const CvImageConstPtr&, const string&, cv_bridge::CvtColorForDisplayOptions)’:
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/cv_bridge.cpp:631:26: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     for (size_t j = 0; j < source->image.rows; ++j) {
                        ~~^~~~~~~~~~~~~~~~~~~~
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/cv_bridge.cpp:632:28: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
       for (size_t i = 0; i < source->image.cols; ++i) {
                          ~~^~~~~~~~~~~~~~~~~~~~
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/cv_bridge.cpp:684:30: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
         for (size_t j = 0; j < source->image.rows; ++j) {
                            ~~^~~~~~~~~~~~~~~~~~~~
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/cv_bridge.cpp:685:32: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
           for (size_t i = 0; i < source->image.cols; ++i) {
                              ~~^~~~~~~~~~~~~~~~~~~~
In file included from /root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/module.cpp:50:0:
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/module.hpp: In function ‘int do_numpy_import()’:
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/module.hpp:39:3: warning: converting to non-pointer type ‘int’ from NULL [-Wconversion-null]
   import_array();
   ^~~~~~~~~~~~
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/module.hpp:40:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
In file included from /root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/module_opencv3.cpp:18:0:
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/module.hpp: In function ‘int do_numpy_import()’:
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/module.hpp:39:3: warning: converting to non-pointer type ‘int’ from NULL [-Wconversion-null]
   import_array();
   ^~~~~~~~~~~~
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/module.hpp:40:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/module_opencv3.cpp: At global scope:
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/src/module_opencv3.cpp:97:19: warning: ‘PyObject* failmsgp(const char*, ...)’ defined but not used [-Wunused-function]
 static PyObject * failmsgp(const char * fmt, ...)
                   ^~~~~~~~
In file included from /root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/test/utest.cpp:16:0:
/root/ros2_ws/install/gtest_vendor/src/gtest_vendor/include/gtest/gtest.h: In instantiation of ‘testing::AssertionResult testing::internal::CmpHelperEQ(const char*, const char*, const T1&, const T2&) [with T1 = unsigned int; T2 = int]’:
/root/ros2_ws/install/gtest_vendor/src/gtest_vendor/include/gtest/gtest.h:1429:23:   required from ‘static testing::AssertionResult testing::internal::EqHelper<lhs_is_null_literal>::Compare(const char*, const char*, const T1&, const T2&) [with T1 = unsigned int; T2 = int; bool lhs_is_null_literal = false]’
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/test/utest.cpp:32:3:   required from here
/root/ros2_ws/install/gtest_vendor/src/gtest_vendor/include/gtest/gtest.h:1401:11: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   if (lhs == rhs) {
       ~~~~^~~~~~
/root/ros2_ws/install/gtest_vendor/src/gtest_vendor/include/gtest/gtest.h: In instantiation of ‘testing::AssertionResult testing::internal::CmpHelperEQ(const char*, const char*, const T1&, const T2&) [with T1 = int; T2 = long unsigned int]’:
/root/ros2_ws/install/gtest_vendor/src/gtest_vendor/include/gtest/gtest.h:1429:23:   required from ‘static testing::AssertionResult testing::internal::EqHelper<lhs_is_null_literal>::Compare(const char*, const char*, const T1&, const T2&) [with T1 = int; T2 = long unsigned int; bool lhs_is_null_literal = false]’
/root/ros2_overlay_ws/src/ros2/vision_opencv/cv_bridge/test/utest.cpp:106:3:   required from here
/root/ros2_ws/install/gtest_vendor/src/gtest_vendor/include/gtest/gtest.h:1401:11: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

@mikaelarguedas
Copy link
Member

PRs to migrate ROS2 CI to use the https://github.com/ros-perception/vision_opencv repository instead of the ros2 fork:
ros2/ci#180
ros2/turtlebot2_demo#86

@testkit @gaoethan please speak up if there is any reason to hold this, otherwise I'll go ahead and merge these tomorrow

@gaoethan
Copy link

gaoethan commented Jun 5, 2018

@mikaelarguedas thanks for your detail. I create an extra issue ros-perception#221 to track and fix the warning and test failure for you to avoid messy topic here, next I'll investigate and fix it there accordingly.

@mikaelarguedas
Copy link
Member

@testkit @gaoethan I'm closing this ticket as I believe the original question has been answered.
If there is any item we forgot to address, please feel free to comment here and we can reopen it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants