Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting caps on rosimagesrc from ROS Image msg #47

Open
sandman opened this issue Jun 16, 2022 · 6 comments
Open

Setting caps on rosimagesrc from ROS Image msg #47

sandman opened this issue Jun 16, 2022 · 6 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@sandman
Copy link

sandman commented Jun 16, 2022

HI @BrettRD,

I noticed that the rosimagesrc element caps are not reading correctly the height, width etc. from the ROS Image.msg and need to be be set explicitly during pipeline initialization.

Excerpt of the logs that show the issue:

..
[sndstream_exe-1] [INFO] [1655377029.211000007] [vehicle.gst_image_src_node_front]: preparing video with caps 'video/x-raw, framerate=(fraction)20/1, **width=(int)1, height=(int)1, format=(string)I420'**
..
[sndstream_exe-1] ===== NVMEDIA: NVENC ===== 
[sndstream_exe-1] [ERROR] [1655377029.315226453] [vehicle.gst_image_src_node_front]: image format changed during playback, step **0 != 3**
[sndstream_exe-1] [ERROR] [1655377029.315432031] [vehicle.gst_image_src_node_front]: image format changed during playback, height **0 != 1200**
[sndstream_exe-1] [ERROR] [1655377029.315552869] [vehicle.gst_image_src_node_front]: image format changed during playback, width **0 != 1920**
[sndstream_exe-1] [ERROR] [1655377029.315624041] [vehicle.gst_image_src_node_front]: image format changed during playback, encoding **unknown != rgb8**

I noticed that the function rosimagesrc_fixate does not actually pick the caps from ROS.

I tried running the ROS image publisher before starting the node containing rosimagesrc just to check if the latter takes the video parameters from ROS and found that it does not.

Have you tried setting the caps from the ROS message?

@sandman
Copy link
Author

sandman commented Jun 16, 2022

I noticed the error now..my GStreamer pipeline is initialized and playing BEFORE the caps are set on the rosimagesrc element (i.e. the rosimagesrc_fixate function call). This is the reason for the error.

However, the broader question of supporting video resolutions changing on-the-fly still applies I think.

@BrettRD
Copy link
Owner

BrettRD commented Jun 17, 2022

Thanks for the interest, this is a known limitation.
Default behaviour should keep the pipeline paused until the first incoming message.
I have a few pipelines where messages dip in and out of ROS a few times, they don't start at all unless you break the deadlock by passing a caps string at launch.

The errors you're seeing are printed where a gstreamer re-negotiate hook should be.
Changing caps involves inserting a block in the pipeline, and starting caps negotiation again.
I'm pretty sure _fixate is the wrong way to get the caps settled in the first place, but I don't know what the right way is.

If you can track down an example of a change-of-format hook, I'd love to get this feature implemented.

@BrettRD BrettRD added enhancement New feature or request help wanted Extra attention is needed labels Jun 17, 2022
@sandman
Copy link
Author

sandman commented Jun 17, 2022

I think GStreamer dynamic pipelines are what we need. A pad added callback which updates the caps based on incoming buffers. https://gstreamer.freedesktop.org/documentation/tutorials/basic/dynamic-pipelines.html?gi-language=c

I'll try to make a minimal example and share here.

@BrettRD
Copy link
Owner

BrettRD commented Jun 18, 2022

I thought dynamic pipelines were a different thing, anything that is triggered at pipeline level can't be driven by the bridge nodes and can't happen under gst-launch

I certainly want to add dynamic pipelines to the pipeline node so that you can reconfigure the pipeline on a remote host from a ros param update callback, but that's a separate thing.

caps renegotiation can be triggered by an element under gst-launch

@sandman
Copy link
Author

sandman commented Jun 21, 2022

Hi @BrettRD, you are right!

I looked into this a bit: we can add a data probe on the sink pad of rosimagesrc and then in the callback access the caps from the incoming message, something like this:

// Add the data probe on sink pad of rosimagesrc
 pad = gst_element_get_static_pad (rosimagesrc, "sink");
  gst_pad_add_probe (pad, GST_PAD_PROBE_TYPE_BUFFER,
      (GstPadProbeCallback) cb_have_data, NULL, NULL);
  gst_object_unref (pad);

// Wait for the probe call back to be fired
static GstPadProbeReturn
cb_have_data (GstPad          *pad,
              GstPadProbeInfo *info,
              gpointer         user_data)
{

    // Somehow need to get the Caps from the buffer. 
   // Can be done if the ROS msg is available / read before the callback is fired
  // Set the caps here
..
  // Remove the probe with gst_pad_remove_probe

    return GST_PAD_PROBE_OK;
}

All this is well and good, but the issue is that rosimagesrc is not of type GstElement. I haven't completely understood the abstraction with klass to figure out how the wrapper works. Perhaps you could help me out here?

@BrettRD
Copy link
Owner

BrettRD commented Jun 22, 2022

Thanks for looking that up!

There is a threading/locking bug in _wait_for_msg called at _create, so don't be afraid to make large changes.
The new ROS message becomes available during _create, can we negotiate new caps then? or do we have to reach higher in the abstraction?

The klass wrapper is pointer magic; the first member of a klass is at the same pointer as the klass.
Have a go with this example: https://godbolt.org/z/qefWKWzhE

You can simply cast the rosimagesrc to anything that it's inherited from and interact with that. That includes rosbasesrc, GstBaseSrc, GstElement, and GObject
(this is also why GObject is not capable of multiple inheritance)

rosbasesrc.h:
  ...
  typedef struct _RosBaseSrc RosBaseSrc;
  struct _RosBaseSrc{
    GstBaseSrc parent;
    ...

use the macros for clarity though: RosBaseSrc *src = GST_ROS_BASE_SRC (element);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants