-
Notifications
You must be signed in to change notification settings - Fork 29
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
Allow requesting a ringtone during SIP transfers #207
Conversation
pkg/sip/inbound.go
Outdated
w := c.lkRoom.SwapOutput(nil) | ||
|
||
defer func() { | ||
if err != nil && !c.done.Load() { |
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.
Ideally this should check an actual return var of the function. I usually declare it with a different name to make sure it's the right one:
func (c *inboundCall) transferCall(...) (gerr error) {
// ...
defer func() {
if gerr != nil {
// ...
}
}()
defer rcancel() | ||
|
||
// mute the room audio to the SIP participant | ||
w := c.lkRoom.SwapOutput(nil) |
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.
Since w
may contain C resampler, we need to make sure it's either set back via SwapOutput
, or closed if it's no longer needed.
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.
Sorry for confusing, but this still is not closed. The c.media.GetAudioWriter()
is fine, it will be closed when c.media
is closed. The problem is with w
- you detach it from the room, so it won't be closed if the transfer succeeds. It should probably be in the defer
below - it either swaps w
back to the room, or should close it.
No description provided.