search

klingtnet / rosc

An OSC library for Rust.
Apache License 2.0
173 stars 25 forks source link

Support empty bundles #11

Closed glindstedt closed 4 years ago

glindstedt commented 4 years ago

From the spec at http://opensoundcontrol.org/spec-1_0

An OSC Bundle consists of the OSC-string "#bundle" followed by an OSC Time Tag, followed by zero or more OSC Bundle Elements. The OSC-timetag is a 64-bit fixed point time tag whose semantics are described below.

An OSC Bundle Element consists of its size and its contents. The size is an int32 representing the number of 8-bit bytes in the contents, and will always be a multiple of 4. The contents are either an OSC Message or an OSC Bundle.

I discovered this wasn't supported in the wild where a specific OSC plugin for a DAW seems to be using empty bundles as heartbeat messages, which would cause the library to panic.

The function names in the code seem to reflect some confusion around the "Bundle Element", the read_bundle_element() function for example reads the contents of the element, but the way I interpret the spec is that an "element" is the tuple (size, content). If you think it's worth it I could try to refactor the read_bundle_element and read_bundle_element_size functions to match this, just to align code structure with spec concepts.

glindstedt commented 4 years ago

I guess it would also clarify the API, so please give it a try but open another PR for this change.

Sure thing, I'll give it a try

klingtnet commented 4 years ago

I am ready to merge and release a 0.3.1 if you're okay with it?

glindstedt commented 4 years ago

Yes definitely, go ahead. Although crates.io says 0.4.0 is the latest?

glindstedt commented 4 years ago

After looking into potential refactoring I'm not convinced myself that there's much benefit, although I would probably rename read_bundle_element() to read_bundle_element_content() for clarity

klingtnet commented 4 years ago

Although crates.io says 0.4.0 is the latest?

You're correct, I will have to check why I did not push the 0.4.0 tag.

klingtnet commented 4 years ago

After looking into potential refactoring I'm not convinced myself that there's much benefit, although I would probably rename read_bundle_element() to read_bundle_element_content() for clarity

Sounds fine to me, will rename it sometime this week.

klingtnet commented 4 years ago

I renamed the function and released a v0.4.2 since this is part of the internal API.

glindstedt commented 4 years ago

Nice 👍