reapply yuv encoding deprecation #257
Conversation
Signed-off-by: Christian Rauch <[email protected]>
Signed-off-by: Christian Rauch <[email protected]>
Signed-off-by: Christian Rauch <[email protected]>
Signed-off-by: Christian Rauch <[email protected]>
|
|
I'm going to be perfectly frank; I'm not sure that this is worth putting in. The simple fact is that these encodings are not standardized anywhere. And I am fine having two aliases for the same thing. Finally, this PR adds in a lot of That said, this is just my opinion, and I'm not going to fight hard for it. If others think that this is much clearer, then I'm not going to put up a fuss about it. |
|
@ahcorde With ros2/rviz#1276 now merged, can you run the CI again? |
| #else | ||
| #pragma GCC diagnostic push | ||
| #pragma GCC diagnostic ignored "-Wdeprecated-declarations" | ||
| #endif |
There was a problem hiding this comment.
For the sake of maintenance, can we scope down where the ifdef/pragmas are required?
I think we could use a function like:
static inline bool isPackedYUV(const std::string& encoding)
That contains the deprecation suppression, and then use that in the other functions?
There was a problem hiding this comment.
Do you also want to use isPackedYUV as a public function? Eventually, the deprecated encodings will be removed. If this is a functionality that is supposed to be exposed to a user and used often, then I can add a isPackedYUV. But doing this just for the more narrow scoping of the encodings is a maintenance overhead IMHO.
|
@ahcorde Can you run the CI again? |
|
Pulls: #257
|
This reapplies #247 with fixes for MSVC.
With ros2/rviz#1276 there are no references to the deprecated encodings
yuv422oryuv422_yuy2left in the core packages (https://github.com/ros2/ros2/blob/rolling/ros2.repos).Requires: