Skip to content
This repository was archived by the owner on Nov 30, 2022. It is now read-only.

Commit fcd2564

Browse files
Switch to using the vendored zstd library. (#59)
* Switch to using the vendored zstd library. We already have it vendored in the core as necessary. Signed-off-by: Chris Lalancette <[email protected]> * Add `can_write_mcap_with_zstd_configured_from_yaml` test - Add unit test to check that rosbag2_mcap_storage plugin can write mcap file with zstd compression configured via storage config yaml file Signed-off-by: Michael Orlov <[email protected]> * Address build failure on CI related to the missing header from zstd lib - Replace target_link_libraries(mcap PRIVATE zstd::zstd) with ament_target_dependencies(mcap zstd) Signed-off-by: Michael Orlov <[email protected]> Signed-off-by: Chris Lalancette <[email protected]> Signed-off-by: Michael Orlov <[email protected]> Co-authored-by: Michael Orlov <[email protected]>
1 parent 295ae9b commit fcd2564

File tree

5 files changed

+87
-16
lines changed

5 files changed

+87
-16
lines changed

mcap_vendor/CMakeLists.txt

+5-15
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ endif()
1313

1414
## Dependencies
1515
find_package(ament_cmake REQUIRED)
16+
find_package(zstd_vendor REQUIRED)
17+
find_package(zstd REQUIRED)
1618

1719

1820
## Define vendor macro
@@ -30,39 +32,25 @@ macro(build_mcap_vendor)
3032
)
3133
fetchcontent_makeavailable(lz4)
3234

