rosbag2 icon indicating copy to clipboard operation
rosbag2 copied to clipboard

Allow write/record to bare file(s) instead of directory with `metadata.yaml`

Open emersonknapp opened this issue 2 years ago • 7 comments

Description

Allow user to record to a bare file (possibly multiple, with splitting). e.g. ros2 bag record -o my_bag.mcap

  • Append _N suffixes to bag base name, if splitting
  • Omit metadata.yaml creation in this case

Related Issues

Related to #972

Implementation Notes / Suggestions

Mostly work in SequentialWriter

emersonknapp avatar Aug 19 '22 21:08 emersonknapp

Would this need a special option? Right now my_bag.mcap would produce a directory with that name and put storage files inside it. We could change that behavior, perhaps, to look for an extension, and if there is a plugin that can handle that extension, then use it for the file. If no plugin found, then create directory with that name? Throw an error? Talking through that makes me think that probably we would need an arg for it.

emersonknapp avatar Aug 19 '22 22:08 emersonknapp

@emersonknapp I like your proposal:

We could change that behavior, perhaps, to look for an extension, and if there is a plugin that can handle that extension, then use it for the file. If no plugin found, then create directory with that name.

I am opposed to the adding special argument for it. We've already have to much parameters in CLI interface. Trying to guess by detected extension is smart and user friendly behavior. Falling back to creating folder if plugin match not found looks consistent and expected from user point of view IMO.

MichaelOrlov avatar Sep 01 '22 02:09 MichaelOrlov

After thinking about this issue some more, I think directly supporting -o foo.mcap and auto-detecting the plugin is a bit of a weird goal. It conflicts with other CLI flags such as auto-splitting, and just passing -s <plugin> seems easier in most cases to change the format.

Auto-detecting the plugin on read (https://github.com/ros2/rosbag2/issues/972) was an important issue and we have implemented that already.

Therefore, I would advocate for closing this ticket as it is written today. However, I do think that we should consider making the metadata.yaml file something that is only written by the sqlite plugin, or removing it entirely. This would open up the possibility of recording files in current working directory (similar to ROS 1, which I think is how most people expect it to work). Prepending a folder to the output files could be optional.

The main reason for metadata.yaml to exist (separating index from payload) is a sqlite limitation - with mcap it is easy to read the index without downloading the whole file.

amacneil avatar Sep 13 '22 04:09 amacneil

@amacneil Could you please clarify your statement about "It conflicts with other CLI flags such as auto-splitting"? I don't quite get it how it conflicts?

MichaelOrlov avatar Sep 13 '22 07:09 MichaelOrlov

@MichaelOrlov what I meant is that if I run the ros2 bag record -a -d 60 -o my_bag.mcap I only gave you one output file and it's not clear how you would split it.

I guess you could ignore my requested output file name, and name the files my_bag001.mcap, my_bag002.mcap, etc. That's not a bad option, but it seems weird that -o my_bag.mcap does not guarantee a file my_bag.mcap will exist afterwards.

It's a bit simpler with an output directory, or outputting to CWD as ROS 1 does.

amacneil avatar Sep 25 '22 22:09 amacneil

@amacneil Currently we are adding sequence number to the filename starting from _0. For the case when filename specified explicitly for instance ros2 bag record -a -d 60 -o my_bag.mcap we can start from exact name my_bag.mcap and continue with my_bag_1.mcap, my_bag_2.mcap when split happened.
It looks consistent and pretty much predictable.

MichaelOrlov avatar Sep 28 '22 06:09 MichaelOrlov

i.e. when specified just folder name ros2 bag record -a -d 60 -o my_bag -s mcap the file naming inside my_bag folder will be my_bag_0.mcap, my_bag_1.mcap, my_bag_2.mcap. Anyway we store filenames in metadata and rely on it during playback. It is not going to affect playback in this case. For reindex we can workaround this situation and try to search for my_bag.mcap if my_bag_0.mcap not found in folder.

MichaelOrlov avatar Sep 28 '22 06:09 MichaelOrlov