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

improve workflow to avoid added latency #895

Open
2 tasks done
bmegli opened this issue May 17, 2023 · 5 comments
Open
2 tasks done

improve workflow to avoid added latency #895

bmegli opened this issue May 17, 2023 · 5 comments

Comments

@bmegli
Copy link

bmegli commented May 17, 2023

Preliminary Checks

  • This issue is not a duplicate. Before opening a new issue, please search existing issues.
  • This issue is not a question, bug report, or anything other than a feature request directly related to this project.

Proposal

Publish depth/rgb data after grabbing instead of:

  • grab polling thread
  • ROS publishing thread

With:

  • 30 hz grab thread
  • 30 hz ROS publish thread

This is adding worst case single frame latency (publish just before new data) and half frame latency on average.

  • 33 ms or 33/2 ms respectively

Use-Case

Any low-latency ROS application needing depth/RGB data

Anything else?

grab thread (std::thread)

void ZEDWrapperNodelet::device_poll_thread_func()

ROS publish thread (ROS thread)

mVideoDepthTimer = mNhNs.createTimer(ros::Duration(1.0 / mVideoDepthFreq), &ZEDWrapperNodelet::callback_pubVideoDepth, this);

Run separately


It may be possible to keep current code structure and trigger ROS publishing after successful grab
(as opposed to timer based publishing)

@Myzhar
Copy link
Member

Myzhar commented May 17, 2023

The original ROS wrapper was made in that way.
We added a separate publishing thread when other users (too much to not listen to their request) asked to publish data at a lower frequency than the grab frequency.

I can understand your requirement, you can apply your modification if you need it.
I'm sorry, but we are not planning major changes to the ROS wrapper since ROS is close to EOL and most of the ROS users are moving to ROS 2.

@bmegli
Copy link
Author

bmegli commented May 17, 2023

In that case, thank you for a quick answer.

@bmegli bmegli closed this as completed May 17, 2023
bmegli added a commit to Extend-Robotics/zed-ros-wrapper that referenced this issue May 18, 2023
- allow setting pub_frame_rate to 0
- when 0
  - disable timer based publishing
  - instead publish immidiately after grabbing

Within current worklow publishing and grabbing threads work separately.

With publishing rate == grabbing rate this
- adds average FRAME_TIME/2 latency
- adds pessimistic FRAME_TIME latency

The fix removes latency by notifying ROS thread to publish after succesful grabbing

Fix for:
- stereolabs#895
@bmegli
Copy link
Author

bmegli commented May 18, 2023

For anybody in need to remove added latency in ROS1 fix is in:

To keep compatibility with existing behavior:

  • if pub_frame_rate > 0 - works as usual
  • if pub_frame_rate == 0 - (not valid value before)
    • timer based publishing is disabled
    • grabber thread notifies about data availability through ROS empty message
    • data is published on notification

All above was used to keep existing threading model and behavior compatibility.

@bmegli bmegli reopened this May 18, 2023
@bmegli
Copy link
Author

bmegli commented May 18, 2023

@Myzhar

We avoid maintaining third-party forks if possible.

If the fix looks like something mergeable we are happy to open pull request (and adapt as requested).

If not let me know, I will close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants