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

Sequentially play multiple bagfiles without metadata.yaml #1918

Open
Tracked by #1915
emersonknapp opened this issue Feb 22, 2025 · 3 comments
Open
Tracked by #1915

Sequentially play multiple bagfiles without metadata.yaml #1918

emersonknapp opened this issue Feb 22, 2025 · 3 comments
Labels
enhancement New feature or request

Comments

@emersonknapp
Copy link
Collaborator

Description

With continuous or just long recording sessions, users need to retrieve and play back just a specific subset of files or a specific directory, which will not have metadata.yaml. That data should not be required to sequentially play a series of files.

Related Issues

** Is this feature dependent on any other features? Is it part of a larger project? Note here. **

Completion Criteria

  • ros2 bag play directory_without_metadata/ will play all discovered files in filesystem order (alphabetical)
  • ros2 bag play fileA.mcap fileB.mcap fileC.mcap will play all specified files sequentially in the order specified

Maybe the directory option isn't required, because the user could do file globbing,

ros2 bag play my_recording/*.mcap

Which would be fine except you lose autodetection, and if you do

ros2 bag play my_recording/*

then it might choke on for example metadata.yaml, which isn't a bagfile.

Implementation Notes / Suggestions

I believe this comes down to removing the metadata.yaml requirement checks from SequentialReader - and instead falling back to just reading the directory.

The multifile option could be as simple as ros2bag CLI allowing nargs='+', then running for bag in args.bags: Play() though this would be a naive implementation that would lose Publisher continuity and introduce latency on file boundaries. So probably better to pass the list of files to SequentialReader which would override the metadata.yaml check.

Testing Notes / Suggestions

** All features in this project need tests. Please give some input on cases that will need to be tested - and how the testing might be implemented. **

@emersonknapp emersonknapp added the enhancement New feature or request label Feb 22, 2025
@emersonknapp emersonknapp changed the title Sequentially play a directory or list of bagfiles without metadata.yaml Sequentially play multiple bagfiles without metadata.yaml Feb 22, 2025
@MichaelOrlov
Copy link
Contributor

@emersonknapp I was thinking about what if add some intelligence to the Rosbag2 player. In case if someone want to replay the directory with files without metadata.yaml file ( i.e. ros2 bag play directory_without_metadata/) the player would do automatically reindexing and creating metadata on the fly the same way as ros2 bag reindex directory_without_metadata/ does but without saving metadata.yaml file?

void Reindexer::reindex(const rosbag2_storage::StorageOptions & storage_options)
{
base_folder_ = storage_options.uri;
ROSBAG2_CPP_LOG_INFO_STREAM("Beginning reindexing bag in directory: " << base_folder_);
auto metadata_io_default = std::make_unique<rosbag2_storage::MetadataIo>();
auto bag_reader = std::make_unique<rosbag2_cpp::readers::SequentialReader>(
std::move(storage_factory_), converter_factory_, std::move(metadata_io_default));
// Identify all bag files
std::vector<fs::path> files;
get_bag_files(base_folder_, files);
if (files.empty()) {
throw std::runtime_error("No storage files found for reindexing. Abort");
}
init_metadata(files, storage_options);
ROSBAG2_CPP_LOG_DEBUG_STREAM("Completed init_metadata");
// Collect all metadata from files
aggregate_metadata(files, bag_reader, storage_options);
ROSBAG2_CPP_LOG_DEBUG_STREAM("Completed aggregate_metadata");
metadata_io_->write_metadata(base_folder_.generic_string(), metadata_);
ROSBAG2_CPP_LOG_INFO("Reindexing complete.");
}

It seems we already have all the necessary heavy-lifting code in the reindexer class - and the fix is going to be literally in 10 lines of code or so. The benefit of it is a consistent behavior with the ros2 bag reindex CLI option.
We will need to rewrite this part of "else" code

} else {
storage_ = storage_factory_->open_read_only(storage_options_);
if (!storage_) {
throw std::runtime_error("No storage could be initialized for the input URI: " +
storage_options.uri);
}
if (!set_read_order(read_order_)) {
ROSBAG2_CPP_LOG_WARN(
"Could not set read order on open(), falling back to file order");
if (!set_read_order(kFallbackOrder)) {
throw std::runtime_error("Could not set read order on open()");
}
}
metadata_ = storage_->get_metadata();
if (metadata_.relative_file_paths.empty()) {
ROSBAG2_CPP_LOG_WARN("No file paths were found in metadata.");
return;
}
file_paths_ = metadata_.relative_file_paths;
current_file_iterator_ = file_paths_.begin();

to use reindexing instead of reading metadata from bag file directly.

Thoughts?

@emersonknapp
Copy link
Collaborator Author

Yeah, I think that a "reindex on-demand" type of implementation makes sense to me! It would then validate right at the beginning that the files are playable. I think we'd want to allow for it skipping files it can't play, like other meta-information files, rather than failing, but that's an error handling detail.

@MichaelOrlov
Copy link
Contributor

Currently, we have a specific regex for choosing bag file names with recording. Log files and other specific non-robag2 files should already be filtered out by default.

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

No branches or pull requests

2 participants