jhudsl / mscstts

R Client for the Microsoft Cognitive Services Text to Speech REST API
GNU General Public License v3.0
8 stars 6 forks source link

region support is not complete #9

Open boltomli opened 4 years ago

boltomli commented 4 years ago

Voice availability on Azure is region specific. I updated ms_list_voices in #8, but unfortunately it turns out other functions may not support region well, such as get key, get token etc.

Major problems when using this feature:

muschellij2 commented 4 years ago

What are you saying here? Revert the PR? What is the "feature". What action item do you want?

boltomli commented 4 years ago

Sorry, got a slow network connection at the moment. Will update the issue later with more details.

boltomli commented 4 years ago

I identified the error stack now. Repro steps:

  1. Get voices from a region that has more voices than "westus" (81 voices), such as "southeastasia".
    > more_voices <- mscstts::ms_list_voices(api_key = "XXXX", region = "southeastasia")
    > length(more_voices$Name)
    [1] 106
  2. Try any neural voice in the list, but that is not found in ms_locale_df()
    > res <- mscstts::ms_synthesize("valid voice in the region", voice = "en-IN-NeerjaNeural", api_key = "XXXX", region = "southeastasia")
    Error in ms_voice_info(voice) : 
    Voice given is not a recognized voice! See ms_locale_df()

The error happens in ms_voice_info(). Currently this function validates voice against embedded ms_locale_df(), which is not an exhaustive list of available voices. It's simply because voice availability is not the same across different Azure regions.

To solve this problem:

Solution Pros Cons
Update ms_locale_df() to add more voices. Easy to modify. Keep runtime fast and stable. Have to update voice list from time to time to follow upstream. Not aware of different regions (a voice in this list may not be available in specific regions).
Refactor ms_locale_df() to add more voices per region. Keep runtime fast and stable. Aware of region differences. Have to update voice list from time to time to follow upstream. Lots of code changes.
Validate voice against dynamic list of voices. Accurate enough (when all is well.) Slower runtime. More requests bring more chance of failure.
Disable voice validation or use a warning instead of error. User friendly and flexible. Moderate code changes. A little too loose?
muschellij2 commented 4 years ago

I'd defer to the warning, but I added some lines in there - Can you see if the commit works now? See https://github.com/muschellij2/mscstts/commit/5e7106bc83555df8095ec55dfa16a79cabfc3ced#diff-39708ad91ad640178ec80e250dc1ec5bR80

boltomli commented 4 years ago

Thanks! Yes, with this commit the short name works as expected.