AlloSphere-Research-Group / allolib

Library for interactive multimedia application development
BSD 3-Clause "New" or "Revised" License
36 stars 14 forks source link

Issue with oscpack #50

Open hannahwolfe opened 2 years ago

hannahwolfe commented 2 years ago

Trying to install and run allolib. Examples that do not require graphics work fine (eg: example_math_random) but examples that require Parameter OSC Handshake server like all the graphics examples do not. This issue has been produced on two of the new macs with M1 chips, but could not be reproduced with a mac with an i9 processor.

Assertion failed: (sizeof(osc::int32) == 4), function OutboundPacketStream, file /[path]/C++/allolib/external/oscpack/osc/OscOutboundPacketStream.cpp, line 166. Abort trap: 6

Heres the error message generated by the macOS system: Process: example_graphics_cieColor [75498] Path: /Users/USER/Desktop/*/example_graphics_cieColor Identifier: example_graphics_cieColor Version: 0 Code Type: ARM-64 (Native) Parent Process: bash [75445] Responsible: Terminal [75442] User ID: 502

Date/Time: 2022-05-30 13:53:19.048 -0400 OS Version: macOS 11.3.1 (20E241) Report Version: 12

mantaraya36 commented 2 years ago

@MyunginLee Have you seen this issue when compiling for M1s?

Since it is an assertion, I am guessing that release builds don't catch this issue, as we have built on M1 devices. The issue is only for ints, so we have not seen issues as their use is less common.

I am guessing the source of the problem is here external/oscpack/osc/OscTypes.h:64 :

#if defined(__x86_64__) || defined(_M_X64)

typedef signed int int32;
typedef unsigned int uint32;

#else

typedef signed long int32;
typedef unsigned long uint32;

#endif

Can you try changing:

typedef signed long int32;
typedef unsigned long uint32;

to

#include <ctypes>
typedef int32_t int32;
typedef uint32_t uint32;
mantaraya36 commented 2 years ago

If the M1 for some reason passes this test:

#if defined(__x86_64__) || defined(_M_X64)

can you try changing the contents of that branch?

MyunginLee commented 2 years ago

Hi, I didn't specifically experienced that issue maybe because I have Rosetta 2 installed. So codes using Parameter OSC Handshake server works fine for me. I noticed my system goes to else instead of:

#if defined(__x86_64__) || defined(_M_X64)

and

typedef int32_t int32;
typedef uint32_t uint32;

didn't work since int32_t is undefined.

Including this didn't work:

#include <ctypes>

so I tried:

#include <ctype.h>

and it showed:

In file included from /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.cpp:37:
/Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.h:73:9: error: unknown type name 'int32_t'; did you mean '__int32_t'?
typedef int32_t int32;
        ^~~~~~~
        __int32_t
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/arm/_types.h:20:33: note: '__int32_t' declared here
typedef int                     __int32_t;
                                ^
In file included from /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.cpp:37:
/Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.h:74:9: error: unknown type name 'uint32_t'
typedef uint32_t uint32;
        ^
2 errors generated.
mantaraya36 commented 2 years ago

Ah, yes, perhaps oscpack is note being compiled with C++11 support, so perhaps

include

will work.

On Mon, Jun 6, 2022 at 2:56 PM Myungin Lee @.***> wrote:

Hi, I didn't specifically experienced that issue maybe because I have Rosetta 2 installed. So I noticed my system goes to else instead of:

if defined(__x86_64__) || defined(_M_X64)

and

typedef int32_t int32; typedef uint32_t uint32;

didn't work since int32_t is undefined.

Including this didn't work:

include

so I tried:

include

and it showed:

In file included from /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.cpp:37: /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.h:73:9: error: unknown type name 'int32_t'; did you mean 'int32_t'? typedef int32_t int32; ^~~ int32_t /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/arm/_types.h:20:33: note: '__int32_t' declared here typedef int __int32_t; ^ In file included from /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.cpp:37: /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.h:74:9: error: unknown type name 'uint32_t' typedef uint32_t uint32; ^ 2 errors generated.

— Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/50#issuecomment-1147973044, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADK3R2EOQZEXB27S5S7VKLVNZXYXANCNFSM5X7OZWBQ . You are receiving this because you commented.Message ID: @.***>

younkhg commented 2 years ago
        how about <cstdint>? (or stdint.h)---- On Mon, 06 Jun 2022 14:56:28 -0700  Myungin ***@***.***> wrote ---- 

Hi, I didn't specifically experienced that issue maybe because I have Rosetta 2 installed. So I noticed my system goes to else instead of:

if defined(__x86_64__) || defined(_M_X64)

and typedef int32_t int32; typedef uint32_t uint32;

didn't work since int32_t is undefined. Including this didn't work:

include

so I tried:

include

and it showed: In file included from /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.cpp:37: /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.h:73:9: error: unknown type name 'int32_t'; did you mean 'int32_t'? typedef int32_t int32; ^~~ int32_t /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/arm/_types.h:20:33: note: '__int32_t' declared here typedef int __int32_t; ^ In file included from /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.cpp:37: /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.h:74:9: error: unknown type name 'uint32_t' typedef uint32_t uint32; ^ 2 errors generated.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

mantaraya36 commented 2 years ago

Yes, that's the correct one. Thanks!

On Mon, Jun 6, 2022 at 2:59 PM Keehong Youn @.***> wrote:

how about ? (or stdint.h)---- On Mon, 06 Jun 2022 14:56:28 -0700 Myungin @.***> wrote ---- Hi, I didn't specifically experienced that issue maybe because I have Rosetta 2 installed. So I noticed my system goes to else instead of:

if defined(__x86_64__) || defined(_M_X64)

and typedef int32_t int32; typedef uint32_t uint32;

didn't work since int32_t is undefined. Including this didn't work:

include

so I tried:

include

and it showed: In file included from /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.cpp:37:

/Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.h:73:9: error: unknown type name 'int32_t'; did you mean 'int32_t'? typedef int32_t int32; ^~~ int32_t /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/arm/_types.h:20:33: note: '__int32_t' declared here typedef int __int32_t; ^ In file included from /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.cpp:37:

/Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscTypes.h:74:9: error: unknown type name 'uint32_t' typedef uint32_t uint32; ^ 2 errors generated.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/50#issuecomment-1147975364, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADK3R234BAAEJFROMZJ65LVNZYEXANCNFSM5X7OZWBQ . You are receiving this because you commented.Message ID: @.***>

MyunginLee commented 2 years ago

Including<cstdint> or <stdint.h> gives:

In file included from /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscReceivedElements.cpp:37:
/Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscReceivedElements.h:104:5: error: constructor cannot be redeclared
    ReceivedPacket( const char *contents, int size )
    ^
/Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscReceivedElements.h:95:5: note: previous definition is here
    ReceivedPacket( const char *contents, osc_bundle_element_size_t size )
    ^
1 error generated.
make[3]: *** [../../../../../build/Release/external/CMakeFiles/oscpack.dir/oscpack/osc/OscReceivedElements.cpp.o] Error 1
make[3]: *** Waiting for unfinished jobs....
In file included from /Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscOutboundPacketStream.cpp:37:
/Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscOutboundPacketStream.h:109:27: error: class member cannot be redeclared
    OutboundPacketStream& operator<<( int rhs )
                          ^
/Users/ben/Desktop/projects/allolib_playground/allolib/external/oscpack/osc/OscOutboundPacketStream.h:106:27: note: previous declaration is here
    OutboundPacketStream& operator<<( int32 rhs );
                          ^
1 error generated.
younkhg commented 2 years ago

int == int32_t, one of the overloads should be disabled. It's our local copy so it should be fine. Maybe pick one that would be more frequently used?

mantaraya36 commented 2 years ago

Something else to consider could be moving away from oscpack? I have wanted to set up the OSC dependency as something that does parsing only on top of raw cross platform sockets? (something that bothers me is that the buffer size for messages in oscpack is pretty small... - we could change that too in our local copy)

Perhaps something like: https://github.com/mhroth/tinyosc or: https://github.com/ssilverman/LiteOSCParser

On Mon, Jun 6, 2022 at 3:17 PM Keehong Youn @.***> wrote:

int == int32_t, one of the overloads should be disabled. It's our local copy so it should be fine. Maybe pick one that would be more frequently used?

— Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/50#issuecomment-1147989363, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADK3RZ5KJFK62NMWCOI6ULVNZ2HZANCNFSM5X7OZWBQ . You are receiving this because you commented.Message ID: @.***>

younkhg commented 2 years ago

Yes, using al_Socket for networking and using smaller one to only serialize/deserialize would be a good option.

kybr commented 2 years ago

Yes..

http://gruntthepeon.free.fr/oscpkt/

But, but lot of examples would change.

On Mon, Jun 6, 2022, 18:37 Keehong Youn @.***> wrote:

Yes, using al_Socket for networking and using smaller one to only serialize/deserialize would be a good option.

— Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/50#issuecomment-1148100185, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMILJSX6EFJHCU25JTFMBDVN2RVXANCNFSM5X7OZWBQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

mantaraya36 commented 2 years ago

Perhaps as we are wrapping the OSC library in al_Osc, it should be possible to retain the API that uses the streaming operator.

On Mon, Jun 6, 2022 at 9:07 PM karl yerkes @.***> wrote:

Yes..

http://gruntthepeon.free.fr/oscpkt/

But, but lot of examples would change.

On Mon, Jun 6, 2022, 18:37 Keehong Youn @.***> wrote:

Yes, using al_Socket for networking and using smaller one to only serialize/deserialize would be a good option.

— Reply to this email directly, view it on GitHub < https://github.com/AlloSphere-Research-Group/allolib/issues/50#issuecomment-1148100185 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAMILJSX6EFJHCU25JTFMBDVN2RVXANCNFSM5X7OZWBQ

. You are receiving this because you are subscribed to this thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/50#issuecomment-1148171598, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADK3R5SFSWVRC57MO3DWSLVN3DJVANCNFSM5X7OZWBQ . You are receiving this because you commented.Message ID: @.***>

mantaraya36 commented 2 years ago

oscpkt looks great. Perhaps the best of the three (at least on paper).

On Mon, Jun 6, 2022 at 9:07 PM karl yerkes @.***> wrote:

Yes..

http://gruntthepeon.free.fr/oscpkt/

But, but lot of examples would change.

On Mon, Jun 6, 2022, 18:37 Keehong Youn @.***> wrote:

Yes, using al_Socket for networking and using smaller one to only serialize/deserialize would be a good option.

— Reply to this email directly, view it on GitHub < https://github.com/AlloSphere-Research-Group/allolib/issues/50#issuecomment-1148100185 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAMILJSX6EFJHCU25JTFMBDVN2RVXANCNFSM5X7OZWBQ

. You are receiving this because you are subscribed to this thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/50#issuecomment-1148171598, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADK3R5SFSWVRC57MO3DWSLVN3DJVANCNFSM5X7OZWBQ . You are receiving this because you commented.Message ID: @.***>

kybr commented 2 years ago

Graham turned me on to it years ago. I use it frequently, but I have not stress-tested it.

Retain the stream operator? Noooooooo... ;)

On Mon, Jun 6, 2022, 22:10 Andres Cabrera @.***> wrote:

oscpkt looks great. Perhaps the best of the three (at least on paper).

On Mon, Jun 6, 2022 at 9:07 PM karl yerkes @.***> wrote:

Yes..

http://gruntthepeon.free.fr/oscpkt/

But, but lot of examples would change.

On Mon, Jun 6, 2022, 18:37 Keehong Youn @.***> wrote:

Yes, using al_Socket for networking and using smaller one to only serialize/deserialize would be a good option.

— Reply to this email directly, view it on GitHub <

https://github.com/AlloSphere-Research-Group/allolib/issues/50#issuecomment-1148100185

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AAMILJSX6EFJHCU25JTFMBDVN2RVXANCNFSM5X7OZWBQ

. You are receiving this because you are subscribed to this thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub < https://github.com/AlloSphere-Research-Group/allolib/issues/50#issuecomment-1148171598 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AADK3R5SFSWVRC57MO3DWSLVN3DJVANCNFSM5X7OZWBQ

. You are receiving this because you commented.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/50#issuecomment-1148203517, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMILJQH7Z4EFULUE24Z6B3VN3KTRANCNFSM5X7OZWBQ . You are receiving this because you commented.Message ID: @.***>

hannahwolfe commented 2 years ago

Thank you for the information, this directed us to a temporary solution, modifying the oscpack library.

So it looks like the below conditional needs to return true on their machines: (defined(__x86_64__) || defined(_M_X64))

This meant making modifications to the conditionals in the following files oscType.h line 64 oscReceiveElements.h line 103 oscOutBoundPacketStream.h line 108