33-
fetchcontent_declare(zstd
34-
GIT_REPOSITORY https://github.com/facebook/zstd.git
35-
GIT_TAG e47e674cd09583ff0503f0f6defd6d23d8b718d3 # v1.5.2
36-
)
37-
fetchcontent_makeavailable(zstd)
38-
39-
file(GLOB _zstd_srcs
40-
${zstd_SOURCE_DIR}/lib/common/*.c
41-
${zstd_SOURCE_DIR}/lib/compress/*.c
42-
${zstd_SOURCE_DIR}/lib/decompress/*.c
43-
${zstd_SOURCE_DIR}/lib/decompress/*.S
44-
)
45-
4635
file(GLOB _lz4_srcs
4736
${lz4_SOURCE_DIR}/lib/*.c)
4837

4938
add_library(
5039
mcap SHARED
5140
src/main.cpp
52-
${_zstd_srcs}
5341
${_lz4_srcs}
5442
)
5543

5644
set(_mcap_include_dir ${mcap_SOURCE_DIR}/cpp/mcap/include)
5745

5846
target_include_directories(mcap SYSTEM PRIVATE
5947
${lz4_SOURCE_DIR}/lib
60-
${zstd_SOURCE_DIR}/lib
6148
)
6249
target_include_directories(mcap SYSTEM PUBLIC
6350
"$<BUILD_INTERFACE:${_mcap_include_dir}>"
6451
"$<INSTALL_INTERFACE:include>"
6552
)
53+
ament_target_dependencies(mcap zstd)
6654

6755
install(
6856
DIRECTORY
@@ -79,6 +67,8 @@ build_mcap_vendor()
7967

8068
ament_export_targets(export_mcap HAS_LIBRARY_TARGET)
8169

70+
ament_export_dependencies(zstd_vendor zstd)
71+
8272
## Tests
8373
if(BUILD_TESTING)
8474
find_package(ament_lint_auto REQUIRED)

mcap_vendor/package.xml

+3-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99

1010
<buildtool_depend>ament_cmake</buildtool_depend>
1111
<buildtool_depend>git</buildtool_depend>
12-
12+
13+
<depend>zstd_vendor</depend>
14+
1315
<test_depend>ament_cmake_clang_format</test_depend>
1416
<test_depend>ament_lint_auto</test_depend>
1517
<test_depend>ament_lint_common</test_depend>

rosbag2_storage_mcap/CMakeLists.txt

+2
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ if(BUILD_TESTING)
8484
find_package(rosbag2_test_common REQUIRED)
8585
find_package(std_msgs REQUIRED)
8686

87+
add_definitions(-D_TEST_RESOURCES_DIR_PATH="${CMAKE_CURRENT_SOURCE_DIR}/test/${PROJECT_NAME}")
88+
8789
set(ament_cmake_clang_format_CONFIG_FILE .clang-format)
8890
list(APPEND AMENT_LINT_AUTO_EXCLUDE ament_cmake_uncrustify)
8991
ament_lint_auto_find_test_dependencies()
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
noCRC: false
2+
noChunking: false
3+
noMessageIndex: false
4+
noSummary: false
5+
chunkSize: 786432
6+
compression: "Zstd"
7+
compressionLevel: "Fast"
8+
forceCompression: true

rosbag2_storage_mcap/test/rosbag2_storage_mcap/test_mcap_storage.cpp

+69
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,72 @@ TEST_F(TemporaryDirectoryFixture, can_write_and_read_basic_mcap_file) {
9999
EXPECT_EQ(msg.data, message_data);
100100
}
101101
}
102+
103+
#ifdef ROSBAG2_STORAGE_MCAP_HAS_STORAGE_OPTIONS
104+
// This test disabled on Foxy since StorageOptions doesn't have storage_config_uri field on it
105+
TEST_F(TemporaryDirectoryFixture, can_write_mcap_with_zstd_configured_from_yaml) {
106+
auto uri = rcpputils::fs::path(temporary_dir_path_) / "bag";
107+
auto expected_bag = uri / "bag_0.mcap";
108+
const int64_t timestamp_nanos = 100; // arbitrary value
109+
rcutils_time_point_value_t time_stamp{timestamp_nanos};
110+
const std::string topic_name = "test_topic";
111+
const std::string message_data = "Test Message 1";
112+
const std::string storage_id = "mcap";
113+
const std::string config_path = _TEST_RESOURCES_DIR_PATH;
114+
// COMPATIBILITY(foxy)
115+
// using verbose APIs for Foxy compatibility which did not yet provide plain-message API
116+
rclcpp::Serialization<std_msgs::msg::String> serialization;
117+
118+
{
119+
StorageOptions options;
120+
options.uri = uri.string();
121+
options.storage_id = storage_id;
122+
options.storage_config_uri = config_path + "/mcap_writer_options_zstd.yaml";
123+
rosbag2_storage::TopicMetadata topic_metadata;
124+
topic_metadata.name = topic_name;
125+
topic_metadata.type = "std_msgs/msg/String";
126+
127+
std_msgs::msg::String msg;
128+
msg.data = message_data;
129+
130+
rosbag2_cpp::Writer writer{std::make_unique<rosbag2_cpp::writers::SequentialWriter>()};
131+
#ifndef ROSBAG2_STORAGE_MCAP_WRITER_CREATES_DIRECTORY
132+
rcpputils::fs::create_directories(uri);
133+
#endif
134+
writer.open(options, rosbag2_cpp::ConverterOptions{});
135+
writer.create_topic(topic_metadata);
136+
137+
auto serialized_msg = std::make_shared<rclcpp::SerializedMessage>();
138+
serialization.serialize_message(&msg, serialized_msg.get());
139+
140+
// This is really kludgy, it's due to a mismatch between types in the rclcpp serialization API
141+
// and the historical Foxy serialized APIs. Prevents the hacked shared ptr from deleting the
142+
// data that `serialized_bag_msg` should reasonably expect to continue existing.
143+
// For this example it wouldn't matter, but in case anybody extends this test, it's for safety.
144+
auto serialized_bag_msg = std::make_shared<rosbag2_storage::SerializedBagMessage>();
145+
serialized_bag_msg->serialized_data = std::shared_ptr<rcutils_uint8_array_t>(
146+
const_cast<rcutils_uint8_array_t*>(&serialized_msg->get_rcl_serialized_message()),
147+
[](rcutils_uint8_array_t* /* data */) {});
148+
serialized_bag_msg->time_stamp = time_stamp;
149+
serialized_bag_msg->topic_name = topic_name;
150+
writer.write(serialized_bag_msg);
151+
writer.write(serialized_bag_msg);
152+
EXPECT_TRUE(expected_bag.is_regular_file());
153+
}
154+
{
155+
StorageOptions options;
156+
options.uri = expected_bag.string();
157+
options.storage_id = storage_id;
158+
159+
rosbag2_cpp::Reader reader{std::make_unique<rosbag2_cpp::readers::SequentialReader>()};
160+
reader.open(options, rosbag2_cpp::ConverterOptions{});
161+
EXPECT_TRUE(reader.has_next());
162+
163+
std_msgs::msg::String msg;
164+
auto serialized_bag_msg = reader.read_next();
165+
rclcpp::SerializedMessage extracted_serialized_msg(*serialized_bag_msg->serialized_data);
166+
serialization.deserialize_message(&extracted_serialized_msg, &msg);
167+
EXPECT_EQ(msg.data, message_data);
168+
}
169+
}
170+
#endif // #ifdef ROSBAG2_STORAGE_MCAP_HAS_STORAGE_OPTIONS

0 commit comments

Comments
 (0)