matrix-org / waterfall

A cascading stream forwarding unit for scalable, distributed voice and video conferencing over Matrix
Apache License 2.0
97 stars 5 forks source link

Use Go in a more idiomatic way #126

Closed daniel-abramov closed 1 year ago

daniel-abramov commented 1 year ago

This is a result of working on https://github.com/matrix-org/waterfall/issues/120 which did not quite work as expected due to some bugs and issues with Pion.

So currently it's more a preparation to properly implement a https://github.com/matrix-org/waterfall/issues/120 and to get ready for https://github.com/matrix-org/waterfall/issues/117

This PR changes the following things:

Note that there is one significant change in the behavior: currently, the router process the messages from the Matrix SDk and sends them to the conference via an unbounded channel. This means that if one conference freezes, it will affect all other conferences! It's not a problem for now since we only primarily use a single conference and the conferences should never freeze (so it helps to check if our implementation is sound), but ideally we probably need to get the buffer back for the conference, otherwise we won't be able to make them all independent.

daniel-abramov commented 1 year ago

I think this looks good, but this is a huge PR with a lot of unrelated changes, making it very hard to review. Moving things out of common, in particular, feels like it doesn't belong here.

Yeah, sorry, perhaps the common part should have been moved into its own PR.

I tried to separate changes by commits, so it's easier to review the PR by checking each individual commit as one commit did one single thing (I tried to rebase them before creating a PR so that it's easier to follow the logic and the changes). I.e. if you review it by commit, I think it should be more manageable as they are mostly small.