Skip to content
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

Make sfu-to-sfu talk MSC3401 and add new features #11

Merged
merged 126 commits into from
Aug 17, 2022
Merged

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented Jun 3, 2022

Fixes #13
Implements MSC3401 SFU bits
Requires matrix-org/matrix-js-sdk#2423
Requires mautrix/go#77

Comment on lines +35 to +38
type LocalTrackWithInfo struct {
Track *webrtc.TrackLocalStaticRTP
Info LocalTrackInfo
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This now looks like an idea much worse than when I first thought of it... This could definitely be done differently now though I think it makes sense to just try to redo this during switching to using a pool of streams since that might change the design again...

@SimonBrandner SimonBrandner changed the title Make sfu-to-sfu talk MSC3401 Make sfu-to-sfu talk MSC3401 and add new features Aug 14, 2022
@SimonBrandner SimonBrandner requested a review from a team August 14, 2022 13:04
@SimonBrandner SimonBrandner marked this pull request as ready for review August 14, 2022 13:04
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Generally looking good. Main things are proper (or at least some) error handling rather than panicking and making sure member functions aren't public unnecessarily. We'll also want to hook up logrus or whatever people use now at some point soon, but probably not in this PR.

scripts/build.sh Show resolved Hide resolved
scripts/run.sh Outdated Show resolved Hide resolved
src/call.go Outdated Show resolved Hide resolved
src/call.go Outdated Show resolved Hide resolved
src/call.go Outdated Show resolved Hide resolved
src/call.go Outdated Show resolved Hide resolved
src/conference.go Show resolved Hide resolved
src/focus.go Show resolved Hide resolved
src/main.go Outdated Show resolved Hide resolved
src/matrix.go Outdated Show resolved Hide resolved
Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
@SimonBrandner
Copy link
Contributor

We'll also want to hook up logrus or whatever people use now at some point soon, but probably not in this PR.

Yep, that has been my thoughts since the fist time I actually need to debug it based on logs :P

Sorry, for the Go newbie mistakes, hopefully, it looks better now

src/call.go Outdated Show resolved Hide resolved
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Looks good other than the ice password. Presumably we're blocked on the mautrix-go PR now?

@SimonBrandner
Copy link
Contributor

Looks good other than the ice password.

Thank you!

Presumably we're blocked on the mautrix-go PR now?

Since I expect some of the events to change shape with further development, I'd hold off and wait and get that merged when we're more sure things are not going to change radically

Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
@SimonBrandner SimonBrandner merged commit b092330 into master Aug 17, 2022
@SimonBrandner SimonBrandner deleted the matthew/matrix branch August 17, 2022 14:29
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.

Basic SFU features
3 participants