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

IOS/USB: Emulate Wii Speak #12567

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

noahpistilli
Copy link
Member

This PR emulates the Wii Speak through passthrough of a real microphone. The register code was derived from this code by degasus, however I suspect there are some inaccuracies.

In its current state the audio that is being passed through is incredibly choppy and delayed, along with an echo effect. I didn't encounter it this badly when I was working with OpenAL, however I switched to cubeb for better compatibility.

Timings are also inaccurate which could be contributing to this issue. Regardless, this allows for the Wii Speak Channel 2.0 to function.

@noahpistilli noahpistilli force-pushed the wii-speak branch 2 times, most recently from d69e44a to 3ad6748 Compare February 7, 2024 03:10
static constexpr u32 SAMPLING_RATE = 8000;
static constexpr u32 BUFFER_SIZE = SAMPLING_RATE / 2;

s16* stream_buffer;
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a unique_ptr? This doesn't seem to actually get freed

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought I deleted it in the destructor, I guess not. unique_ptr would be better though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't a buffer allocated on the stack (for instance, using a std::array) be used if the size is hardcoded/known in advance? Which seems to be the case here.

Copy link
Contributor

@sepalani sepalani left a comment

Choose a reason for hiding this comment

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

Here is a quick review regarding the code. I'm going to test this PR later this week.

Comment on lines +6 to +14
#include <thread>

#include "Common/CommonTypes.h"
#include "Common/Event.h"
#include "Common/WorkQueueThread.h"
#include "Core/HW/Memmap.h"
#include "Core/IOS/USB/Common.h"
#include "Core/IOS/USB/Emulated/Microphone.h"
#include "Core/System.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Are all these includes necessary? <thread> and "Common/WorkQueueThread.h" don't seem used here.

u8 m_active_interface = 0;
bool m_device_attached = false;
bool init = false;
std::unique_ptr<Microphone> m_microphone;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't the Microphone class be forward declared so its header file doesn't need to be included?

Comment on lines +5 to +6
#include "Core/HW/Memmap.h"
#include "Core/System.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't these two headers already included?

Comment on lines +12 to +24
m_vid = 0x57E;
m_pid = 0x0308;
m_id = (u64(m_vid) << 32 | u64(m_pid) << 16 | u64(9) << 8 | u64(1));
m_device_descriptor =
DeviceDescriptor{0x12, 0x1, 0x200, 0, 0, 0, 0x10, 0x57E, 0x0308, 0x0214, 0x1, 0x2, 0x0, 0x1};
m_config_descriptor.emplace_back(ConfigDescriptor{0x9, 0x2, 0x0030, 0x1, 0x1, 0x0, 0x80, 0x32});
m_interface_descriptor.emplace_back(
InterfaceDescriptor{0x9, 0x4, 0x0, 0x0, 0x0, 0xFF, 0xFF, 0xFF, 0x0});
m_interface_descriptor.emplace_back(
InterfaceDescriptor{0x9, 0x4, 0x0, 0x01, 0x03, 0xFF, 0xFF, 0xFF, 0x0});
m_endpoint_descriptor.emplace_back(EndpointDescriptor{0x7, 0x5, 0x81, 0x1, 0x0020, 0x1});
m_endpoint_descriptor.emplace_back(EndpointDescriptor{0x7, 0x5, 0x2, 0x2, 0x0020, 0});
m_endpoint_descriptor.emplace_back(EndpointDescriptor{0x7, 0x5, 0x3, 0x1, 0x0040, 1});
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't these be const member variables?

