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

Microphone support #4671

Merged
merged 11 commits into from Mar 20, 2019

Conversation

@jroweboy
Copy link
Member

jroweboy commented Mar 5, 2019

This adds microphone service implementations using both a real microphone and a static microphone. As i'm not too sure about the design, I pushed it working and would like to ask for any feedback before cleaning up and getting it merged. It should be canary ready from the get go if someone wants to tag it.

Design:
Frontend selects a Mic implementation. Core will "get" the current Mic implementation when loading MIC_U services. When the game will configure the parameters for sampling, core will forward these to the frontend mic impl. Each frontend mic is responsible for collecting whatever data it needs, and managing its threads. The MIC_U service will start a CoreTiming event loop that will call Read on the frontend Mic impl and get the latest data from the frontend. This is sync and blocks the core emulation, so frontends should take care to do as little as possible at this point.

In general the code should be decent quality, but I am already bored of working on it again, so I want to get it out. Requesting for any design decisions before I get the standard c++ code quality treatment, so i don't have to waste much time if the whole thing needs a rewrite anyway.

Real microphone runs on a separate thread using cubeb input support and writes the data into a spsc queue. When read is called, it will pull as much as it can from the queue and write it to shared mem.

Static microphone returns the same N random bytes every time Read is called.

Thanks to tobi for fixing that bug i had and pressuring me to finally finish this.

THIS DOES NOT INCLUDE A KEY OR CONTROLLER MAPPING TO LET YOU "PUSH TO TALK" Someone else can add that later if they want. shouldn't be too hard.


This change is Reviewable

jroweboy and others added some commits Feb 27, 2019

long CubebInput::Impl::DataCallback(cubeb_stream* stream, void* user_data, const void* input_buffer,
void* output_buffer, long num_frames) {
Impl* impl = static_cast<Impl*>(user_data);
u8 const* data = reinterpret_cast<u8 const*>(input_buffer);

This comment has been minimized.

Copy link
@lioncash

lioncash Mar 5, 2019

Member
Suggested change
u8 const* data = reinterpret_cast<u8 const*>(input_buffer);
const u8* data = reinterpret_cast<const u8*>(input_buffer);

This can also be moved after the conditional check below.

return (sample_size == 8) ? CACHE_8_BIT : CACHE_16_BIT;
}

static std::shared_ptr<Mic::Interface> current_mic;

This comment has been minimized.

Copy link
@lioncash

lioncash Mar 5, 2019

Member

This should probably go into the system class.

Show resolved Hide resolved src/core/frontend/mic.h Outdated
Show resolved Hide resolved src/core/hle/service/mic_u.h Outdated
Show resolved Hide resolved src/core/settings.h Outdated
@legend800

This comment has been minimized.

Copy link

legend800 commented Mar 5, 2019

Just tested static and real mic and everything functioned correctly with SM3DL and WarioWare Gold. Amazing work!

Show resolved Hide resolved src/citra_qt/configuration/config.cpp Outdated
Show resolved Hide resolved src/citra_qt/configuration/configure_audio.cpp
Show resolved Hide resolved src/citra_qt/configuration/configure_audio.cpp Outdated
Show resolved Hide resolved src/core/frontend/mic.cpp Outdated

#include <memory>
#include <vector>
#include "common/swap.h"

This comment has been minimized.

Copy link
@zhaowenlan1779

zhaowenlan1779 Mar 5, 2019

Member

where is this used?

Show resolved Hide resolved src/core/frontend/mic.h Outdated
Show resolved Hide resolved src/core/hle/service/mic_u.cpp
Show resolved Hide resolved src/core/hle/service/mic_u.cpp Outdated
Show resolved Hide resolved src/core/hle/service/mic_u.cpp Outdated
@wwylele
Copy link
Member

wwylele left a comment

I think the overall design is fine as in I don't see we are going to rewrite the entire thing, so I skipped that step and went into the nit-picking phase 😛

Show resolved Hide resolved src/audio_core/cubeb_input.cpp Outdated
Show resolved Hide resolved src/core/hle/service/mic_u.cpp Outdated
Show resolved Hide resolved src/citra_qt/configuration/configure_audio.cpp
Show resolved Hide resolved src/core/frontend/mic.h Outdated
Show resolved Hide resolved src/core/frontend/mic.h Outdated
Show resolved Hide resolved src/core/hle/service/mic_u.cpp Outdated
Show resolved Hide resolved src/core/hle/service/mic_u.cpp Outdated
Show resolved Hide resolved src/core/frontend/mic.h Outdated
Show resolved Hide resolved src/core/frontend/mic.cpp
Show resolved Hide resolved src/core/hle/service/mic_u.cpp

