Add microphone helper sample #1326
Conversation
|
|
|
|
||
| class MicrophoneHelper | ||
| { | ||
| #if ENABLE_WINMD_SUPPORT |
There was a problem hiding this comment.
Maybe we should add a note that this only works for IL2CPP and .NET 4.6.
We also need to add the dll references for this to work properly in the MRTK project. We will also need to add documentation for devs to know they will also need to setup their projects to use this as well.
There was a problem hiding this comment.
This works with both .Net and IL2CPP backends. There is no need for extra dll references. There shouldn't be any need to do extra setup in your project to use this. If you know of a scenario that adding this script will break let me know.
There was a problem hiding this comment.
Will do. I was just going off the Unity Documentation regarding this usage.
There was a problem hiding this comment.
I added a comment to the class and a link to the Unity documentation.
| #endif | ||
|
|
||
| // TODO: namespace might need to be updated for consistency with other MRTK samples | ||
| namespace MixedRealityToolkit |
There was a problem hiding this comment.
namespace HoloToolkit.Unity
| } | ||
| } | ||
|
|
||
| public class MicrophoneHelperSample : MonoBehaviour |
There was a problem hiding this comment.
classes should be in their own files.
Also this class is in the generic namespace instead of the MRTK's
| #if ENABLE_WINMD_SUPPORT | ||
| async void Update () | ||
| #else | ||
| void Update() |
There was a problem hiding this comment.
nit: private access modifier
| // This sample code is a modified version of an official UWP sample which can be found on: | ||
| // https://github.com/Microsoft/Windows-universal-samples/blob/master/Samples/SpeechRecognitionAndSynthesis/cs/AudioCapturePermissions.cs | ||
| // | ||
| //********************************************************* |
There was a problem hiding this comment.
Take a look at the other headers in the toolkit.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.
| /// For full details see unity documentation page | ||
| /// https://docs.unity3d.com/Manual/IL2CPP-WindowsRuntimeSupport.html | ||
| /// </summary> | ||
| class MicrophoneHelper |
There was a problem hiding this comment.
nit: missing access modifier
|
|
||
| namespace HoloToolkit.Unity | ||
| { | ||
| enum MicrophoneStatus |
There was a problem hiding this comment.
nit: missing access modifier
| { | ||
| #if ENABLE_WINMD_SUPPORT | ||
| // If no recording device is attached, attempting to get access to audio capture devices will throw | ||
| // a System.Exception object, with HResult = 0xC00DABE0 set. |
There was a problem hiding this comment.
nit: <summary> tags?
There was a problem hiding this comment.
I'm not sure what you mean here. Can you elaborate?
There was a problem hiding this comment.
Update the comment to use <summary> tags.
or even <remarks> tags explaining it's usage.
| public class MicrophoneHelperSample : MonoBehaviour | ||
| { | ||
| [Tooltip("Assign key to quickly test the microphone helper code")] | ||
| public KeyCode checkMicrophoneKey = KeyCode.M; |
There was a problem hiding this comment.
Have we checked to make sure KeyCode.M isn't used elsewhere in the project?
There was a problem hiding this comment.
A search in all .cs files did not return any other references.
| #if ENABLE_WINMD_SUPPORT | ||
| var status = await MixedRealityToolkit.MicrophoneHelper.GetMicrophoneStatus(); | ||
| var status = await MicrophoneHelper.GetMicrophoneStatus(); |
There was a problem hiding this comment.
Is there a way we can use MicrophoneHelper.GetMicrophoneStatus() without having to check for ENABLE_WINMD_SUPPORT in our implementation?
There was a problem hiding this comment.
I could also make the stub method async, but then I'll get a warning that await is not being used anywhere. If you're fine with an extra warning I can change that.
There was a problem hiding this comment.
When will the warning show? During compile or build time?
There was a problem hiding this comment.
If compile time, I'd rather not, but if it's at build time, then it's not that big a deal.
There was a problem hiding this comment.
Actually this is not a good idea. async/await were introduces with C# 5 and require .Net 4.5+. It would be better it remained this way.
There was a problem hiding this comment.
Just trying to think about ways to make this simpler for devs to use in their own code.
| } | ||
| catch (Exception exception) | ||
| { | ||
| // This can be replicated by using remote desktop to a system, but not redirecting the microphone input. |
There was a problem hiding this comment.
Mixed Reality won't work in remote desktop or VM contexts, so you can probably edit down this comment a little to avoid confusion (still possible to disable all of the audio endpoints though)
There was a problem hiding this comment.
That's a good point, would this also include Unity's holographic remoting?
There was a problem hiding this comment.
Holographic remoting is still playing in the editor, where this script would be no-op. I will remove the remote desktop references from the comments.
|
|
||
| namespace HoloToolkit.Unity | ||
| { | ||
| public class MicrophoneHelperSample : MonoBehaviour |
There was a problem hiding this comment.
Is this a test/example class? It doesn't belong in the core toolkit folder.
Microphone helper sample to query microphone presence and availability at runtime. Partners have asked for such sample to make decisions in game whether to show speech use helpers.