-
Notifications
You must be signed in to change notification settings - Fork 14
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
Adds support for configuring volume filtering #91
Conversation
@@ -230,6 +230,11 @@ def get_transcription_config( | |||
]: | |||
config[option] = True if args.get(option) else config.get(option) | |||
|
|||
if args.get("volume_threshold"): | |||
config["audio_filtering_config"] = { |
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.
can't this be a property in the top level dict?
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.
Do you mean audio_filtering_config
should be a top level property of the whole thing, or that volume_threshold
should be a property in transcription_config
?
If the first, then I think audio filtering is a transcription option (and unlike a standalone option configuring a speech capability)
If the second, it's because it follows the related openapi-spec MR and audio_filtering_config
is open for extension, even though it only has one setting at the moment - the analogy is with ``speaker_diarization_config`.
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.
Resolving since the openapi-specifications change is merged.
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.
LGTM
Supports unreleased volume filtering feature: