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

Add voice switcher for TTS #77

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

igubanov
Copy link
Contributor

@igubanov igubanov commented Oct 21, 2023

tts engines offer various voices, this change introduces the option to choose which voice to use

src/Translumo.TTS/TtsFactory.cs Outdated Show resolved Hide resolved
src/Translumo.TTS/TtsConfiguration.cs Outdated Show resolved Hide resolved
src/Translumo.TTS/TtsFactory.cs Outdated Show resolved Hide resolved
src/Translumo.TTS/TtsFactory.cs Outdated Show resolved Hide resolved
src/Translumo.TTS/TtsFactory.cs Outdated Show resolved Hide resolved

public void UpdateVoice(IList<string> voices)
{
var currentVoice = voices.Contains(CurrentVoice) ? CurrentVoice : voices.First();
Copy link
Owner

Choose a reason for hiding this comment

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

unused variable?

@@ -146,5 +148,9 @@ lang switch
_ => null
};

public string[] GetVoices() => _voices;

public void SetVoice(string voice) => _voice = _voices.First(x => x.Equals(voice, StringComparison.OrdinalIgnoreCase));
Copy link
Owner

Choose a reason for hiding this comment

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

There is a bug, exception occurred when TTS with voice is selected, then user switch to another TTS. Need to set TtsConfiguration.CurrentVoice after(before) this update.

@@ -34,6 +37,14 @@ public sealed class LanguagesSettingsViewModel : BindableBase, IAdditionalPanelC

public TtsConfiguration TtsSettings { get; set; }

// NOTE: wfp doesnt update combobox source for non-static propertions or properties from non-singletone class 💀, I cant find another workaround
public static ObservableCollection<string> AvailableVoices { get; } = new();
Copy link
Owner

Choose a reason for hiding this comment

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

There is some problem with VM instances... I will investigate it further and come back with update

Copy link
Owner

Choose a reason for hiding this comment

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

I got it. VM has scoped lifetime, but TtsFactory receives another VM instance (out of main scope), so UpdateVoice is called also on that wrong instance. Generally speaking, it the consequence of incorrect dependency (factory shouldn't update external component).
Let's get back to implementation with AvailableVoices in TTSConfiguration :) But with updating outside Factory (in ProcessingService). It's still not the best option though, but I have further plans for more global refactoring in that part VM <--> ProcessingService

@Danily07
Copy link
Owner

Hi, sorry for such long delay with answer. Now I got back to development and left some comments

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.

2 participants