Fix build for ROS2 Foxy

DomenicP commented 3 years ago

Building on #107 with some additional changes that I needed to get the package to successfully build under Foxy. Not sure on the proper procedure here since the previous PR was never merged, so just opening a second PR for now.

Notes

Credit to @nplan for the subscription API and topic discovery fixes from #107.
Switched image_transport.h to image_transport.hpp to fix a deprecation warning.
Moved ament_package() to the end of CMakeLists.txt. I believe this solved the issue with not being able to ros2 run after source dev_ws/install/setup.bash as mentioned in https://github.com/RobotWebTools/web_video_server/issues/105#issuecomment-667663265
The CV_IMWRITE_JPEG_QUALITY and CV_IMWRITE_PNG_COMPRESSION symbols from OpenCV were not found by the compiler. Switched to cv::IMWRITE_JPEG_QUALITY and cv::IMWRITE_PNG_COMPRESSION which should have the same values.
- See https://docs.opencv.org/4.2.0/d6/d87/imgcodecs_8hpp.html and https://docs.opencv.org/3.4/da/d0a/group__imgcodecs__c.html
Switched from rclcpp::executor::create_default_executor_arguments() to rclcpp::ExecutorOptions() to fix a deprecation warning.

One unresolved issue is that there are some deprecation warnings regarding libavcodec. I wasn't familiar enough with that library to attempt fixing them, so I just left them for now. Will post an example of the warnings in a comment below.

Library Versions

ros-foxy-ros-core/now 0.9.2-1focal.20201008.004629 amd64
libopencv-core4.2/now 4.2.0+dfsg-5 amd64
libavcodec58/now 7:4.2.4-1ubuntu0.1 amd64

DomenicP commented 3 years ago

Here's an example of the deprecation error for libavcodec:

--- stderr: web_video_server
/app/dep_ws/src/web_video_server/src/libav_streamer.cpp: In constructor ‘web_video_server::LibavStreamer::LibavStreamer(const async_web_server_cpp::HttpRequest&, async_web_server_cpp::HttpConnectionPtr, rclcpp::Node::SharedPtr, const string&, const string&, const string&)’:
/app/dep_ws/src/web_video_server/src/libav_streamer.cpp:65:55: warning: ‘int av_lockmgr_register(int (*)(void**, AVLockOp))’ is deprecated [-Wdeprecated-declarations]
   65 |   av_lockmgr_register(&ffmpeg_boost_mutex_lock_manager);
      |                                                       ^
In file included from /app/dep_ws/src/web_video_server/include/web_video_server/libav_streamer.h:11,
                 from /app/dep_ws/src/web_video_server/src/libav_streamer.cpp:1:
/usr/include/x86_64-linux-gnu/libavcodec/avcodec.h:6163:5: note: declared here
 6163 | int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op));
      |     ^~~~~~~~~~~~~~~~~~~
/app/dep_ws/src/web_video_server/src/libav_streamer.cpp:65:55: warning: ‘int av_lockmgr_register(int (*)(void**, AVLockOp))’ is deprecated [-Wdeprecated-declarations]
   65 |   av_lockmgr_register(&ffmpeg_boost_mutex_lock_manager);
      |                                                       ^
In file included from /app/dep_ws/src/web_video_server/include/web_video_server/libav_streamer.h:11,
                 from /app/dep_ws/src/web_video_server/src/libav_streamer.cpp:1:
/usr/include/x86_64-linux-gnu/libavcodec/avcodec.h:6163:5: note: declared here
 6163 | int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op));
      |     ^~~~~~~~~~~~~~~~~~~
/app/dep_ws/src/web_video_server/src/libav_streamer.cpp:66:19: warning: ‘void av_register_all()’ is deprecated [-Wdeprecated-declarations]
   66 |   av_register_all();
      |                   ^
In file included from /app/dep_ws/src/web_video_server/include/web_video_server/libav_streamer.h:12,
                 from /app/dep_ws/src/web_video_server/src/libav_streamer.cpp:1:
