-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
IOS/USB: Emulate Wii Speak using cubeb #12769
base: master
Are you sure you want to change the base?
Conversation
cf3916c
to
46d7831
Compare
I fixed a few more things. The Wii Speak Channel appears to work properly now (its initial test won't work though, as we don't emulate the echo reduction). I also added the Android support. To test this PR, you'll need to enable the emulated Wii Speak under The Wii Speak Channel can be tested using a regular one (but by redirecting Nintendo domains to nothing like localhost, resulting in a skippable 20100 error) or via a Riiconnect24 patched version (AFAICT, WiiLink24 patcher doesn't seem to offer it currently, Wiimmfi WAD Patcher does). A Wii friend is required in order to be able to record Wii messages like this: https://www.youtube.com/watch?v=4kpNID36pW0 This PR can be tested on Monster Hunter 3 as well. You will need to patch the game to go online using alternative servers. You will need a friend in-game also using a Wii Speak (Dolphin or a real Wii) to be able to speak in a city. I haven't checked this PR with other games yet, such as Animal Crossing. I'll when I'll get the time to do so. Otherwise, this PR is still ready to be reviewed & tested. |
97daea9
to
3693fe5
Compare
bool m_device_attached = false; | ||
bool init = false; | ||
std::unique_ptr<Microphone> m_microphone{}; | ||
const DeviceDescriptor m_device_descriptor{0x12, 0x1, 0x200, 0, 0, 0, 0x10, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const DeviceDescriptor m_device_descriptor{0x12, 0x1, 0x200, 0, 0, 0, 0x10, | |
const DeviceDescriptor m_device_descriptor{ | |
.bLength = 0x12, | |
.bDescriptorType = 0x01, | |
.bcdUSB = 0x0200, | |
.bDeviceClass = 0x00, | |
... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAICT, this syntax isn't used in the current codebase for similar device classes. I wouldn't mind changing it in a follow up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does the permission prompt work on Android? Does cubeb automagically make the permission prompt show up?
@@ -923,4 +923,6 @@ It can efficiently compress both junk data and encrypted Wii data. | |||
<string name="incompatible_figure_selected">Incompatible Figure Selected</string> | |||
<string name="select_compatible_figure">Please select a compatible figure file</string> | |||
|
|||
<string name="emulate_wii_speak">Wii Speak</string> | |||
<string name="disconnect_wii_speak">Mute Wii Speak</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Mute" doesn't match what the setting is called in the code or in DolphinQt. Is this something that was overlooked, or is it intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is intended since it is presented differently on PC and Android. On PC it has its own dedicated window (similar to the emulated Infinity base):
Whereas on Android, it's only a toggle settings below the one enabling the Wii Speak. So to make it clearer for the end-user I decided to rename it (as the effect and purpose of disconnecting it is not to record audio). I feel that having a "Wii Speak" toggle, plus a "Disconnect Wii Speak" toggle right under it sounds strange from a UI point of view.
I'm open to better ways of presenting this option to the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does the setting do from the perspective of the emulated software? Does it disconnect the Wii Speak USB device, or does it just make the audio be silence? If it's the latter, I think both the Android UI and the PC UI are okay, but the internal name for the setting would make more sense as "mute" than "connect". "Connect" makes sense in the PC UI only because it's in the context of whether you want to connect a host microphone.
dc72880
to
35436ac
Compare
Ideally, it should have been used to disconnect the device. However, I couldn't find a way to do so and it had a side effect so I chose not to process the microphone data instead. dolphin/Source/Core/Core/IOS/USB/Emulated/WiiSpeak.cpp Lines 97 to 101 in 35436ac
dolphin/Source/Core/Core/IOS/USB/Emulated/Microphone.cpp Lines 252 to 255 in 35436ac
Apparently, enabling the permission after cubeb was initialised doesn't seem to work. I added a warning message before requesting the permission to warn the user about this issue. This part (especially the android code) needs to be properly reviewed as it's been a while since I've programmed for Android and I'm unsure that's the proper way to do it. |
5d1ea67
to
0835422
Compare
Is anyone else planning to review this? Is this considered finished at this point? |
b21121e
to
9023aa3
Compare
The PR is finished. Though, it might be worth testing to ensure there's no oversight. Especially, on the Android part where I'm not very confident as it's working on my end but I'm unsure that the way I'm getting the activity is correct (or should I check the other callbacks too). |
Does anyone else want to review this? I can give this a test on Android, but I won't be reviewing the code. |
Are you still intending to add a microphone selector? Having a window with a single toggle is quite odd, as in that case you could just put the toggle itself into the file menu. |
Doesn't the selector appear when the emulated device is enabled? I'm just using a layout similar to the one of the Infinity Base, another emulated USB device. I'm open to suggestion regarding this window design. The only constraint that it has to take into account (currently) is that it cannot enable or disable the emulated USB device when the emulation has started (i.e. it has to be chosen before launching the game). |
When I tested this I only saw what was in the top window. What scenario does that occur? |
Currently, the microphone configuration groupbox is displayed only when the Emulate Wii Speak checkbox is checked. So the groupbox should (dis)appear when the checkbox state is changed. If there is a bug and it doesn't behave properly, then I'll have to investigate to see what's causing this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay in following up. I haven't been able to review all the USB and audio code in detail, but I've caught a few things.
...in/java/org/dolphinemu/dolphinemu/features/settings/ui/viewholder/SwitchSettingViewHolder.kt
Outdated
Show resolved
Hide resolved
Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/ActivityTracker.kt
Outdated
Show resolved
Hide resolved
b938bcf
to
987896a
Compare
Last call for reviews on this. It's been waiting very long time and if I test it and the use cases are working on Linux and Windows, I will be merging it unless someone posts here and says they plan on reviewing it again or tells me not to merge it. |
018c914
to
0352330
Compare
Credits to @degasus and @shuffle2 (godisgovernment): https://github.com/degasus/dolphin/tree/wiispeak
Based on @noahpistilli (Sketch) PR: dolphin-emu#12567 Fixed the Windows support and the heisenbug caused by uninitialized members. Config system integration finalized.
The register should be 12 (i.e. 0x0c) instead of 0xc0
It seems to fix random echoes and reduce noises when nobody is speaking
Based on LibusbDevice::SubmitTransfer code
Add a volume modifier to the UI which relies on gain.
I rebased this PR and added some kind of loudness level calculation. I'm not an audio expert so it might be worth checking that the formulas I'm using are correct. I also switched this PR back to draft as I'm doing some more testing regarding the loudness level calculation. |
This PR is based on #12567.
I don't have much knowledge regarding the technical details of the implementation. I mainly fixed the Windows support, the heisenbug due to uninitialised members and finalised the config integration.
Ready to be reviewed and tested.
This change is