@wwylele wwylele added canary-merge and removed canary-merge labels Mar 5, 2019

@wwylele

This comment has been minimized.

Copy link
Member

wwylele commented Mar 5, 2019

Please fix compiler error before we can merge this into canary.

}

IPC::RequestBuilder rb = rp.MakeBuilder(1, 0);
rb.Push(RESULT_SUCCESS);

LOG_WARNING(Service_MIC, "called, size=0x{:X}", size);
LOG_TRACE(Service_MIC, "MIC:U MapSharedMem called, size=0x{:X}", size);

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Mar 6, 2019

Contributor

no need to include function name here.

}

void UnmapSharedMem(Kernel::HLERequestContext& ctx) {
IPC::RequestParser rp{ctx, 0x02, 0, 0};
IPC::RequestBuilder rb = rp.MakeBuilder(1, 0);
shared_memory = nullptr;
rb.Push(RESULT_SUCCESS);
LOG_WARNING(Service_MIC, "called");
LOG_TRACE(Service_MIC, "MIC:U UnmapSharedMem called");

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Mar 6, 2019

Contributor

dito.

static_cast<u32>(encoding), static_cast<u32>(sample_rate), audio_buffer_offset,
audio_buffer_size, audio_buffer_loop);
LOG_TRACE(Service_MIC,
"MIC:U StartSampling called, encoding={}, sample_rate={}, "

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Mar 6, 2019

Contributor

dito


IPC::RequestBuilder rb = rp.MakeBuilder(1, 0);
rb.Push(RESULT_SUCCESS);
LOG_WARNING(Service_MIC, "(STUBBED) called, sample_rate={}", static_cast<u32>(sample_rate));
LOG_TRACE(Service_MIC, "MIC:U AdjustSampling sample_rate={}",

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Mar 6, 2019

Contributor

dito

rb.Push<bool>(is_sampling);
LOG_WARNING(Service_MIC, "(STUBBED) called");
LOG_TRACE(Service_MIC, "MIC:U IsSampling: {}", is_sampling);

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Mar 6, 2019

Contributor

dito


IPC::RequestBuilder rb = rp.MakeBuilder(1, 0);
rb.Push(RESULT_SUCCESS);
LOG_WARNING(Service_MIC, "(STUBBED) called, mic_gain={}", mic_gain);
LOG_TRACE(Service_MIC, "MIC:U SetGain gain={}", gain);

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Mar 6, 2019

Contributor

dito

is_sampling = false;
LOG_WARNING(Service_MIC, "(STUBBED) called");
mic->StopSampling();
LOG_TRACE(Service_MIC, "MIC:U StopSampling called");

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Mar 6, 2019

Contributor

dito.

LOG_WARNING(Service_MIC, "(STUBBED) called");
u8 gain = mic->GetGain();
rb.Push<u8>(gain);
LOG_TRACE(Service_MIC, "MIC:U GetGain gain={}", gain);

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Mar 6, 2019

Contributor

dito.


IPC::RequestBuilder rb = rp.MakeBuilder(1, 0);
rb.Push(RESULT_SUCCESS);
LOG_WARNING(Service_MIC, "(STUBBED) called, mic_power={}", mic_power);
LOG_TRACE(Service_MIC, "MIC:U SetPower mic_power={}", power);

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Mar 6, 2019

Contributor

dito.

rb.Push<u8>(mic_power);
LOG_WARNING(Service_MIC, "(STUBBED) called");
LOG_TRACE(Service_MIC, "MIC:U GetPower called");

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Mar 6, 2019

Contributor

dito.

@jroweboy

This comment has been minimized.

Copy link
Member Author

jroweboy commented Mar 6, 2019

I think i've addressed all the feedback. Let me know if i missed your suggestion, there was a lot.

Show resolved Hide resolved src/core/frontend/mic.cpp Outdated
constexpr u64 BufferUpdateRate8180 = BASE_CLOCK_RATE_ARM11 / 511;
constexpr u64 BufferUpdateRate10910 = BASE_CLOCK_RATE_ARM11 / 681;
constexpr u64 BufferUpdateRate16360 = BASE_CLOCK_RATE_ARM11 / 1022;
constexpr u64 BufferUpdateRate32730 = BASE_CLOCK_RATE_ARM11 / 2045;

This comment has been minimized.

Copy link
@wwylele

wwylele Mar 6, 2019

Member

Leftover code you forgot to remove?

Show resolved Hide resolved src/core/hle/service/mic_u.cpp Outdated
Show resolved Hide resolved src/audio_core/cubeb_input.cpp Outdated
Show resolved Hide resolved src/core/frontend/mic.h Outdated
Show resolved Hide resolved src/core/hle/service/mic_u.cpp Outdated

@wwylele wwylele added the canary-merge label Mar 6, 2019

@jroweboy

This comment has been minimized.

Copy link
Member Author

jroweboy commented Mar 7, 2019

Changes since last update:
Removed global state/ addressed all review feedback
Add mic hot swapping (so you can change mic settings while the game is running seamlessly)
Should've fixed android compiling support (no real mic support for android atm)

@jroweboy jroweboy changed the title [RFC] Microphone support Microphone support Mar 7, 2019

@wwylele

wwylele approved these changes Mar 8, 2019

@bunnei

bunnei approved these changes Mar 8, 2019

@zhaowenlan1779
Copy link
Member

zhaowenlan1779 left a comment

just two minor include nits

@@ -5,10 +5,12 @@
#include <utility>
#include "audio_core/dsp_interface.h"
#include "core/core.h"
#include "core/frontend/emu_window.h"

This comment has been minimized.

Copy link
@zhaowenlan1779

zhaowenlan1779 Mar 8, 2019

Member

where is this used?


#include <array>
#include "core/frontend/mic.h"
#include "core/hle/service/mic_u.h"

This comment has been minimized.

Copy link
@zhaowenlan1779

zhaowenlan1779 Mar 8, 2019

Member

where is this used?

@slashiee

This comment has been minimized.

Copy link

slashiee commented Mar 12, 2019

Hmm... my microphone seems to not be cooperating with this feature?
https://youtu.be/H_iqTS2Q8gY

@jroweboy

This comment has been minimized.

Copy link
Member Author

jroweboy commented Mar 18, 2019

@slashiee please retest with canary 1260 or greater. if its still not working, please paste a log and a save file for it (if needed) so we can replicate the issue.

Changes since last time

  • Addressed review comments
  • Added support for 8bit pcm samples in cubeb (real device) input.
  • Removed my attempt to up the volume of the users mic since it didn't seem to work anyway...

I haven't tested 8bit yet, but its pretty straight forward. If its broken, i'll go the extra mile to get that tested as well.

@slashiee

This comment has been minimized.

Copy link

slashiee commented Mar 18, 2019

It's working now, thank you

@jroweboy

This comment has been minimized.

Copy link
Member Author

jroweboy commented Mar 19, 2019

CI is unrelated.

The macOS crash is verified fixed https://community.citra-emu.org/t/real-mic-input-causes-canary-to-crash-on-osx-10-14/93053/4

Ready for mergeeeeeeeeeeee?

@jroweboy jroweboy force-pushed the jroweboy:mic4 branch from 411c4f8 to 9b5fa32 Mar 20, 2019

@jroweboy jroweboy force-pushed the jroweboy:mic4 branch from 9b5fa32 to b4d5384 Mar 20, 2019

@CaptV0rt3x CaptV0rt3x merged commit aedf5a8 into citra-emu:master Mar 20, 2019

3 checks passed

ci/bitrise/4ccd8e5720f0d13b/pr Passed - citra
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@NeilZander

This comment has been minimized.

Copy link

NeilZander commented Apr 3, 2019

I'm sorry for acting like a noob. Not entirely sure where to go with finding things on github... I'm having trouble finding where the download is so I can finally use microphone support? or am I sadly reading this wrong and it's not available yet?

@slashiee

This comment has been minimized.

Copy link

slashiee commented Apr 3, 2019

It's already a part of nightly and Canary.

@NeilZander

This comment has been minimized.

Copy link

NeilZander commented Apr 3, 2019

ah okay thanks for replying one last question then does it activate automatically? When I downloaded the update I could not find anything to show it was actually there that's why I could not tell if I already had it or not.

@jroweboy

This comment has been minimized.

Copy link
Member Author

jroweboy commented Apr 3, 2019

@NeilZander its in configure system audio settings. But also this is an issue tracker for developers and not a support forum. Please use the forums for support requests. https://community.citra-emu.org/ Go there if you need more information, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.