Repository navigation
fix: parse ROS image trailers and expose RealSense depth units - #155
Closed
theluckystrike wants to merge 1 commit into
Closed
theluckystrike wants to merge 1 commit into
theluckystrike wants to merge 1 commit into
Conversation
ROSBag no longer guesses how many trailing bytes to strip. Image and CompressedImage messages are parsed up to the end of their data array for ROS1 and CDR, legal CDR padding is accepted, and a 4-byte RealSense footer is decoded as depth_units instead of being dropped. ROSBag gains depth_units (None, 'auto' or a float, metres per unit) and invalid_is_nan, with the precedence described in the issue. Closes petercorke#151
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 64 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
4 tasks done
Owner
|
Thank you for your interest in MVTB, and for this contribution. We have adopted your idea for building synthetic RealSense bags in the tests (#163), and credited you as co-author. We have left out the type-registration changes and the topic-name depth detection, and the |
petercorke
added a commit
that referenced
this pull request
Oct 22, 2026
…a guess (#163) The Intel RealSense SDK follows every image message in a bag with 4 extra bytes (a float32 depth scale). The reader handled that by assuming 4 trailing bytes on every message of any bag that has a realsense_msgs connection, and only when release="auto". Tested on real bags this had three problems: - ROSBag(path) with the default release="ROS1_NOETIC" read no frames at all from the RealSense recording: SerdeError "ROS1 message size mismatch". Current rosbags raises SerdeError, the code only caught AssertionError. - Intel's own sample bag (single_depth_color_640x480.bag) has realsense_msgs connections but no extra bytes. With release="auto" the reader removed 4 real bytes from every image and silently returned no frames. - The depth scale was discarded. Now sensor_msgs/Image and CompressedImage (ROS 1 and CDR, either byte order) are parsed up to the end of their data array, checking that the array length is height * step. Only those bytes are deserialized and anything after them is the footer. Pixels are unchanged. What to make of the footer is decided by context, because the bytes cannot decide it: about half of all 4-byte patterns are a positive, finite float32. In a bag recognised as RealSense (a realsense_msgs connection), a positive finite float32 is attached to the image as `depth_units` (metres per unit) and 4 zero bytes (a color image) mean no scale. Anything else after an image, such as the same 4 bytes in a bag that is not recognised, another length, or a negative or non-finite value, is ignored with a RuntimeWarning, once per topic. CDR padding (up to 3 zero bytes) is silent. Every other message type must be exactly the length its type says, as a serialization format requires, and is no longer tried with 0, 4 and 8 bytes removed. That guess was unnecessary: all 68,889 non-image messages in the 13 real bags tested decode at their exact length. Verified on every bag in bags/ (14 files, old reader vs new): output is byte-identical for all except the two cases above, which now work; the RealSense bag gives depth_units == 0.001 on its 37 depth frames and no warnings. Adds tests/test_ros_realsense.py: the layout parser and footer decoder, synthetic RealSense-style bags written to a temporary directory by tests/create_ros_fixture.py (ROS 1 and CDR, with and without footers and realsense_msgs), unexpected trailing bytes, exact-length decoding of other messages, and checks against the real bags when they are present. Refs #151. The depth_units=/invalid_is_nan= conversion options are not in this change. The layout-parsing approach follows #154 and the technique for building the synthetic bag follows #155. Co-authored-by: TianHengZhuang <39034691+TianHengZhuang@users.noreply.github.com> Co-authored-by: theluckystrike <51033404+theluckystrike@users.noreply.github.com> Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Owner
|
Closing as superseded by #163, which has now merged with your synthetic RealSense bag technique credited on the commit. The |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #151
ROSBagguessed how many trailing bytes to strip from each image message, so RealSense bags lost their depth units footer and a RealSense bag with no trailer (Intel'ssingle_depth_color_640x480.baglayout) would have had 4 real bytes chopped off.What changed, following the checklist in the issue
ImageandCompressedImageare parsed by hand up to the end of the data array (header, height, width, encoding, is_bigendian, step, then the array length), for both ROS1 and CDR. Onlyraw[:end]goes to the type store,n == height * stepis checked, and legal CDR alignment padding is accepted. The old 0/4/8 probe is kept only as a fallback for other message types.tests/create_ros_fixture.py, with a regression test for a bag that hasStreamInfoand 0 extra bytes.depth_unitson the sample (depth frames carry the scale, colour frames0.0).StreamInfoandCameraInfogo through the fallback trailer check.printin this path is replaced withlogging.Depth units API
ROSBag(..., depth_units=None | "auto" | float, invalid_is_nan=True), wheredepth_unitsis metres per native unit as in librealsense. The docstring notes that this is the reciprocal of the Open3D and TUM RGB-Ddepth_scale(1000 and 5000).The scale comes from, in order, a user float, the per message footer, the bag's
.../option/Depth_Units/valuetopic, the REP 118 encoding (16UC1is millimetres,32FC1is metres), then unknown.None(default) returns the native dtype unchanged withdepth_unitsattached as metadata."auto"or a float converts to float32 metres, and raw 0 becomes NaN wheninvalid_is_nanis true. For32FC1a user float acts as a gain.Type registration for bag connections now distinguishes ROS1
.msgfrom ROS2 IDL definitions, registers only missing types and skips incompatible typestore candidates during auto detection, so an unrelated or custom IDL type can't abort reading the images.Tests
301 tests pass in
tests/test_ros.pyand the full suite passes locally. Ruff reports no new findings in the changed files.One related thing I left alone because it behaves the same on
main. The per message handlers catchKeyErrorandAssertionErrorbut not rosbags'SerdeError, so a bag with an undecodable non image message still stops iteration. I can follow up in #150 if you want that handled.