-
Notifications
You must be signed in to change notification settings - Fork 5
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 support for speech rate multiplier #136
Conversation
WalkthroughThis update introduces a speech rate feature across the application. A new property for speech rate is added to both the voice metadata and the text-to-speech request data transfer objects. The cloning logic in the voice metadata model now preserves this property. The text-to-speech service forwards the speech rate when requesting audio. A new converter and UI control in the general configuration view allow users to select and display the speech rate, while the view model manages allowed speech rates and applies the configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant V as GeneralConfigView (UI)
participant VM as GeneralConfigViewModel
participant M as AtisVoiceMeta Model
participant TTS as TextToSpeechService
U->>V: Select speech rate from ComboBox
V->>VM: Update SelectedSpeechRate
VM->>M: Validate and apply new SpeechRate
VM->>TTS: Include SpeechRate in TTS request
TTS-->>VM: Process updated audio request
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ Context from checks skipped due to timeout of 90000ms (7)
🔇 Additional comments (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
vATIS.Desktop/Ui/Converters/SpeechRateMultiplierConverter.cs (2)
20-28
: Add null check and culture-aware formatting.The Convert method could be more robust:
- Add explicit null handling
- Use culture-aware number formatting
public object? Convert(object? value, Type targetType, object? parameter, CultureInfo culture) { + if (value is null) + return null; + if (value is double rate) { - return $"{rate}x"; // Format as 0.5x, 1.0x, etc. + return $"{rate.ToString(culture)}x"; // Format as 0.5x, 1.0x, etc. } return value; }
38-47
: Enhance ConvertBack method's error handling.The ConvertBack method could be more robust in handling invalid input formats.
public object? ConvertBack(object? value, Type targetType, object? parameter, CultureInfo culture) { + if (value is null) + return null; + // If converting back (e.g., from "0.5x" to 0.5), remove the "x" and parse the number. - if (value is string rateString && double.TryParse(rateString.Replace("x", ""), out var result)) + if (value is string rateString && + double.TryParse(rateString.Replace("x", "").Trim(), culture, out var result)) { return result; } return value; }vATIS.Desktop/Ui/AtisConfiguration/GeneralConfigView.axaml (1)
119-120
: Consider extracting duplicate tooltip text.The same tooltip text appears in both the TextBlock and ComboBox. Consider moving it to a resource or constant to avoid duplication and make maintenance easier.
+ <UserControl.Resources> + <x:String x:Key="SpeechRateTooltip">Adjust the speech speed. Choose from: 0.5x (slowest), 0.75x (slower), 1x (normal), 1.25x (faster), and 1.5x (fastest).</x:String> + </UserControl.Resources> ... - <TextBlock VerticalAlignment="Center" ToolTip.Tip="Adjust the speech speed. Choose from: 0.5x (slowest), 0.75x (slower), 1x (normal), 1.25x (faster), and 1.5x (fastest).">Speech Rate Multiplier:</TextBlock> + <TextBlock VerticalAlignment="Center" ToolTip.Tip="{StaticResource SpeechRateTooltip}">Speech Rate Multiplier:</TextBlock> - <ComboBox Width="100" Classes="Dark" ItemsSource="{Binding SpeechRates, DataType=vm:GeneralConfigViewModel}" SelectedValue="{Binding SelectedSpeechRate, DataType=vm:GeneralConfigViewModel}" IsEnabled="{Binding UseTextToSpeech, DataType=vm:GeneralConfigViewModel, FallbackValue=False}" ToolTip.Tip="Adjust the speech speed. Choose from: 0.5x (slowest), 0.75x (slower), 1x (normal), 1.25x (faster), and 1.5x (fastest)."> + <ComboBox Width="100" Classes="Dark" ItemsSource="{Binding SpeechRates, DataType=vm:GeneralConfigViewModel}" SelectedValue="{Binding SelectedSpeechRate, DataType=vm:GeneralConfigViewModel}" IsEnabled="{Binding UseTextToSpeech, DataType=vm:GeneralConfigViewModel, FallbackValue=False}" ToolTip.Tip="{StaticResource SpeechRateTooltip}">vATIS.Desktop/Ui/ViewModels/AtisConfiguration/GeneralConfigViewModel.cs (3)
29-29
: Avoid duplicating speech rate values.The same speech rate values are defined in both
s_allowedSpeechRates
andSpeechRates
. Consider using the static array to initialize the property to maintain consistency.- public ObservableCollection<double> SpeechRates { get; } = [0.5, 0.75, 1.0, 1.25, 1.5]; + public ObservableCollection<double> SpeechRates { get; } = new(s_allowedSpeechRates);Also applies to: 255-255
400-407
: Consider optimizing speech rate validation.The current validation uses
Array.Exists
with floating-point comparison. Consider using LINQ'sContains
with a HashSet for more efficient validation.+ private static readonly HashSet<double> s_allowedSpeechRatesSet = new(s_allowedSpeechRates); ... - SelectedStation.AtisVoice.SpeechRateMultiplier = Array.Exists(s_allowedSpeechRates, - rate => Math.Abs(rate - SelectedSpeechRate) < 0.0001) + SelectedStation.AtisVoice.SpeechRateMultiplier = s_allowedSpeechRatesSet.Contains(SelectedSpeechRate) ? SelectedSpeechRate : 1.0; // If not valid, set to 1.0
446-449
: Consider optimizing speech rate initialization.Similar to the validation in
ApplyConfig
, consider using a HashSet for more efficient initialization of the speech rate.- SelectedSpeechRate = Array.Exists(s_allowedSpeechRates, - rate => Math.Abs(rate - station.AtisVoice.SpeechRateMultiplier) < 0.0001) + SelectedSpeechRate = s_allowedSpeechRatesSet.Contains(station.AtisVoice.SpeechRateMultiplier) ? station.AtisVoice.SpeechRateMultiplier : 1.0; // Default value if the rate is not allowed
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
vATIS.Desktop/Profiles/Models/AtisVoiceMeta.cs
(2 hunks)vATIS.Desktop/TextToSpeech/TextToSpeechRequestDto.cs
(1 hunks)vATIS.Desktop/TextToSpeech/TextToSpeechService.cs
(1 hunks)vATIS.Desktop/Ui/AtisConfiguration/GeneralConfigView.axaml
(3 hunks)vATIS.Desktop/Ui/Converters/SpeechRateMultiplierConverter.cs
(1 hunks)vATIS.Desktop/Ui/ViewModels/AtisConfiguration/GeneralConfigViewModel.cs
(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: Analyze (cpp)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (cpp)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (8)
vATIS.Desktop/TextToSpeech/TextToSpeechRequestDto.cs (1)
28-31
: LGTM! Well-documented property addition.The new
SpeechRateMultiplier
property is appropriately defined as nullable and follows the established documentation pattern.vATIS.Desktop/Profiles/Models/AtisVoiceMeta.cs (2)
23-26
: LGTM! Well-structured property with appropriate default.The
SpeechRateMultiplier
property is well-documented and has a sensible default value of 1.0 for normal speed.
38-38
: LGTM! Property correctly included in Clone method.The
SpeechRateMultiplier
is properly handled in the Clone method, ensuring complete object copying.vATIS.Desktop/TextToSpeech/TextToSpeechService.cs (2)
77-77
: LGTM! Speech rate multiplier properly forwarded.The speech rate multiplier is correctly forwarded from the station's ATIS voice configuration to the request DTO.
40-40
:❓ Verification inconclusive
Verify C# version compatibility for collection expression syntax.
The code uses collection expression syntax (
[]
) which requires C# 12.0. Ensure your build environment supports this feature.
🏁 Script executed:
#!/bin/bash # Check .csproj file for language version fd -e csproj . --exec grep -l "LangVersion"Length of output: 43
Confirm Build Environment Supports C# 12.0 Collection Expressions
It appears that no explicit
<LangVersion>
is set in any of the csproj files. Since the collection expression syntax (VoiceList = [];
) requires C# 12.0, please verify manually that your build environment (and the associated .NET SDK) indeed supports C# 12.0 features. If necessary, update the csproj file with an explicit language version (e.g.,<LangVersion>12.0</LangVersion>
) to eliminate any ambiguity.vATIS.Desktop/Ui/AtisConfiguration/GeneralConfigView.axaml (2)
14-15
: LGTM!The converter registration is properly added and maintains consistency with existing patterns.
118-127
: LGTM! Well-designed UI controls.The speech rate controls are well-implemented with:
- Clear tooltips explaining available options
- Proper binding to ViewModel properties
- Consistent disabling behavior with text-to-speech toggle
vATIS.Desktop/Ui/ViewModels/AtisConfiguration/GeneralConfigViewModel.cs (1)
260-271
: LGTM!The
SelectedSpeechRate
property implementation follows the established pattern and correctly handles change notifications and unsaved changes tracking.
Summary by CodeRabbit