Using Reopen() to flush built-up readBuffer #576
Conversation
| // Creates a new underlying UDP connection on the same port. | ||
| // The practical use of this function is to discard the assicuated OS-level read buffer of the old socket. | ||
| func (c *udpConn) Reopen() error { | ||
| lc := &net.ListenConfig{ |
There was a problem hiding this comment.
It is possible to enable SO_REUSEADDR here, instead of on creation, but would require an entire rework of these interface chains. Avoiding that for now.
| lc := &net.ListenConfig{ | ||
| Control: func(network, address string, c syscall.RawConn) error { | ||
| return c.Control(func(fd uintptr) { | ||
| syscall.SetsockoptInt(int(fd), syscall.SOL_SOCKET, syscall.SO_REUSEADDR, 1) |
There was a problem hiding this comment.
Ideally, anything that has to do with syscall should be in a separate file, under a build flag. This also implies that there should be a file that provides a fallback if these syscalls are not available.
So maybe we could solve it a bit more naively to avoid these portability issues? For example, we could just run the read loop on creation, but do not pass packets down until the port is explicitly enabled.
There was a problem hiding this comment.
we could just run the read loop on creation
That would have been ideal, and was my first go-to approach. But sure to how things are structured right now, we can one of the following:
- This PR or a variation (no reopen, just straight up close and immediately open on the same port).
- Start a "custom" rtp loop - just read & discard function until told to stop. Not reusing the
Sessiontype. - Start a dummy
Sessiontype, have it run the normal, then, when we receive the answer SDP (and thus have*MediaConf), we can determine whether itssrtp.NewSession(p.log, p.port, c.Crypto)orrtp.NewSession(p.log, p.port)and drop the old session. - Ideally, I'd like to be able to start the normal session without knowing the selected codecs/crypto just yet, but that requires quite a lot of changes that I'm not sure need to be taken on today.
All these aren't great to some extent. I think option 2 is easy enough as well, but I'd like to avoid 3.
Do you have any specific preferences on this?
There was a problem hiding this comment.
From initial PR, timeline:
This PR is here to address that issue.