Foxglove h.264 support

[pfavr2]

Thank you for a great ROS2 component!

This is a feature request: to support the foxglove.CompressedVideo message type in ffmpeg_image_transport

In this blog post Foxglove announces H.264 support in Foxglove:

https://foxglove.dev/blog/announcing-h264-support-in-foxglove

"...NAL units must be written to your log in a format that Foxglove will understand. For this, we’ve created the foxglove.CompressedVideo message schema – set the format field to “h264”, and the data field to contain the NAL units in Annex B format."

I think it would be nice to have this included in ffmpeg_image_transport when using h.264.

I could maybe help getting it done?

[berndpfrommer]

Do you see a path to achieving compatibility without me outright adopting Foxglove's message format? There are a couple of reasons I don't want to do that: 1) I already have data collected in the ffmpeg_image_transport_msgs format. LOTS of it. Same could be true for other people 2) The Foxglove message is missing:

• std_msgs/Header header this one is useful since all sensor_msgs have this header, and stuff like ROS time synchronizers expect that header.
• width and height of the frame: not much overhead but useful because some decoders may like to know the width / height before the first packet arrives.
• uint64 pts not sure how useful that is, forgot what I used it for
• uint8 flags indicates if the frame is a key frame. Useful so the decoding can be started on a key frame.

I believe Foxglove gets away with a simpler message format because they place restrictions on the streamed data that ffmpeg generally doesn't have. Like these: "For packet-based video codecs this data must begin and end on packet boundaries (no partial packets), and must contain enough video packets to decode exactly one image (either a keyframe or delta frame). Note: Foxglove Studio does not support video streams that include B frames because they require lookahead."

I would be loathe to forgo B frames. Seems easier for FoxGlove to support ffmpeg_image_transport_msgs. Not sure they want to do that either because if the customer selects an encoder that produces B frames, my plugin will work just fine, and later the customer will complain that they can't view the data they have collected.

[pfavr2]

Thank you for the good answer and I agree with you points 👍

If I decide to move forward with this, I'll try to make a patch with a parameter to select the message format (and keep the original as default).

You may close this issue and thanks!

[pfavr2]

One further note:

Foxglove also has an "experimental extension" with h.264 support using "sensor_msgs::msg::CompressedImage". I think Foxglove should implement a single "panel" with support for multiple message types instead of having multiple panels. Maybe this will happen in the future...

[tonynajjar]

Thank you for the good answer and I agree with you points 👍

If I decide to move forward with this, I'll try to make a patch with a parameter to select the message format (and keep the original as default).

You may close this issue and thanks!

Hi @pfavr2, did you end up moving forward with that? I'd be interested to have that patch as well.

Foxglove also has an "experimental extension" with h.264 support using "sensor_msgs::msg::CompressedImage

Is it this one you are referring to? https://github.com/codewithpassion/foxglove-studio-h264-extension Or is there one from Foxglove themselves?

[tonynajjar]

FYI https://github.com/angsa-robotics/ffmpeg_image_transport/pull/1/files. I did it hastily though, if you have any remarks please shoot

[berndpfrommer]

@tonynajjar great to have a working implementation! I noticed the absence of modifications to the decoder. Does that mean the foxglove framework never uses the image transport for decoding, but works off of rosbags and manually decodes from there? Obviously your implementation for now is just proof-of-concept because it switches hard from ffmpeg_image_transport_msgs to the compressed foxglove messages, without a flag to select what message types should be used. How about this suggestion: 1) We lift out the FFMPEG encoder/decoder classes into a separate package ("ffmpeg_image_transport_codec"). 2) We create a new transport plugin (in the same repository) ("foxglove_h264_image_transport") which has publisher/subscriber plugins that use the foxglove messages. This would avoid having ffmpeg_image_transport depending on foxglove messages and additionally allow testing of the foxglove_h264_image_transport with e.g. rqt_image_view

[tonynajjar]

I noticed the absence of modifications to the decoder. Does that mean the foxglove framework never uses the image transport for decoding, but works off of rosbags and manually decodes from there?

That's what I assumed, that the foxglove image panel implements its own decoding under the hood, unfortunately it's not open-source anymore to take a look.

Your suggestion sounds generally good, it would be nice to not have my fork for long. Unfortunately I need to move on to something else for now so I won't be able to implement it but how about we open back this ticket and hopefully somebody picks it up?

[berndpfrommer]

I do have the cycles to implement it. I contacted foxglove to clarify if they consider "foxglove_compressed_video_transport" a trademark violation. I do not use foxglove so I would count on you to test the plugin once it's done (I will test with rqt_image_view).

[defunctzombie]

Does that mean the foxglove framework never uses the image transport for decoding, but works off of rosbags and manually decodes from there?

Correct. For our app (which can run in desktop and web) we do not assume or require the user have a "ros workspace" installed. This is great for being able to view data anywhere - but also has the side-effect that encodings and messages need to be more strictly specified than the typical ROS approach of "go install this image transport and it handles whatever it made up". This is why it is important for us to be clear in our message definitions what the "format" is - because we can only support certain things rather than anything any local image transport can do.

I contacted foxglove to clarify if they consider "foxglove_compressed_video_transport" a trademark violation.

We'll get back to you on this and the above mentioned foxglove_h264_image_transport. At first glance I imagine it is fine if it is clear that it is not a "Foxglove" product but instead named this way to differentiate from other transports that it is for integration with foxglove things.

[shrijitsingh99]

@defunctzombie @berndpfrommer Any updates on this?

[berndpfrommer]

I was distracted by other projects. Will work a bit on it over the weekend.

[berndpfrommer]

The core encoder/decoder modules have been lifted out now to the ffmpeg_encoder_decoder repo

[berndpfrommer]

The foxglove image transport plugin is finished. Please check it out to see if it works for you.