Use mcap storage compression when compression is requested

Aposhian commented 9 months ago

Description

You can enable compression via --compression-mode and --compression-format with -s mcap, but this produces:

with --compression-mode=file: .mcap.zstd files that tools like Foxglove Studio does not understand
with --compression-mode=message: Foxglove Studio cannot read .mcap files encoded like this.

However, if you make a config file like mcap-config.yaml:

compression: Zstd
compressionLevel: Default

and then run:

ros2 bag record -s mcap --storage-config-file ./mcap-config.yaml

Then you get a .mcap file that is compressed with zstd, that Foxglove Studio can also load. This also gives you control over compression level/speed.

I understand that it makes difficult for rosbag2 to treat mcap's compression as a special case, but currently if you are trying to figure out how to use compression with rosbag2, your go-to in the documentation will simply be to use the top-level flags, which gives a suboptimal result for mcap.

Related Issues

N/A

Completion Criteria

Users are not misled into producing an unreadable .mcap file.

Implementation Notes / Suggestions

Maybe rosbag2 should just give a warning or something when you specify --compression-mode with the mcap storage plugin.

Testing Notes / Suggestions

Need to test that produced .mcap files are compatible with Foxglove Studio.

tobiastorben commented 7 months ago

Second this. We have a lot of bags that have been recorded in the mcap format with message-level compression that we are unable to open. Is there any way to salvage these?

Aposhian commented 7 months ago

Second this. We have a lot of bags that have been recorded in the mcap format with message-level compression that we are unable to open. Is there any way to salvage these?

I would think you should be able to use ros2 bag convert if you specify that the input bag has message level compression, but I haven't tried that.

MichaelOrlov commented 7 months ago

What we can do here is to update the documentation in the readme file.

The --compression-mode and --compression-format produce different types of compression which unified and independent of the underlying storage plugins or file formats. This type of compression is done on a higher level and still could be a valid option for MCAP in some cases.

I would appreciate someone's help with updating the readme file in this regard.

Aposhian commented 7 months ago

The --compression-mode and --compression-format produce different types of compression which unified and independent of the underlying storage plugins or file formats. This type of compression is done on a higher level and still could be a valid option for MCAP in some cases.

I do appreciate that --compression-mode and --compression-format are storage type agnostic. However, I'm having a hard time understanding when you would want to use them over the native compression format that is still an indexable file. I think updating the README would be good, but also providing a CLI warning with this storage plugin and CLI option combination could be helpful.

tobiastorben commented 7 months ago

Second this. We have a lot of bags that have been recorded in the mcap format with message-level compression that we are unable to open. Is there any way to salvage these?

I would think you should be able to use ros2 bag convert if you specify that the input bag has message level compression, but I haven't tried that.

Thanks! ros2 bag convert -i <path_to_bag_dir> -o decompress.yaml was able to recover the uncompressed bag. decompress.yaml:

output_bags:
- uri: decompressed
  all: true
  storage_id: "mcap"

defunctzombie commented 5 months ago

@MichaelOrlov Thoughts on disabling --compression-mode=message when recording to an mcap file? MCAP does not have message level compression which is what leads to using this mode producing mcap files that other tools cannot read.

defunctzombie commented 5 months ago

It has been pointed out to me that this bit of code should have prevented per-message compression but I also see it is only present if ROSBAG2_STORAGE_MCAP_HAS_UPDATE_METADATA is defined. Is this always defined?

MichaelOrlov commented 5 months ago

@defunctzombie We added update_metadata(..) API for the storage plugin interface some time ago in the scope of the https://github.com/ros2/rosbag2/issues/1149.
It is present on Iron, Rolling and Jazzy ROS 2 distros. However, it is absent on Humble.

If the matter of concern to add the same exception as in the https://github.com/ros2/rosbag2/blob/317286c98472205593d5b0a791d06fea42613ba3/rosbag2_storage_mcap/src/mcap_storage.cpp#L845-L849 somewhere on Humble - I am fine with it.

ros2 / rosbag2