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

SpeechToText #808 #1127

Merged
merged 29 commits into from
May 18, 2023
Merged

SpeechToText #808 #1127

merged 29 commits into from
May 18, 2023

Conversation

VladislavAntonyuk
Copy link
Collaborator

@VladislavAntonyuk VladislavAntonyuk commented Apr 4, 2023

Description of Change

Linked Issues

PR Checklist

  • Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Documentation created or updated: SpeechToText MicrosoftDocs/CommunityToolkit#267

Additional information

@VladislavAntonyuk VladislavAntonyuk requested a review from a team April 4, 2023 16:39
@VladislavAntonyuk VladislavAntonyuk self-assigned this Apr 4, 2023
@brminnick
Copy link
Collaborator

brminnick commented Apr 12, 2023

Thanks Vlad!

I made a couple tweaks to this PR:

  • Changed namespace to CommunityToolkit.Maui.Media
  • Changed Listen() to ListenAsync()
    • This aligns to the TextToSpeech.SpeakAsync() and should make it easier to promote this feature into .NET MAUI
    • (Albeit, I personally don't like to add the Async suffix unless there's an equivalent non-async suffix
  • Changed TextToSpeechImplementation classes from public to internal
    • They don't need to be public and we can avoid future Issues / bugs by keeping them internal
  • Added Windows Error Handling
  • Updated the Sample App
    • Replaced the Editor with Entry to force users to use speech-to-text
    • Added Label at the top explaining ISpeechToText
    • Added NSSpeechRecognitionUsageDescription to both iOS + MacCatalyst Info.plist
  • Added SharedSpeechToTextImplementation.macios.cs to combine the shared logic between iOS + MacCatalyst
  • Ran dotnet format

I test the sample on iOS + MacCatalyst and it works great! I was unable to test on Windows + Android because my current build machine isn't configured properly.

@JoonghyunCho
Copy link
Member

Tizen Implementation is added :)

Copy link
Collaborator

@brminnick brminnick left a comment

Choose a reason for hiding this comment

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

Thanks Vlad! This is getting really really close!

I confirmed iOS, Android + Windows works.

However, Speech to Text on macOS isn't working for me and I can't quite figure out why. I'll mark this as Request changes just to ensure we address the macOS issue before accidentally merging it.

Copy link
Collaborator

@brminnick brminnick left a comment

Choose a reason for hiding this comment

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

Thanks Vlad!

We're really really close to finishing this.

I confirmed that speech-to-text is working on macOS now, but I found a weird bug that I can't figure out: When I speak into an external microphone on my MacBook Pro, the sample app doesn't detect any sound.

Are you able to duplicate + diagnose this behavior?

In the meantime, I'd say that this API surface is finalized and we're free to start working on the docs! @bijington - do you have availability this week to help with docs for this PR?

ScreenFlow

@VladislavAntonyuk
Copy link
Collaborator Author

@bijington dont worry. I will create a docs pr today.

@VladislavAntonyuk
Copy link
Collaborator Author

@brminnick It works for both microphones:

Screen.Recording.2023-05-16.at.20.14.44.mov

@JoonghyunCho
Copy link
Member

I fixed the Tizen implementation to solve the timing issue between prepare and start listening steps.

@brminnick brminnick enabled auto-merge (squash) May 18, 2023 17:08
@brminnick brminnick merged commit 21cb4ea into main May 18, 2023
@brminnick brminnick deleted the SpeechToText branch May 18, 2023 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Proposal] SpeechToText
4 participants