-
Notifications
You must be signed in to change notification settings - Fork 49
[RSDK-8293] Pass in Connection to Answerer in WebRTC Server #410
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
Changes from 7 commits
f22bb6d
3a4a9d0
503332b
e0818f7
933dcaf
33b1814
f2cdf28
c979ad2
771c707
8e637a5
80539b3
7813ed2
dc8946d
cd80af8
8a03b70
2e66377
321abff
e55a28f
19572b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -64,6 +64,9 @@ type dialOptions struct { | |||||
| // interceptors | ||||||
| unaryInterceptor grpc.UnaryClientInterceptor | ||||||
| streamInterceptor grpc.StreamClientInterceptor | ||||||
|
|
||||||
| // conn can be used to force the webrtcSignalingAnswerer to use a preexisting connection instead of dialing and managing its own. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| externalSignalerConn ClientConn | ||||||
| } | ||||||
|
|
||||||
| // DialMulticastDNSOptions dictate any special settings to apply while dialing via mDNS. | ||||||
|
|
@@ -280,3 +283,10 @@ func WithForceDirectGRPC() DialOption { | |||||
| o.disableDirect = false | ||||||
| }) | ||||||
| } | ||||||
|
|
||||||
| // WithConn provides a preexisting connection to use. This option forces the webrtcSignalingAnswerer to not dial or manage a connection. | ||||||
| func WithConn(externalSignalerConn ClientConn) DialOption { | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rename |
||||||
| return newFuncDialOption(func(o *dialOptions) { | ||||||
| o.externalSignalerConn = externalSignalerConn | ||||||
| }) | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -40,8 +40,9 @@ type webrtcSignalingAnswerer struct { | |||||||||||||||
| // conn is used to share the direct gRPC connection used by the answerer workers. As direct gRPC connections | ||||||||||||||||
| // reconnect on their own, custom reconnect logic is not needed. However, keepalives are necessary for the connection | ||||||||||||||||
| // to realize it's been disconnected quickly and start reconnecting. | ||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. update comment here |
||||||||||||||||
| connMu sync.Mutex | ||||||||||||||||
| conn ClientConn | ||||||||||||||||
| connMu sync.Mutex | ||||||||||||||||
| conn ClientConn | ||||||||||||||||
| sharedConn bool | ||||||||||||||||
|
|
||||||||||||||||
| logger utils.ZapCompatibleLogger | ||||||||||||||||
| } | ||||||||||||||||
|
|
@@ -61,8 +62,12 @@ func newWebRTCSignalingAnswerer( | |||||||||||||||
| dialOptsCopy := make([]DialOption, len(dialOpts)) | ||||||||||||||||
| copy(dialOptsCopy, dialOpts) | ||||||||||||||||
| dialOptsCopy = append(dialOptsCopy, WithWebRTCOptions(DialWebRTCOptions{Disable: true})) | ||||||||||||||||
| options := &dialOptions{} | ||||||||||||||||
| for _, opt := range dialOptsCopy { | ||||||||||||||||
| opt.apply(options) | ||||||||||||||||
| } | ||||||||||||||||
| bgWorkers := utils.NewBackgroundStoppableWorkers() | ||||||||||||||||
| return &webrtcSignalingAnswerer{ | ||||||||||||||||
| ans := &webrtcSignalingAnswerer{ | ||||||||||||||||
| address: address, | ||||||||||||||||
| hosts: hosts, | ||||||||||||||||
| server: server, | ||||||||||||||||
|
|
@@ -71,6 +76,11 @@ func newWebRTCSignalingAnswerer( | |||||||||||||||
| bgWorkers: bgWorkers, | ||||||||||||||||
| logger: logger, | ||||||||||||||||
| } | ||||||||||||||||
| if options.conn != nil { | ||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the internal signaler answer shouldn't be using the same connection - it is instantiated here Lines 659 to 665 in cf6c56e
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't require any change does it, since its instantiation has its own fresh set of options passed? |
||||||||||||||||
| ans.conn = options.conn | ||||||||||||||||
| ans.sharedConn = true | ||||||||||||||||
| } | ||||||||||||||||
| return ans | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| const ( | ||||||||||||||||
|
|
@@ -260,9 +270,11 @@ func (ans *webrtcSignalingAnswerer) Stop() { | |||||||||||||||
| ans.connMu.Lock() | ||||||||||||||||
| defer ans.connMu.Unlock() | ||||||||||||||||
| if ans.conn != nil { | ||||||||||||||||
| err := ans.conn.Close() | ||||||||||||||||
| if isNetworkError(err) { | ||||||||||||||||
| ans.logger.Errorw("error closing signaling connection", "error", err) | ||||||||||||||||
| if !ans.sharedConn { | ||||||||||||||||
| err := ans.conn.Close() | ||||||||||||||||
| if isNetworkError(err) { | ||||||||||||||||
| ans.logger.Errorw("error closing signaling connection", "error", err) | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| ans.conn = nil | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.