Skip to content

Conversation

@telephon
Copy link
Collaborator

@telephon telephon commented Nov 27, 2024

This is a first draft. It works fine, for one channel.

I wonder if:

  • we should separate audio rate and control rate again. The code would be clearer, but
    -- I can't think of good names.
    -- the room names are global between KR and AR, right? So if there exists an ar-room, I can't make a kr-room of the same name.
  • we should implement multichannel expansion by generating multiple rooms automatically (\roomname_0, \roomname_1 ...)
  • the grace time is correct. I added it because when we close and reopen a room too quickly, or hit cmd-period and start again too quickly, the server may refuse the connection.

@capital-G
Copy link
Owner

Thanks for this, eager to try it out! Some first comments regarding the questions

  • the room names are global between KR and AR, right? So if there exists an ar-room, I can't make a kr-room of the same name.

They are not exclusive as both types use a separate dictionary on the server.
There is actually a third kind of room called chat room - it is unclear what to do with it because each audio or control room also has an implicit channel to transfer string data, but maybe it would be nice to chat w/o sharing audio/control rate?

  • we should implement multichannel expansion by generating multiple rooms automatically (\roomname_0, \roomname_1 ...)

It is possible to put multiple streams into one WebRTC connection - currently we try to keep things simple.

How to handle multi-channel signals is somehow complex as we also wouldn't like to spill the web frontend with multi-channel rooms.

@telephon
Copy link
Collaborator Author

Ah good, then I'll split them up again,

Naming, naming:

  • AudioStecker and ControlStecker
  • Stecker and LFStecker
  • AudioRoom and ControlRoom (and ChatRoom … )

I think the room metaphor is the best, but what do you think?

@telephon
Copy link
Collaborator Author

The examples are in comments (which now don't get "autoformatted" anymore!!. I see that I want the IDE to do region selection also in comments …)

@capital-G
Copy link
Owner

capital-G commented Dec 18, 2024

Ah sorry, this hasn't been merged yet - let's merge and test it.
For a public release I'd like to change some variable names and make them more more verbose (e.g. res is common jitlib talk but I'd appreciate if it actually reflects the actual name of the resource), but this seems pre-mature optimization :)

@telephon can you apply the formatting (simply run pre-commit install and pre-commit run --all-files and commit the changes)? Then we can merge this. (see https://github.com/capital-G/stecker/actions/runs/12055587737/job/33616251564?pr=30#step:3:75)

Copy link
Owner

@capital-G capital-G left a comment

Choose a reason for hiding this comment

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

Ah, still had an un-saved review.

var <bus, <synth;
var <>graceTime = 3, <fresh = true;

classvar <>all;
Copy link
Owner

Choose a reason for hiding this comment

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

Should there be a setter for this?

Also this combines Data and Audio rooms, maybe use?

classvar <audioBusses;
classvar <controlBusses;

Copy link
Collaborator Author

@telephon telephon Dec 18, 2024

Choose a reason for hiding this comment

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

Actually, I thought that it may be better to separate the two again.
I just can't think of good names!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe: Stecker and DataStecker?

^res
}

*ar { |roomName, numChannels|
Copy link
Owner

Choose a reason for hiding this comment

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

Should numChannels default to 1?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for audio, normally you would expect 2.
But currently we only support 1, we need to be a bit careful. It woun't be complicated to implement multichannel via (internally) modified room names (like \foo_2)

}

*new { |roomName, input|
var res = all[roomName];
Copy link
Owner

Choose a reason for hiding this comment

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

I know res is proxy-speak, but I'd found steckerBus a bit more clear in this case.

@telephon
Copy link
Collaborator Author

OK, did a refactoring …

@telephon
Copy link
Collaborator Author

telephon commented Mar 7, 2025

@capital-G is there anything needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants