-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Ingest Client - Multichannel support added #2186
base: master
Are you sure you want to change the base?
Conversation
@@ -136,6 +136,13 @@ | |||
"description": "A value indicating whether diarization (speaker separation) is requested." | |||
} | |||
}, | |||
"Channels": { | |||
"defaultValue": "1", |
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.
the default value of transcription is [0,1]. i.e. If no channels is specified, both channel 0 and channel 1 of stereo will be transcribed.
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.
It's not what happens if we do not pass Channels in the Transcription request properties , it results with Invalid Data error .Tested on multichannel audios.
"channels": [
0,
1
],
"type": "String", | ||
"allowedValues": [ |
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.
So we support TextAnalytics in any region now?
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.
Please refer to latest release of ARM in this repo yes Endpoint is supported specifically to support Private Endpoints that are not regional. Not sure why latest Tag is not merged in master
@@ -55,5 +55,7 @@ public static class StartTranscriptionEnvironmentVariables | |||
public static readonly string PunctuationMode = Environment.GetEnvironmentVariable(nameof(PunctuationMode), EnvironmentVariableTarget.Process); | |||
|
|||
public static readonly string StartTranscriptionServiceBusConnectionString = Environment.GetEnvironmentVariable(nameof(StartTranscriptionServiceBusConnectionString), EnvironmentVariableTarget.Process); | |||
|
|||
public static readonly int[] Channels = int.TryParse(Environment.GetEnvironmentVariable(nameof(Channels), EnvironmentVariableTarget.Process), out int result) && result == 1 ? Constants.Channels : new int[] { result }; |
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.
Reading channels from environment variables looks weird. Do we ask customers to change the environment variable value for each audio to be transcribed? How about if multiple audios are submitted but with different channel settings?
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.
it's global setting for Azure Function applicable to all audios
@@ -15,7 +15,7 @@ public sealed class TranscriptionDefinition | |||
string description, | |||
string locale, | |||
IEnumerable<string> contentUrls, | |||
Dictionary<string, string> properties, | |||
Dictionary<string, object> properties, |
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.
This looks weird. Why do we need to change this to "object"?
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.
Properties for transcription request not always just string , for example
"properties": {
"diarizationEnabled": false,
"wordLevelTimestampsEnabled": true,
"channels": [
0,
1
],
"punctuationMode": "DictatedAndAutomatic",
"profanityFilterMode": "Masked",
"languageIdentification": {
"candidateLocales": [
"en-US",
"de-DE",
"es-ES"
]
}
},
@@ -30,5 +30,7 @@ public static class Constants | |||
public const int DefaultConversationAnalysisMaxChunkSize = 5000; | |||
|
|||
public const string SummarizationSupportedLocalePrefix = "en"; | |||
|
|||
public static readonly int[] Channels = new int[] { 0, 1 }; |
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.
Not sure that I understand the purpose of this PR. Multichannel support should be enabled by default in batch transcription. Only if a customer wants to transcribe a specific channel, he needs to specify the channel number in request property. For example, he only wants to transcribe channel 1, but not channel 0, of a stereo audio.
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.
Please test the default settings - it always returns "Invalid Data" for multi Channel audios, it does not do that by default.
Purpose
Add MultiChannel Support for audio files in batch transcription
Pull Request Type
What kind of change does this Pull Request introduce?
Support to set MultiChannel transcription feature
#2185