daniilidis-group / ffmpeg_image_transport

image transport that uses libavcodec for compression
Apache License 2.0
42 stars 13 forks source link

use boost::placeholders::_1/_2 instead of deprecated global namespaced _1/_2 #8

Closed lucasw closed 2 years ago

berndpfrommer commented 2 years ago

Looks good but could you comment on what motivated the change? Thanks!

lucasw commented 2 years ago

Compiling this on Ubuntu 22.04 results in this error:

/home/lucasw/catkin_ws/src/ros/ffmpeg_image_transport/src/ffmpeg_subscriber.cpp: In member function ‘virtual void ffmpeg_image_transport::FFMPEGSubscriber::internalCallback(const ConstPtr&, const Callback&)’:
/home/lucasw/catkin_ws/src/ros/ffmpeg_image_transport/src/ffmpeg_subscriber.cpp:39:69: error: ‘::_1’ has not been declared
   39 |             msg, boost::bind(&FFMPEGSubscriber::frameReady, this, ::_1, ::_2),

It's possible to include boost/bind.hpp which will bring them back but

/usr/include/boost/bind.hpp:36:1: note: ‘#pragma message: The practice of declaring the Bind placeholders (_1, _2, ...) in the global namespace is deprecated. Please use <boost/bind/bind.hpp> + using namespace boost::placeholders, or define BOOST_BIND_GLOBAL_PLACEHOLDERS to retain the current behavior.’

Probably boost/bind.hpp was getting included elsewhere (something ffmpeg_image_transport was dependent on) in previous versions but that has been removed to avoid the deprecation warning.

Another change is needed to build it in 22.04, that is to use C++17 021ce0f321d273663f32aa799eb4a8a8c4b51605 - I can put that commit here too if you want it, which will be fine for noetic, probably melodic too but not as sure about that.

berndpfrommer commented 2 years ago

yes, please add the C++17 flag as well such that things compile and work on Ubuntu 22.04. You are testing on noetic on 22.04? Once this PR has been merged I will test on Ubuntu 18.04 + melodic. I still have a machine with that setup.

lucasw commented 2 years ago

You are testing on noetic on 22.04?

Mostly I'm still using 20.04, 22.04 is experimental but so far so good- I'm using core ros packages from Debian through apt, some sources from http://github.com/ros-o, and many of my own forks, though many of the changes are getting upstreamed into official repos in the noetic branches.

berndpfrommer commented 2 years ago

Tested on Ubuntu 18.04 + melodic