-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[0.x], [1.x] VideoRoom RTP Forwarder Doesn't Work When IPv6 is Disabled #2915
Comments
Already discussed at length here: #2051
Exactly, you don't know in advance what will be needed, and we currently use a single socket for all forwards coming from the same publisher, which is why we need a socket that can send to anything. The alternative would be creating a different socket for each forwarded stream, which is something I personally don't like. |
What about creating "just" two sockets, one for IPv4 and one for IPv6 and one or the other according to the destination? Edit: I just saw the previously suggested PR. Is a two-socket solution still reasonable? It's a bit more cumbersome that a single socket but better than one for each forwarder... |
I forgot the discussion there, and the potential action item on a conditional check of |
Even though that wouldn't fix the MacOS problem, where the socket is created without problems, but it's sending to an IPv4 address from an IPv6 socket that fails. But anyway, that's a different issue than yours, which I can think can be solved more simply with the check at startup to know the constraints. |
@shmuelzon please test the PR above and let me know if it fixes the problem for you. |
@shmuelzon ping 🙂 |
Hey, I'm really sorry for not replying, I was shifted to a different urgent matter. I've been using pre-built packages up until now so I need to try to compile this one myself. Are there any major difference (external APIs and/or configuration files) between 1.x and 0.x or only internal implementation? If so, I'll need to apply your changes on 0.x to test in my environment but I don't think much has changed there. Thanks for your help! |
Most APIs were kept for backwards compatibility, so it should be safe to test. The VideoRoom in particular can still be accessed via legacy APIs. |
It is worth mentioning that grub configuration must be regenerated after this change. In Ubuntu you can use the helper
If the helper is not available or you are using other distros the command is
(double check |
@shmuelzon any update? |
Hey, Once again, I'm really sorry for the delay here but I'm happy to say I finally got the chance to test this.
And RTP forwarding for video rooms also works. I'd just note that I only tested the video room plugin on 0.10.5 and needed to backport your patch. There were some conflicts (nothing major) so I'm attaching here the patch I used: Thanks again for all you help! |
Ack, thanks for the feedback, I'll merge then, and after that I'll backport the same fix to |
The above commit added the same feature to |
Will do, thanks again! |
Hey,
What version of Janus is this happening on?
I've seen it on v0.10.5 but I believe is relevant to v1.0.0 as well
Have you tested a more recent version of Janus too?
No, mainly due to other dependencies in the system.
Is there a gdb or libasan trace of the issue?
No trace, as there's no crash. Just an error message in the logs:
Additional context
Looking at the sources, the socket created for forwarding RTP packets is always created as IPv6 and then marked for IPv4 as well. On systems where IPv6 is disabled, e.g., when adding
ipv6.disable=1
to the kernel command line, the call tosocket(AF_INET6, SOCK_DGRAM, IPPROTO_UDP);
fails. Although thertp_forward
command does allow to specify the host family, it seems that this is only used for DNS resolving.I'm not sure what would be the best way to over come this but perhaps if all the forward stream requests are IPv4, we can create the socket that way in the first place? Though that would only be relevant for the first time the socket is created. Following calls to
rtp_forward
might add a new IPv6 host.Thanks in advance!
The text was updated successfully, but these errors were encountered: