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

Strange behavior setting voices with WinRT #29

Closed
Bear-03 opened this issue Jul 21, 2022 · 8 comments · May be fixed by #31
Closed

Strange behavior setting voices with WinRT #29

Bear-03 opened this issue Jul 21, 2022 · 8 comments · May be fixed by #31

Comments

@Bear-03
Copy link
Contributor

Bear-03 commented Jul 21, 2022

When I run this snippet, even though I changed the voice, Microsoft David is still being used when I call Tts::speak.

let mut tts = Tts::default().unwrap();
let voice_names: Vec<_> = tts
            .voices()
            .unwrap()
            .into_iter()
            .map(|v| v.name())
            .collect();

println!("{:?}", voice_names);

tts.set_voice(&tts.voices().unwrap()[1]).unwrap();
tts.speak("hello", false).unwrap();

stdout:

["Microsoft David", "Microsoft Zira", "Microsoft Mark"]

Looking at the output of the program, "hello" should be said by Zira, not David. Interestingly enough, everything works fine with the following change:

// ... (same as before)

tts.speak("", false).unwrap(); // Fixes voices
tts.set_voice(&tts.voices().unwrap()[1]).unwrap();
tts.speak("hello", false).unwrap(); // Now this will be said by Zira

This makes me think Tts::speak performs some kind of initialization that allows changing the voices afterwards.

@ndarilek
Copy link
Owner

Give main a shot, and if it works for you then I'll make a new release. Sorry about that--there were indeed two paths and I hadn't set the voice on one of them. Thanks for the report!

@Bear-03
Copy link
Contributor Author

Bear-03 commented Jul 22, 2022

Setting the voice works in the main branch, but getting it doesn't. If I call Tts::voice, without calling Tts::speak first, the old voice will be returned.

Maybe it would make more sense for Tts::voice to return self.voice instead of calling self.synth.Voice(), although I think calling self.synth.SetVoice() when Tts::set_voice is called would be better, as it avoids having to store the Voice in the struct.

@ndarilek
Copy link
Owner

ndarilek commented Jul 22, 2022 via email

@Bear-03
Copy link
Contributor Author

Bear-03 commented Jul 22, 2022

I'll open a PR, it's a quick fix :P. Is there any technical reason why you chose to store the voice, rate, pitch and volume in the struct itself? As I see it, calling the methods in SpeechSynthesizer directly would reduce the complexity of the struct, and also prevent errors like this one.

@ndarilek
Copy link
Owner

ndarilek commented Jul 25, 2022 via email

@Bear-03
Copy link
Contributor Author

Bear-03 commented Jul 25, 2022

Isn't it possible to call all of these in their respective set methods, instead of doing it in every Tts::speak call?

self.synth.Options()?.SetSpeakingRate(self.rate.into())?;
self.synth.Options()?.SetAudioPitch(self.pitch.into())?;
self.synth.Options()?.SetAudioVolume(self.volume.into())?;
self.synth.SetVoice(&self.voice)?;

You would still have to store the voice because there is no way to convert from Voice to VoiceInformation in set_voice, but could remove self.pitch, self.rate and self.volume because you could pass the properties directly to the synth like this:

fn get_volume(&self) -> std::result::Result<f32, Error> {
    Ok(self.synth.Options()?.AudioVolume()? as f32)
}

fn set_volume(&mut self, volume: f32) -> std::result::Result<(), Error> {
    self.synth.Options()?.SetAudioVolume(volume as f64)?;
    Ok(())
}

@ndarilek
Copy link
Owner

ndarilek commented Jul 26, 2022 via email

@Bear-03
Copy link
Contributor Author

Bear-03 commented Jul 26, 2022

Great! I'll give it a go :D

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 a pull request may close this issue.

2 participants