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

Rust: make mcap-rs wasm compatible #989

Merged
merged 5 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,9 @@ jobs:
components: "rustfmt, clippy"
- run: cargo fmt --all -- --check
- run: cargo clippy -- --no-deps
- run: cargo clippy --no-default-features -- --no-deps
- run: cargo clippy --no-default-features --features lz4 -- --no-deps
- run: cargo clippy --no-default-features --features zstd -- --no-deps
- run: cargo build
- run: cargo test
- name: "publish to crates.io"
Expand Down
16 changes: 11 additions & 5 deletions rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
name = "mcap"
description = "A library for reading and writing MCAP files"
homepage = "https://mcap.dev"
keywords = [ "foxglove", "mcap" ]
categories = [ "science::robotics", "compression" ]
keywords = ["foxglove", "mcap"]
categories = ["science::robotics", "compression"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this some auto-formatter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I can revert it.

repository = "https://github.com/foxglove/mcap"
documentation = "https://docs.rs/mcap"
readme = "README.md"
Expand All @@ -19,21 +19,27 @@ byteorder = "1.4"
crc32fast = "1.3"
enumset = "1.0.11"
log = "0.4"
lz4 = "1.0"
num_cpus = "1.13"
paste = "1.0"
thiserror = "1.0"
lz4_flex = { version = "0.11.1", optional = true }

[target.'cfg(target_arch = "wasm32")'.dependencies]
zstd = { version = "0.11", features = ["wasm"], optional = true }

[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
Comment on lines +27 to +30
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice - appreciate you only pulling in wasm support when it's the target!

zstd = { version = "0.11", features = ["zstdmt"], optional = true }

[features]
default = ["zstd"]
default = ["zstd", "lz4"]
zstd = ["dep:zstd"]
lz4 = ["dep:lz4_flex"]

[dev-dependencies]
anyhow = "1"
atty = "0.2"
camino = "1.0"
clap = { version = "3.2", features = ["derive"]}
clap = { version = "3.2", features = ["derive"] }
itertools = "0.10"
memmap = "0.7"
rayon = "1.5"
Expand Down
20 changes: 10 additions & 10 deletions rust/examples/common/conformance_writer_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub struct Record {
}

impl Record {
pub fn get_field(self: &Self, name: &str) -> &Value {
pub fn get_field(&self, name: &str) -> &Value {
return &self
.fields
.iter()
Expand All @@ -29,19 +29,19 @@ impl Record {
.1;
}

pub fn get_field_data(self: &Self, name: &str) -> Vec<u8> {
pub fn get_field_data(&self, name: &str) -> Vec<u8> {
let data: Vec<u8> = self
.get_field(name)
.as_array()
.unwrap_or_else(|| panic!("Invalid: {}", name))
.into_iter()
.iter()
.filter_map(|v| v.as_u64())
.filter_map(|n| u8::try_from(n).ok())
.collect();
return data;
data
}

pub fn get_field_meta(self: &Self, name: &str) -> BTreeMap<String, String> {
pub fn get_field_meta(&self, name: &str) -> BTreeMap<String, String> {
let data = self
.get_field(name)
.as_object()
Expand All @@ -50,33 +50,33 @@ impl Record {
for (key, value) in data.iter() {
result.insert(key.to_string(), value.as_str().unwrap().to_string());
}
return result;
result
}

pub fn get_field_str(self: &Self, name: &str) -> &str {
pub fn get_field_str(&self, name: &str) -> &str {
return self
.get_field(name)
.as_str()
.unwrap_or_else(|| panic!("Invalid: {}", name));
}

pub fn get_field_u16(self: &Self, name: &str) -> u16 {
pub fn get_field_u16(&self, name: &str) -> u16 {
return self
.get_field(name)
.as_str()
.and_then(|s| s.parse::<u16>().ok())
.unwrap_or_else(|| panic!("Invalid: {}", name));
}

pub fn get_field_u32(self: &Self, name: &str) -> u32 {
pub fn get_field_u32(&self, name: &str) -> u32 {
return self
.get_field(name)
.as_str()
.and_then(|s| s.parse::<u32>().ok())
.unwrap_or_else(|| panic!("Invalid: {}", name));
}

pub fn get_field_u64(self: &Self, name: &str) -> u64 {
pub fn get_field_u64(&self, name: &str) -> u64 {
return self
.get_field(name)
.as_str()
Expand Down
8 changes: 3 additions & 5 deletions rust/examples/conformance_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@ fn write_file(spec: &conformance_writer_spec::WriterSpec) {
}
"DataEnd" => {
let data_section_crc = record.get_field_u32("data_section_crc");
let _data_end = mcap::records::DataEnd {
data_section_crc: data_section_crc,
};
let _data_end = mcap::records::DataEnd { data_section_crc };
// write data end
}
"Footer" => {
Expand Down Expand Up @@ -105,7 +103,7 @@ fn write_file(spec: &conformance_writer_spec::WriterSpec) {
let name = record.get_field_str("name");
let encoding = record.get_field_str("encoding");
let id = record.get_field_u64("id");
let data: Vec<u8> = record.get_field_data(&"data");
let data: Vec<u8> = record.get_field_data("data");
let schema = mcap::Schema {
name: name.to_owned(),
encoding: encoding.to_owned(),
Expand All @@ -128,7 +126,7 @@ fn write_file(spec: &conformance_writer_spec::WriterSpec) {

let contents = std::fs::read(tmp_path).expect("Couldn't read output");
std::io::stdout()
.write(&contents)
.write_all(&contents)
.expect("Couldn't write output");
}

Expand Down
4 changes: 4 additions & 0 deletions rust/src/io_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ pub struct CountingCrcReader<R> {
}

impl<R: Read> CountingCrcReader<R> {
/// Creates a new `CountingCrcReader` with the given reader.
///
/// This is not used when both `lz4` and `zstd` features are disabled.
#[allow(dead_code)]
pub fn new(inner: R) -> Self {
Self {
inner,
Expand Down
1 change: 1 addition & 0 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ pub const MAGIC: &[u8] = &[0x89, b'M', b'C', b'A', b'P', 0x30, b'\r', b'\n'];
pub enum Compression {
#[cfg(feature = "zstd")]
Zstd,
#[cfg(feature = "lz4")]
Lz4,
}

Expand Down
8 changes: 7 additions & 1 deletion rust/src/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,8 @@ fn read_record(op: u8, body: &[u8]) -> McapResult<records::Record<'_>> {

enum ChunkDecompressor<'a> {
Null(LinearReader<'a>),
/// This is not used when both `zstd` and `lz4` features are disabled.
#[allow(dead_code)]
Compressed(Option<CountingCrcReader<Box<dyn Read + Send + 'a>>>),
}

Expand All @@ -274,10 +276,14 @@ impl<'a> ChunkReader<'a> {
#[cfg(not(feature = "zstd"))]
"zstd" => panic!("Unsupported compression format: zstd"),

#[cfg(feature = "lz4")]
"lz4" => ChunkDecompressor::Compressed(Some(CountingCrcReader::new(Box::new(
lz4::Decoder::new(data)?,
lz4_flex::frame::FrameDecoder::new(data),
)))),

#[cfg(not(feature = "lz4"))]
"lz4" => panic!("Unsupported compression format: lz4"),

"" => {
if header.uncompressed_size != header.compressed_size {
warn!(
Expand Down
1 change: 1 addition & 0 deletions rust/src/records.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ fn parse_vec<T: BinRead<Args<'static> = ()>>() -> BinResult<Vec<T>> {
Ok(parsed)
}

#[allow(clippy::ptr_arg)]
#[binrw::writer(writer, endian)]
fn write_vec<T: BinWrite<Args<'static> = ()>>(v: &Vec<T>) -> BinResult<()> {
use std::io::SeekFrom;
Expand Down
30 changes: 16 additions & 14 deletions rust/src/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,7 @@ impl WriteOptions {
/// If `None`, chunks will not be automatically closed and the user must call `flush()` to
/// begin a new chunk.
pub fn chunk_size(self, chunk_size: Option<u64>) -> Self {
Self {
chunk_size: chunk_size,
..self
}
Self { chunk_size, ..self }
}

/// Creates a [`Writer`] whch writes to `w` using the given options
Expand Down Expand Up @@ -649,7 +646,8 @@ enum Compressor<W: Write> {
Null(W),
#[cfg(feature = "zstd")]
Zstd(zstd::Encoder<'static, W>),
Lz4(lz4::Encoder<W>),
#[cfg(feature = "lz4")]
Lz4(lz4_flex::frame::FrameEncoder<W>),
}

impl<W: Write> Compressor<W> {
Expand All @@ -658,11 +656,8 @@ impl<W: Write> Compressor<W> {
Compressor::Null(w) => w,
#[cfg(feature = "zstd")]
Compressor::Zstd(w) => w.finish()?,
Compressor::Lz4(w) => {
let (w, err) = w.finish();
err?;
w
}
#[cfg(feature = "lz4")]
Compressor::Lz4(w) => w.finish()?,
})
}
}
Expand All @@ -673,6 +668,7 @@ impl<W: Write> Write for Compressor<W> {
Compressor::Null(w) => w.write(buf),
#[cfg(feature = "zstd")]
Compressor::Zstd(w) => w.write(buf),
#[cfg(feature = "lz4")]
Compressor::Lz4(w) => w.write(buf),
}
}
Expand All @@ -682,6 +678,7 @@ impl<W: Write> Write for Compressor<W> {
Compressor::Null(w) => w.flush(),
#[cfg(feature = "zstd")]
Compressor::Zstd(w) => w.flush(),
#[cfg(feature = "lz4")]
Compressor::Lz4(w) => w.flush(),
}
}
Expand All @@ -706,7 +703,10 @@ impl<W: Write + Seek> ChunkWriter<W> {
let compression_name = match compression {
#[cfg(feature = "zstd")]
Some(Compression::Zstd) => "zstd",
#[cfg(feature = "lz4")]
Some(Compression::Lz4) => "lz4",
#[cfg(not(any(feature = "zstd", feature = "lz4")))]
Some(_) => unreachable!("`Compression` is an empty enum that cannot be instantiated"),
None => "",
};

Expand All @@ -726,14 +726,16 @@ impl<W: Write + Seek> ChunkWriter<W> {
let compressor = match compression {
#[cfg(feature = "zstd")]
Some(Compression::Zstd) => {
#[allow(unused_mut)]
let mut enc = zstd::Encoder::new(writer, 0)?;
#[cfg(not(target_arch = "wasm32"))]
enc.multithread(num_cpus::get_physical() as u32)?;
Compressor::Zstd(enc)
}
Some(Compression::Lz4) => {
let b = lz4::EncoderBuilder::new();
Compressor::Lz4(b.build(writer)?)
}
#[cfg(feature = "lz4")]
Some(Compression::Lz4) => Compressor::Lz4(lz4_flex::frame::FrameEncoder::new(writer)),
#[cfg(not(any(feature = "zstd", feature = "lz4")))]
Some(_) => unreachable!("`Compression` is an empty enum that cannot be instantiated"),
None => Compressor::Null(writer),
};
let compressor = CountingCrcWriter::new(compressor);
Expand Down
1 change: 1 addition & 0 deletions rust/tests/compression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ fn zstd_round_trip() -> Result<()> {
round_trip(Some(mcap::Compression::Zstd))
}

#[cfg(feature = "lz4")]
#[test]
fn lz4_round_trip() -> Result<()> {
round_trip(Some(mcap::Compression::Lz4))
Expand Down
Loading