-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Multi Channel #1561
Multi Channel #1561
Conversation
@nnegrey , @dizcology , @tswast can you please review this PR? |
speech/cloud-client/beta_snippets.py
Outdated
@@ -157,6 +158,35 @@ def transcribe_file_with_diarization(path): | |||
# [END speech_transcribe_diarization] | |||
|
|||
|
|||
# [START speech_transcribe_multichannel] |
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.
Nit: per our snippet rubric snippets should show concrete values for variables whenever possible. I recommend moving this region tag into the function rather than outside of it. The snippet should also show the necessary imports.
Example:
Note: we comment out the import and client creation in BigQuery because we have ~100 snippets and repeating auth every time slows down the tests substantially, but this is not necessary if there are only a few snippets as there are here.
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.
Good observation @tswast. However all other samples in the speech directory seem to have the region tags outside the functions. So, if we move the region tags inside the function for this sample, it will look inconsistent with the rest of the samples for speech. What do you think?
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.
Better to follow the new standard than to follow the old one. Eventually we will convert all samples to the new standard (starting with translate).
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.
Fair point @tswast, I moved the region tags in beta_snippets.py inside the functions
speech/cloud-client/beta_snippets.py
Outdated
@@ -157,6 +158,35 @@ def transcribe_file_with_diarization(path): | |||
# [END speech_transcribe_diarization] | |||
|
|||
|
|||
# [START speech_transcribe_multichannel] | |||
def transcribe_file_with_multichannel(speech_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.
I recommend showing an example value for speech_file
within the snippet.
# [START speech_transcribe_multichannel]
from google.cloud import speech
client = speech.SpeechClient()
# TODO(developer): Uncomment and set to a path to your audio file.
# speech_file = 'path/to/file.wav'
...
Example:
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.
Good call. I added it (and also for Diarization sample which was merged in yesterday).
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.
A couple nits. LGTM after they are addressed (either by renaming the argument or updating the comment)
speech/cloud-client/beta_snippets.py
Outdated
client = speech.SpeechClient() | ||
|
||
# TODO(developer): Uncomment and set to a path to your audio file. | ||
# speech_file = 'path/to/file.wav' |
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.
In this sample the variable appears to be called "path".
speech/cloud-client/beta_snippets.py
Outdated
client = speech.SpeechClient() | ||
|
||
# TODO(developer): Uncomment and set to a path to your audio file. | ||
# speech_file = 'path/to/file.wav' |
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.
In this sample the variable appears to be called "path".
speech/cloud-client/beta_snippets.py
Outdated
client = speech.SpeechClient() | ||
|
||
# TODO(developer): Uncomment and set to a path to your audio file. | ||
# speech_file = 'path/to/file.wav' |
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.
In this sample the variable appears to be called "path".
speech/cloud-client/beta_snippets.py
Outdated
client = speech.SpeechClient() | ||
|
||
# TODO(developer): Uncomment and set to a path to your audio file. | ||
# speech_file = 'path/to/file.wav' |
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.
In this sample the variable appears to be called "path".
"""Transcribe the given audio file using an enhanced model.""" | ||
# [START speech_transcribe_file_with_enhanced_model] |
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.
I see this is a different way to use the region tags than commonly done in Python so far. Is this the convention going forward?
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.
Yes, this is a result of our most recent samples rubric working group. I'll be presenting on this in a team meeting post-Next.
print('-' * 20) | ||
print('First alternative of result {}'.format(i)) | ||
print(u'Transcript: {}'.format(alternative.transcript)) | ||
print(u'Channel Tag: {}'.format(result.channel_tag)) |
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.
what happens where there are multiple channels? do we get the output from all channels into the same alternative? or does the output from the second channel go to the second alternative, and so on?
os.path.join(RESOURCES, 'Google_Gnome.wav')) | ||
out, err = capsys.readouterr() | ||
|
||
assert 'OK Google stream stranger things from Netflix to my TV' in out |
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 is a single channel audio file, use an actual multichannel test file.
* Implemented the multi channel sample * Added parameter comment * Moved region tags inside the functions * Deleted the extra line * Fixing typos
* Implemented the multi channel sample * Added parameter comment * Moved region tags inside the functions * Deleted the extra line * Fixing typos
* Implemented the multi channel sample * Added parameter comment * Moved region tags inside the functions * Deleted the extra line * Fixing typos
* Implemented the multi channel sample * Added parameter comment * Moved region tags inside the functions * Deleted the extra line * Fixing typos
* Implemented the multi channel sample * Added parameter comment * Moved region tags inside the functions * Deleted the extra line * Fixing typos
* Implemented the multi channel sample * Added parameter comment * Moved region tags inside the functions * Deleted the extra line * Fixing typos
Implemented the multi channel sample