{
case USBHDR(DIR_DEVICE2HOST, TYPE_STANDARD, REC_INTERFACE, REQUEST_GET_INTERFACE):
{
std::array<u8, 1> data{1};
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't this array be static const/constexpr ?

Comment on lines +76 to +77
void GetRegister(std::unique_ptr<CtrlMessage>& cmd);
void SetRegister(std::unique_ptr<CtrlMessage>& cmd);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't these parameters be const& and GetRegister a const method?

#pragma once

#include <mutex>
#include <string>
Copy link
Contributor

Choose a reason for hiding this comment

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

This <string> include should be removed if not used in this header file.

static constexpr u32 SAMPLING_RATE = 8000;
static constexpr u32 BUFFER_SIZE = SAMPLING_RATE / 2;

s16* stream_buffer;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't a buffer allocated on the stack (for instance, using a std::array) be used if the size is hardcoded/known in advance? Which seems to be the case here.

m_work_queue.EmplaceItem([this, &sync_event] {
Common::ScopeGuard sync_event_guard([&sync_event] { sync_event.Set(); });
#endif
stream_size = buff_size_samples * 500;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't stream_size and buff_size_samples be const?

}

if (cubeb_stream_init(m_cubeb_ctx.get(), &m_cubeb_stream,
"Dolphin Emulated GameCube Microphone", nullptr, &params, nullptr,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this referring to the GameCube's Microphone which used the memory card port? If it is a typo, referring to "Dolphin Emulated Wii Speak" seems reasonable to me.

Copy link
Contributor

@sepalani sepalani left a comment

Choose a reason for hiding this comment

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

I tested it on two Dolphin portable builds with Monster Hunter Tri (it requires 2 players to be friends). The Wii Speak is detected but it doesn't detect sound and/or register my laptop microphone (which is odd as I don't have issues using it on Firefox).

I have PMIC symbols on some of my games including MH3 so I might be able to diagnose the issue. I'll do more testing and try to throw one or two of my Wii into the mix to see if there is any change.

BTW, I have faced an haisenbug with this build. I can't reproduce it with a debugger attached or using a debug build. It occurs when the game try to initialise or detect the Wii Speak, the second instance trying to do so crashes instantly. Sometimes, it doesn't happen which is very odd. It might be a clash when trying to acquire the microphone but I'm not sure.

Comment on lines +201 to +202
auto wii_speak = std::make_unique<USB::WiiSpeak>(GetEmulationKernel(), "Wii Speak");
CheckAndAddDevice(std::move(wii_speak), new_devices, hooks, always_add_hooks);
Copy link
Contributor

Choose a reason for hiding this comment

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

You should add an option in Dolphin's config system to enable/disable emulated wii speak as users might want to disable it in some circumstances.

sepalani added a commit to sepalani/dolphin that referenced this pull request May 9, 2024
Based on @noahpistilli (Sketch) PR:
dolphin-emu#12567

Fixed the Windows support and the haisenbug caused by uninitialized
members.

Config system integration finalized.
sepalani added a commit to sepalani/dolphin that referenced this pull request May 9, 2024
Based on @noahpistilli (Sketch) PR:
dolphin-emu#12567

Fixed the Windows support and the haisenbug caused by uninitialized
members.

Config system integration finalized.
sepalani added a commit to sepalani/dolphin that referenced this pull request May 9, 2024
Based on @noahpistilli (Sketch) PR:
dolphin-emu#12567

Fixed the Windows support and the heisenbug caused by uninitialized
members.

Config system integration finalized.
sepalani added a commit to sepalani/dolphin that referenced this pull request May 9, 2024
Based on @noahpistilli (Sketch) PR:
dolphin-emu#12567

Fixed the Windows support and the heisenbug caused by uninitialized
members.

Config system integration finalized.
sepalani added a commit to sepalani/dolphin that referenced this pull request May 10, 2024
Based on @noahpistilli (Sketch) PR:
dolphin-emu#12567

Fixed the Windows support and the heisenbug caused by uninitialized
members.

Config system integration finalized.
sepalani added a commit to sepalani/dolphin that referenced this pull request May 10, 2024
Based on @noahpistilli (Sketch) PR:
dolphin-emu#12567

Fixed the Windows support and the heisenbug caused by uninitialized
members.

Config system integration finalized.
sepalani added a commit to sepalani/dolphin that referenced this pull request May 12, 2024
Based on @noahpistilli (Sketch) PR:
dolphin-emu#12567

Fixed the Windows support and the heisenbug caused by uninitialized
members.

Config system integration finalized.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants