dolphin icon indicating copy to clipboard operation
dolphin copied to clipboard

Published 20 hours ago • verified publisher icon dolphin-emu

BBA/BuiltIn: Add SSDP multicast support

Open sepalani opened this issue 2 years ago • 7 comments

This PR implements the SSDP multicast support properly. It fixes one issue I had where my Windows host, its Linux VM and my smartphone couldn't see each other joining/leaving the LAN consistently. This method also seems firewall friendly, I don't see packets being dropped anymore on Wireshark.

The UDP port not opening issue isn't fixed in this PR. If the Windows discovery service is enabled, the VM and its hosts can't see each other consistently (shouldn't matter as I don't expect many people to use Dolphin in a VM to play LANs/online).

Ready to be reviewed & tested.

This change is Reviewable

sepalani avatar Jul 29 '22 15:07 sepalani

This PR causes a crash on android for my device (Pixel 3a) but apparently no one else has the problem.

JMC47 avatar Jul 29 '22 16:07 JMC47

After some testing, here's what's worth mentioning for Windows users:

  • When the SSDP discovery service is disabled, it makes the console detection (using the SSDP protocol) more stable.
  • Disabling Windows firewall also makes matchmaking/negotiation (UDP) more consistent and less likely to fail (due to the firewall dropping the UDP packet.

sepalani avatar Jul 30 '22 07:07 sepalani

For reference, so that it isn't lost: Here is a log from JMC reproducing the crash. No symbols for the native code, unfortunately. https://pastebin.com/BFpmnxby

JosJuice avatar Jul 30 '22 19:07 JosJuice

Needs a rebase

lioncash avatar Aug 03 '22 18:08 lioncash

@JMC47 I chose another approach that doesn't rely on the JNI code. It might not be 100% accurate but it's still better than introducing a JNI crash. Can you confirm that this new build doesn't crash on Android anymore?

sepalani avatar Aug 04 '22 18:08 sepalani

I somehow missed this comment. My bad. Testing now

JMC47 avatar Aug 06 '22 19:08 JMC47

@sepalani this works and does not crash on Android.

JMC47 avatar Aug 06 '22 19:08 JMC47

@iwubcode Done. I wrote a more detailed comment regarding network interface and I improved the error logs to add the reason of failure.

This PR was rebased to use Common::DecodeNetworkError, I'll test it again later this week to make sure it didn't introduce regressions.

sepalani avatar Aug 24 '22 17:08 sepalani

Is this all done now? I'd like to get the BBA stuff squared away before the Progress Report and Beta build.

JMC47 avatar Aug 28 '22 17:08 JMC47

@JMC47 Yes, I'm done here and with https://github.com/dolphin-emu/dolphin/pull/10985. Tested on MKDD and Kirby Air Ride, the changes are working as intended on my end.

sepalani avatar Aug 28 '22 18:08 sepalani