/usr/include/x86_64-linux-gnu/libavformat/avformat.h:2050:6: note: declared here
 2050 | void av_register_all(void);
      |      ^~~~~~~~~~~~~~~
/app/dep_ws/src/web_video_server/src/libav_streamer.cpp:66:19: warning: ‘void av_register_all()’ is deprecated [-Wdeprecated-declarations]
   66 |   av_register_all();
      |                   ^
In file included from /app/dep_ws/src/web_video_server/include/web_video_server/libav_streamer.h:12,
                 from /app/dep_ws/src/web_video_server/src/libav_streamer.cpp:1:
/usr/include/x86_64-linux-gnu/libavformat/avformat.h:2050:6: note: declared here
 2050 | void av_register_all(void);
      |      ^~~~~~~~~~~~~~~
/app/dep_ws/src/web_video_server/src/libav_streamer.cpp: In member function ‘virtual void web_video_server::LibavStreamer::initialize(const cv::Mat&)’:
/app/dep_ws/src/web_video_server/src/libav_streamer.cpp:160:35: warning: ‘AVStream::codec’ is deprecated [-Wdeprecated-declarations]
  160 |   codec_context_ = video_stream_->codec;
      |                                   ^~~~~
In file included from /app/dep_ws/src/web_video_server/include/web_video_server/libav_streamer.h:12,
                 from /app/dep_ws/src/web_video_server/src/libav_streamer.cpp:1:
/usr/include/x86_64-linux-gnu/libavformat/avformat.h:885:21: note: declared here
  885 |     AVCodecContext *codec;
      |                     ^~~~~
/app/dep_ws/src/web_video_server/src/libav_streamer.cpp:160:35: warning: ‘AVStream::codec’ is deprecated [-Wdeprecated-declarations]
  160 |   codec_context_ = video_stream_->codec;
      |                                   ^~~~~
In file included from /app/dep_ws/src/web_video_server/include/web_video_server/libav_streamer.h:12,
                 from /app/dep_ws/src/web_video_server/src/libav_streamer.cpp:1:
/usr/include/x86_64-linux-gnu/libavformat/avformat.h:885:21: note: declared here
  885 |     AVCodecContext *codec;
      |                     ^~~~~
/app/dep_ws/src/web_video_server/src/libav_streamer.cpp:160:35: warning: ‘AVStream::codec’ is deprecated [-Wdeprecated-declarations]
  160 |   codec_context_ = video_stream_->codec;
      |                                   ^~~~~
In file included from /app/dep_ws/src/web_video_server/include/web_video_server/libav_streamer.h:12,
                 from /app/dep_ws/src/web_video_server/src/libav_streamer.cpp:1:
/usr/include/x86_64-linux-gnu/libavformat/avformat.h:885:21: note: declared here
  885 |     AVCodecContext *codec;
      |                     ^~~~~
---
Finished <<< web_video_server [24.6s]

DomenicP commented 3 years ago

Pushed an additional commit to fix a bug that was causing the following error message in the console output:

[web_video_server]: exception: can't subtract times with different time sources [1 != 2]

This exception was being thrown because the default clock type for rclcpp::Node::now() is RCL_ROS_TIME, but the default clock type when using the standard rclcpp::Time::Time(int32_t, uint32_t) constructor is RCL_SYSTEM_TIME. When the incoming CompressedImage message was being parsed, the timestamp was being parsed with RCL_SYSTEM_TIME, which caused an exception when attempting to do math with timestamps generated by rclcpp::Node::now().

There is an additional rclcpp::Time::Time(const builtin_interfaces::msg::Time &) constructor which defaults to RCL_ROS_TIME. I switched the code to use this constructor instead to eliminate the exception. For my use case at least, this is consistent since i use rclcpp::Node::now() with RCL_ROS_TIME in my camera node to generate the image message timestamps in the first place.

pianosnake commented 3 years ago

Thank you for doing this PR! I'm in no position to approve it, but I am using it for my project. I am able to see an image generated with cam2image, like so: ros2 run image_tools cam2image --ros-args -p burger_mode:=True (This requires the ROS package image_tools.) This image shows up fine at http://localhost:8080/stream_viewer?topic=/image

But I'm not able to see this image when the QoS (quality of service) parameter is "Best effort", which is the type of video coming out of Gazebo when running foxy: ros2 run image_tools cam2image --ros-args -p burger_mode:=True -p reliability:=best_effort

Do you know if there is a way for web_video_server to support a "best effort" QoS parameter?

DomenicP commented 3 years ago

@pianosnake That's a good question. I recently ran into some problems with ROS2 QoS myself. According to the first table under https://index.ros.org/doc/ros2/Concepts/About-Quality-of-Service-Settings/#qos-compatibilities, a pub/sub connection will fail if the publisher uses the Best Effort QoS policy but the subscriber uses the Reliable QoS policy.

Here's an example from the RosCompressedStreamer where the subscription to the regular ROS image topic is made:

https://github.com/DomenicP/web_video_server/blob/4dc8df5fdc918adc7c710bbc881e97d404d8854f/src/ros_compressed_streamer.cpp#L21-L22

Instead of specifying a full QoS profile, it looks like just a queue depth of 1 is specified. This means that the rmw default QoS values will be used for the other policies, which likely results in a Reliable QoS policy for the subscriber which causes the connection to the Best Effort publisher fail.

To fix this, you would have to change the QoS policy to Best Effort for the subscriber in the streamer class that you are using. Perhaps this could be a node parameter that each of the streamer classes could check when initializing their subscribers?

emersonknapp commented 3 years ago

@rctoris @mitchellwills as maintainers - do you have any input on this PR? It seems that this package does not build on Foxy without it. If you're having trouble evaluating whether a PR works - I might suggest using the https://github.com/ros-tooling/action-ros-ci GitHub Action to build against ROS (1 or 2) - which would automatically evaluate a PR like this.

jihoonl commented 3 years ago

@dirk-thomas you might want to comment?

dirk-thomas commented 3 years ago

@jihoonl sorry, I am not involved in the project anymore.

jihoonl commented 3 years ago

Um, there is no active maintainer. I merge this to move forward. If anyone wants to become maintainer for ros2 version, please let me know.

emersonknapp commented 3 years ago

@jihoonl is there some sort of "RobotWebTools working group" that pays attention to projects in this organization? Or, is the governance a little more ad-hoc than that?

jihoonl commented 3 years ago

It has become more like ad-hoc these days. As many maintainers have contributed some times and moved to other positions or other works.

Even myself, I have been just believing the contributors' PRs without reviewing these days as I don't have any resources(time as well as a machine for testing) to review the new changes.

So, I will be happy to grant a maintainership if trustable someone stands up because I believe moving in any direction is better than no change. Would you help us? 😉

emersonknapp commented 3 years ago

I might have some bandwidth for helping out with general Working Group style repository management - I think that most of the tools we've built at https://github.com/ros-tooling could help with these projects, and I have found the meeting format useful. For some contributions, I would be able to evaluate general code/contribution quality, though I haven't actively developed any RobotWebTools projects to date - so it would take some time to be familiar with the code functionality.

With the above clarifications - yes, I'd be willing to offer some help in maintenance here.

To start a conversation about it - I created a Discourse topic here https://discourse.ros.org/t/call-for-interest-robotwebtools-working-group/20451 - maybe that will start some useful discussion.

jihoonl commented 3 years ago

Thank you for the great initiation. I hope that the active working group gets formed again by this initiation to revitalize the project to move forward. I am cc-ing @J-Rojas @MatthijsBurgh @Rayman @mvollrath @minggangw @roehling @jspricke who have maintained and contributed RWT in recent.

Hello everyone, I am cc-ing you because you may have opinions or ideas for this working group initiation. 😃

RobotWebTools / web_video_server

Fix build for ROS2 Foxy #111

Notes

Library Versions