Skip to content
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

fix(connection): 🐛 Try localhost if broadcast fails, fixes wifi being required for cabled #1962

Merged
merged 3 commits into from
Feb 4, 2024

Conversation

Meister1593
Copy link
Collaborator

This makes so that if client can't broadcast, it will try to do announce on localhost.
Thus, making wifi requirement being needed on alvr being optional.

I also added and changed some hud signs, as well as removed wiki line about client discovery - apparently if you set ip on server and have adb working, it doesn't matter if you have client discovery enabled or not, it will connect to wired in either case.

@Meister1593 Meister1593 requested review from zmerp and Vixea January 15, 2024 15:23
Copy link
Member

@zmerp zmerp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks amazing! 😄 there is a thing I would change. I would like to hide as much as possible socket details in connection.rs. So you can make two methods announce_broadcast() and announce_local(). Even better would be to handle the local fallback all inside the AnnouncerSocket, but I'm ok if you would like to show the in between status on the HUD.

@Meister1593
Copy link
Collaborator Author

Looks amazing! 😄 there is a thing I would change. I would like to hide as much as possible socket details in connection.rs. So you can make two methods announce_broadcast() and announce_local(). Even better would be to handle the local fallback all inside the AnnouncerSocket, but I'm ok if you would like to show the in between status on the HUD.

Done. Changed to separate functions, left hud elements and distinction so people can see if they are connecting via cable or not :)

@Meister1593
Copy link
Collaborator Author

I think i may have accidentally did workaround over broadcast and if i remove local connect, nothing will change
as it's... well
udp
and adb is tcp...
i may need to think about this how to do it properly

@Meister1593 Meister1593 changed the title fix(connection): 🐛 Try localhost if broadcast fails, fixes wifi being required for cabled Draft: fix(connection): 🐛 Try localhost if broadcast fails, fixes wifi being required for cabled Jan 15, 2024
@Meister1593 Meister1593 marked this pull request as draft January 15, 2024 19:45
@fred41
Copy link

fred41 commented Feb 2, 2024

I think udp broadcasting is not working via adb tcp forwarding,
but for disabled wireless on client (with USB3 link connected), you could do the following in sockets.rs:

pub fn broadcast(&self) -> Result<()> {
        if LOCAL_IP != Ipv4Addr::UNSPECIFIED {
            self.socket.send_to(&self.packet, (Ipv4Addr::BROADCAST, CONTROL_PORT))?;
        }

        Ok(())
 }

So with wireless enabled, everything works as usually. With wireless disabled, we just don't try to broadcast.
This works very well on my system, so maybe this is a possible approach?

@Meister1593 Meister1593 force-pushed the fix-offline-connection branch from 81123d5 to 904a453 Compare February 3, 2024 10:58
@Meister1593 Meister1593 changed the title Draft: fix(connection): 🐛 Try localhost if broadcast fails, fixes wifi being required for cabled fix(connection): 🐛 Try localhost if broadcast fails, fixes wifi being required for cabled Feb 3, 2024
@Meister1593
Copy link
Collaborator Author

I think udp broadcasting is not working via adb tcp forwarding, but for disabled wireless on client (with USB3 link connected), you could do the following in sockets.rs:

pub fn broadcast(&self) -> Result<()> {
        if LOCAL_IP != Ipv4Addr::UNSPECIFIED {
            self.socket.send_to(&self.packet, (Ipv4Addr::BROADCAST, CONTROL_PORT))?;
        }

        Ok(())
 }

So with wireless enabled, everything works as usually. With wireless disabled, we just don't try to broadcast. This works very well on my system, so maybe this is a possible approach?

Thanks for suggestion, i changed my existing code to not assume that there is broadcast available on TCP cabled connection

I tested with: WIFI on, WIFI off, WIFI on different network, with and without client discovery - all works correctly as it should.
All what streamer should have is tcp and localhost ip set in it to be able to stream via wire regardless of how it's connected.

@Meister1593
Copy link
Collaborator Author

Without WiFi enabled it might do a couple of attempts to properly connect, but otherwise it does connect just fine after that.

@Meister1593 Meister1593 marked this pull request as ready for review February 3, 2024 11:05
@Vixea
Copy link
Collaborator

Vixea commented Feb 4, 2024

merging as is after I test it locally

@Vixea Vixea merged commit ab4f57c into alvr-org:master Feb 4, 2024
6 checks passed
@XenoPL
Copy link

XenoPL commented Feb 6, 2024

Hi, could this change improve support for wired ethernet connection via USB adapter (router<->usb ehternet adapter<->headset)?
I couldn't make my PC connect reliably with Pico4 with ALVR. Headset alone has no problems with internet access and streaming some videos, but ALVR client never detects assigned IP, connection to the PC is very choppy and laggy (it's unable to keep up even with Steam dashboard). Possibly to very high usage of HMDs CPU.
PC is connected to TPLink Archer VR2100 vDSL router with 1GBps ehternet cable.
I've tried two different usb-c 1Gbps LAN adapters
https://www.amazon.pl/dp/B09MQDWP6H?psc=1&ref=ppx_yo2ov_dt_b_product_details
https://www.amazon.pl/gp/product/B086SV2FK4/ref=ox_sc_act_title_1?smid=A2R2221NX79QZP&psc=1
and USB-C cable extender:
https://www.amazon.pl/dp/B0CL52SBLP?psc=1&ref=ppx_yo2ov_dt_b_product_details

@Meister1593
Copy link
Collaborator Author

Hi, could this change improve support for wired ethernet connection via USB adapter (router<->usb ehternet adapter<->headset)? I couldn't make my PC connect reliably with Pico4 with ALVR. Headset alone has no problems with internet access and streaming some videos, but ALVR client never detects assigned IP, connection to the PC is very choppy and laggy (it's unable to keep up even with Steam dashboard). Possibly to very high usage of HMDs CPU. PC is connected to TPLink Archer VR2100 vDSL router with 1GBps ehternet cable. I've tried two different usb-c 1Gbps LAN adapters https://www.amazon.pl/dp/B09MQDWP6H?psc=1&ref=ppx_yo2ov_dt_b_product_details https://www.amazon.pl/gp/product/B086SV2FK4/ref=ox_sc_act_title_1?smid=A2R2221NX79QZP&psc=1 and USB-C cable extender: https://www.amazon.pl/dp/B0CL52SBLP?psc=1&ref=ppx_yo2ov_dt_b_product_details

Honestly, i'm not too familiar with this kind of setup...
If i understand it correctly, this can only work in case if ALVR will attempt to directly connect by ip to headset (that's all it needs, this pr just changes so that it doesn't require multicast over network announce packet for connection)

@Meister1593
Copy link
Collaborator Author

If you can get ip of headset when it's connected like that and put it into alvr, it should work.

@XenoPL
Copy link

XenoPL commented Feb 6, 2024

Thanks for an answer, I got it working after few tweaks. Point was I took effect for a cause. It made HMD stutter because improved bandwidth and latency let ALVR kick bitrate to the roof. Once I reduced requested bitrate to ~120-130Mbps it made actual bitrate to be in 160-210MBps range which is what Pico can decode via hardware and keep its tiny CPUs from being overloaded.
Nice FPS boost in games was nice bonus too.

@Meister1593
Copy link
Collaborator Author

Fyi i can easily decode 800 mbps h264 and 200-220 HEVC on pico 4 pro, on linux, with type-c cable

@Meister1593 Meister1593 deleted the fix-offline-connection branch April 6, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants