-
Notifications
You must be signed in to change notification settings - Fork 218
feat: add an OscOutput operator and its example #404
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an excellent PR! Most of my comments are nitpicky about code-style and conventions. But there are three things that should be done:
- The Op should implement the Dispose-Pattern and probably disconnect when disposing. (If not, it might happen that the op keeps an connection open).
- Another problem might be that too many open connections will lead to performance issues. I'm not sure if this is true but OscInput uses a shared OscConnectionManager to pool and reusing connections. Although this optimization might be quite involved. I think we can live without that here.
- Please implement
IStatusProvider
. This will show a warning icon whenever the op has a problem (like a missing connection or an invalid IP address). Check [AbletonLiveSync] for an example.
(disregard latest commit, involving very experimental/wip/poc regarding text support for OscInput, i wanna keep this change focused on OscOutput), switching it to draft, as i might be noisy in between) |
4f0e76c
to
7ec3579
Compare
Thanks ! :)
done.
The conManager is built with a server purpose in mind, and won't be much use for us here. I'm not sure a threaded model would help us in this context, as there's no error check on the We can check a later stage, for a threaded model, if we get performance issue, but i think we're fine here. On another, perf-related note, the operator gives a tiny glitch once in while, but seems to be related to the refresh rate of Tooll (and phase with touchosc frame rate (where you can't disable vsync) : it completely disappear when I disable vsync (in tooll) and it stays well under 1ms frame-time). This is a general issue with OSC client (TouchOSC does that too (this is why many standalone sequencer/drumbox were attempted and failed (15ms is barely noticeable for eye, but sound like a clumsy drummer for a sequencer)
Done! (edit: quotes readability, bis..) |
Awesome! |
This is a new operator, counterpart to OscInput
It only supports strings and float numbers, it tries to fail gracefully on bad location and failure to connect.
Error and state check is grunty, please advise if I'm doing it too wrong.