Fix bind to local interface when sending to udp ep #3122
Conversation
|
Do these changes also need to be made in |
|
Yes, please. BTW I just checked this on my windows build and seems like the problem has been fixed. Note one more thing: if this distinction for the platform is not necessary in the constructor of UdpCommon (the removed |
| interface_addr = sockaddr_any(AF_INET); | ||
| interface_addr.sin.sin_family = AF_INET; | ||
| interface_addr.sin.sin_addr.s_addr = htonl(INADDR_ANY); | ||
| interface_addr.sin.sin_port = htons(port); |
There was a problem hiding this comment.
You can use the API of sockaddr_any directly, or better use CreateAddr as you already have it.
| interface_addr = sockaddr_any(AF_INET); | |
| interface_addr.sin.sin_family = AF_INET; | |
| interface_addr.sin.sin_addr.s_addr = htonl(INADDR_ANY); | |
| interface_addr.sin.sin_port = htons(port); | |
| interface_addr = CreateAddr("", port, AF_INET); |
| // Also, sets port sharing when working with multicast | ||
| sadr = maddr; | ||
| int reuse = 1; | ||
| int shareAddrRes = setsockopt(m_sock, SOL_SOCKET, SO_REUSEADDR, reinterpret_cast<const char*>(&reuse), sizeof(reuse)); |
There was a problem hiding this comment.
Check if this is still necessary to be set before setting the multicast option, or can be moved to the place directly before binding so that the Windows-specific code is in one place.
There was a problem hiding this comment.
Ok, confirmed here: setting SO_REUSEADDR is not necessary because the same option is set the same way in the same function few lines earlier, this time for any platform, so this part setting the option can be removed. So please, as you are changing things here, remove this all, and then place the Verb() calls in appropriate conditionals before the ::bind() call.
| int stat = | ||
| #if defined(_WIN32) || defined(__CYGWIN__) | ||
| is_multicast ? ::bind(m_sock, interface_addr.get(), interface_addr.size()) : | ||
| #endif | ||
| ::bind(m_sock, target_addr.get(), target_addr.size()); |
There was a problem hiding this comment.
It is better without cpp entanglement.
| int stat = | |
| #if defined(_WIN32) || defined(__CYGWIN__) | |
| is_multicast ? ::bind(m_sock, interface_addr.get(), interface_addr.size()) : | |
| #endif | |
| ::bind(m_sock, target_addr.get(), target_addr.size()); | |
| #if defined(_WIN32) || defined(__CYGWIN__) | |
| sockaddr_any bindaddr = is_multicast ? interface_addr : target_addr; | |
| #else | |
| sockaddr_any bindaddr = target_addr; | |
| #endif | |
| ::bind(m_sock, bindaddr.get(), bindaddr.size()); |
|
|
||
| Setup(host, port, attr); | ||
| if (adapter != "") | ||
| if (is_multicast && interface_addr.isany() == false) |
There was a problem hiding this comment.
I think == false makes too much noise.
| if (is_multicast && interface_addr.isany() == false) | |
| if (is_multicast && !interface_addr.isany()) |
ethouris
added
Type: Bug
|
Replaced by #3126; same changes with modifications have been merged. |
2 participants
Fixes #3121
Tested in 1.5.4 release, but it doesn't look like UDP has changed since then.
I've tested Windows output (srt-live-transmit.exe srt://XXX.XXX.XXX.XXX udp://239.XXX.XXX.XXX) but not UDP input or on Linux
E: There's a bunch of renaming that might not conform to the repo's style, but the most important change is the remove of line 914, and the new binding logic used in line 1003 and 1058