BrettRD / ros-gst-bridge

a bidirectional ros to gstreamer bridge and utilities for dynamic pipelines
Other
129 stars 30 forks source link

Provide parent classes to handle repeated ROS details #14

Closed BrettRD closed 3 years ago

BrettRD commented 3 years ago

This change allows new sources and sinks to be created with less boilerplate, and should allow other users to create new packages to with new element collections with specific dependencies.

This also collects some particularly sinister issues into two files. In particular, https://github.com/BrettRD/ros-gst-bridge/issues/4 needs a thread to call spin() from. This can go in rosbasesrc and rosbasesink and fix all derived elements in a later patch.

clydemcqueen commented 3 years ago

I ran a few smoke tests.

I was able to publish images: gst-launch-1.0 --gst-plugin-path=install/gst_bridge/lib/gst_bridge -v videotestsrc ! videoconvert ! rosimagesink

I was able to subscribe to the resulting ROS images and display them: ros2 run image_tools showimage --ros-args -p reliability:=reliable -r image:=/gst_image_pub

I was able to list, get and set parameters on /gst_image_sink_node while it was running. (Though the only parameter is use_sim_time.)

This worked, though sometimes there was a long delay in getting the 1st message: gst-launch-1.0 --gst-plugin-path=install/gst_bridge/lib/gst_bridge -v rosimagesrc ros-topic=/gst_image_pub ! queue ! videoconvert ! xvimagesink

When I hit ^C on the rosimagesrc pipeline it crashed:

Setting pipeline to PAUSED ...
Pipeline is live and does not need PREROLL ...
[INFO] [1615994861.512551377] [gst_image_src_node]: getcaps with filter 'NULL'
[INFO] [1615994861.512593365] [gst_image_src_node]: waiting for first message
Setting pipeline to PLAYING ...
New clock: GstSystemClock
[INFO] [1615994863.538389744] [gst_image_src_node]: preparing video with caps 'video/x-raw, format=(string)RGBA, height=(int)240, width=(int)320, framerate=(fraction)0/1'
/GstPipeline:pipeline0/Rosimagesrc:rosimagesrc0.GstPad:src: caps = video/x-raw, format=(string)RGBA, height=(int)240, width=(int)320, framerate=(fraction)0/1
/GstPipeline:pipeline0/GstQueue:queue0.GstPad:sink: caps = video/x-raw, format=(string)RGBA, height=(int)240, width=(int)320, framerate=(fraction)0/1
/GstPipeline:pipeline0/GstQueue:queue0.GstPad:src: caps = video/x-raw, format=(string)RGBA, height=(int)240, width=(int)320, framerate=(fraction)0/1
/GstPipeline:pipeline0/GstVideoConvert:videoconvert0.GstPad:src: caps = video/x-raw, height=(int)240, width=(int)320, framerate=(fraction)0/1, format=(string)YUY2
/GstPipeline:pipeline0/GstXvImageSink:xvimagesink0.GstPad:sink: caps = video/x-raw, height=(int)240, width=(int)320, framerate=(fraction)0/1, format=(string)YUY2
/GstPipeline:pipeline0/GstVideoConvert:videoconvert0.GstPad:sink: caps = video/x-raw, format=(string)RGBA, height=(int)240, width=(int)320, framerate=(fraction)0/1
^Chandling interrupt.
Interrupt: Stopping pipeline ...
Execution ended after 0:00:27.761869556
Setting pipeline to PAUSED ...
Setting pipeline to READY ...

(gst-launch-1.0:968279): GStreamer-CRITICAL **: 08:28:09.303: gst_buffer_map_range: assertion 'GST_IS_BUFFER (buffer)' failed
Caught SIGSEGV
Spinning.  Please run 'gdb gst-launch-1.0 968279' to continue debugging, Ctrl-C to quit, or Ctrl-\ to dump core.
^C

I skimmed the code, but I have no experience with the gst plugin patterns, so I'm not much help there. But the ROS pieces look reasonable to me.

I hope this helps.

BrettRD commented 3 years ago

Thanks so much for that! I saw similar but I couldn't get gdb onto it

I traced that that segfault to memcpy in rosimagesrc_create, it needs more checks to validate the buffers are correctly sized and the message pointer isn't null, especially when gstreamer is flushing the pipeline.

I'm satisfied that the upstream branch contains this bug, and it only affects pipeline close.