rbdannenberg / o2 Goto Github PK

o2-1.1 also fails on i386 (aka x86, x86_32 or just "Intel 32bit"), but that seems to be "only" an issue in the actual tests: comparing floats for equality with 123.456F+i is a brittle thing, esp. when coercing values.

A patch for the bad comparison could be found at https://salsa.debian.org/multimedia-team/o2/-/blob/master/debian/patches/float_comparision.patch

The patch is against o2-1.1 (so it won't apply against o2-2.x), but I'm currently stuck with #24 so with o2-2.x I don't even get to the float-comparision in my test-runs.
However, looking at

o2_malloc seems to return unaligned memory in my build. As a result, osc_to_o2 and I suspect much of o2 message parsing is broken.

running ./arraytest on a BIGendian machine (like s390x) aborts with an assertion:

$ ./arraytest 
arraytest: .../o2/src/o2node.cpp:239: int64_t get_hash(O2string): Assertion `((c & 0xff) && (c & 0xff00) && (c & 0xff0000) && (c & 0xff000000)) || ((((c & 0xff) != 0) || ((c & 0xff00) == 0)) && (((c & 0xff00) != 0) || ((c & 0xff0000) == 0)) && ((c & 0xff000000) == 0))' failed.
Aborted

this is because the assertion uses bitmasks that are only valid for littleendian.
here's a patch for o2-1.1:
https://salsa.debian.org/multimedia-team/o2/-/blob/master/debian/patches/big_endian.patch

eventually i will update the patch for o2-2.x and submit a PR (if you want to)

As tests are added, the number of Xcode targets is going to get very large, so make a CMake option to include the tests or not.

Every global symbol should start with o2_.

Every non-global symbol could be shortened by omitting the o2_ prefix.

In particular, let's write a test that repeatedly calls o2 functions and where malloc fails after a random number of calls -- this can then be left running (I hope) in a loop that constantly reinitializes o2 and runs until failure and cleans up.

running cmake without having the libavahi-client development headers installed succeeds without error, but the build fails.
you probably should check for their presence in the configuration stage and abort early.

how stable is the o2 API?

i noticed that o2 tends towards a version 2.0, changing the name of some types, the call semantics of a few functions and removing some functions alltogether.

given that version 2 is indeed a major version bump, this is totally ok and suggests that the API can indeed be considered stable according to semantic versioning.

do i get this correct?
a stable API means that it should be possible to make use of dynamic libraries (see #16 ), as opposed to building everything as static.
i'm mostly interested, because i'd like to get o2 into the Debian/Ubuntu/... universe as a proper package.

sidenote: for static building this is not an issue, but the public API (o2.h) contains conditional symbols, e.g.

is there any reason to not make the symbols in the header-files unconditional, and just provide dummy implementations if the library is compiled without debugging/hub/osc/bundles/... support?

I tried building on Windows 10 and received

CMake Error at CMakeLists.txt:60 (add_executable):
  Cannot find source file:

    test/longtest.c

(This is as of commit da07902.)

on non-macOS, o2 is unconditionally compiled with the -mcx16 flag, as can be seen in

however, this flag is only available for x86 CPUs (32bit, 64bit), and not for other archs.

I discovered this when compiling for s390x (which is admittedly an odd target architecture for o2), but the problem persists with common architectures like armhf (as found e.g. in the RaspberryPi) and arm64 (RPi4,...)

The linux port should be very similar to OS X except for some include files and small library differences.

file says python3 but seems to be python 2.7 -- probably it should be compatible with both or there should be a comment in the source explaining which python version to use and what was the problem making it run in the other version

Error recovery is spotty. Need to systematically review every function that can fail and make sure the failure is detected and propagated back to the caller.

o2 may need some work to recover and clean up.

o2 may need some work to disallow function calls if initialization fails and user ignores the failure and proceeds to assume that o2 functions are callable.

Thanks for the 2.0 update!

I was playing around with adding O2 to Kronos as a default input module in the hopes that it could replace OSC with the forwarding functionality. However, seems like the conversion of OSC payloads to O2 is broken at the moment. I found the following problems in osc_to_o2:

• O2 pads strings by 5-8 zeros (o2_next_o2string will look for a quadruplet of characters that ends with null, then skips 4 more.
• Osc pads strings by 1-4 zeros (address pattern and type string are scanned until a quadruplet is found that ends in null)
• osc_to_o2 uses the base pointer of the OSC message to compute alignment; on my Mac, the underlying packet from the network is often not even 32-bit aligned! So it is the length of the strings that should be aligned to 4, not the pointer to the end of the string inside the (misaligned) Osc packet.
• For these reasons, it is not straightforward to compute o2 message length from Osc message length (you have to analyze and realign the address pattern and type string), and the payload from the Osc message is often missing or corrupted.
• It has probably no practical effect, but osc_to_o2 applies endian-conversion to all fields of the new O2 message, including the data.misc = O2_TCP_FLAG that was just set in native alignment.

Sorry, no pull request at this time - I gave up at the string alignment/length mismatches.

infotest2 is not documented in test/README.txt
infotest2 is not run by regression_tests.py
infotest2 is not run by regression_tests.sh

So far, we've only run on OS X.
In particular, sockets for Windows differ from both linux and OS X. Note that in windows, sockets are 32-bit integers that can be large. Not the same as the small integers used by Unix (OS X and Linux). I think on OS X and Linux, we'll use poll() and on Windows, we'll use select().

running ./arraytest compiled for the x86 architecture (aka x86_32, or i386,... 32bit intel), but also on armhf (32bit arm, as found in the RaspberryPi) aborts with an assertion:

 ./arraytest 
arraytest: .../o2/src/o2mem.cpp:428: void o2_mem_init(char*, int64_t): Assertion `sizeof(O2queue) == 16' failed.
Aborted

on these architectures sizeof(O2queue)==8
I have no patch for this yet.

also, this is just the beginning of assertion failures on these architectures.

There is a command, o2_delegate_to_osc() to redirect O2 messages to OSC clients. This needs to be implemented fully and tested.
I think we should handle timestamps by delaying the message locally until the timestamp time, then send the message without timestamps. (It would be better to translate timestamp to an OSC timestamp, form a bundle, and send a timestamped message immediately, but I do not think it is common for OSC systems to use timestamps; therefore, timestamped bundles would probably not be received properly.)

it seems that the CMakeFile sets the build flags

would it be possible to use something like this instead:

set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=c11 -mcx16")

?

o2_create_osc_port() is not fully implemented or tested