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.

Sign up for GitHub

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

Jump to bottom

speech: initial support #1407

Merged
merged 19 commits into from
Sep 20, 2016
Merged

speech: initial support #1407

merged 19 commits into from
Sep 20, 2016

Conversation

jmdobry
Copy link
Contributor

@jmdobry jmdobry commented Jul 2, 2016 •
edited by stephenplusplus
Loading

Fixes #1406

Add support for the Speech API (v1beta1)!

  • Implementation
    • Speech#recognize (SyncRecognize)
    • Speech#startRecognition (AsyncRecognize)
    • Speech#createRecognizeStream (StreamingRecognize)
  • Docs
  • Tests
    • System
      • Speech#recognize
      • Speech#startRecognition
      • Speech#createRecognizeStream
    • Unit
      • Speech#recognize
      • Speech#startRecognition
      • Speech#createRecognizeStream

@googlebot googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Jul 2, 2016
jgeewax
jgeewax reviewed Jul 4, 2016
View reviewed changes
CONTRIBUTING.md
@@ -23,6 +23,8 @@ To run the system tests, first create and configure a project in the Google Deve
- **GCLOUD_TESTS_KEY**: The path to the JSON key file.
- ***GCLOUD_TESTS_API_KEY*** (*optional*): An API key that can be used to test the Translate API.
- ***GCLOUD_TESTS_DNS_DOMAIN*** (*optional*): A domain you own managed by Google Cloud DNS (expected format: `'gcloud-node.com.'`).
- ***GCLOUD_TESTS_BIGTABLE_ZONE*** (*optional*): A zone containing a Google Cloud Bigtable cluster.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@stephenplusplus stephenplusplus added the api: speech Issues related to the Speech-to-Text API. label Jul 6, 2016
pbogle
pbogle reviewed Jul 7, 2016
View reviewed changes
lib/speech/index.js
baseUrl: 'speech.googleapis.com',
projectIdRequired: false,
service: 'speech',
apiVersion: 'v1',

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@googlebot googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Jul 11, 2016
@stephenplusplus
Copy link
Contributor

Sorry for being quiet on this. It looks great to me so far... I'll dive in deeper asap. Thanks!

@jmdobry
Copy link
Contributor Author

jmdobry commented Jul 12, 2016

Yeah, this is ready for review. I was looking at using something like https://github.com/audiocogs/aurora.js to detect encoding and sampleRate, but after some testing I couldn't get it to work reliably (maybe because the Speech API supports only a small set of encodings?). Also https://github.com/audiocogs/aurora.js requires some native dependencies, which I'm not sure we'd want.

If some day another API also implements the Operations service interface, then we could extract the operations-related code out of the speech code. operation.js is already generally re-usable, it just has speech-specific comments in it. The getOperation method is easily generalizable as well. Maybe the operations code could move into grpc-service.js and be configurable in the service config object during instantiation.

@stephenplusplus
Copy link
Contributor

Assigned to @callmehiphop for a review.

callmehiphop
callmehiphop reviewed Jul 13, 2016
View reviewed changes
lib/common/grpc-service.js
// We must establish an authClient to give to grpc.
this.getGrpcCredentials_(function(err, credentials) {
if (err) {
setImmediate(function() {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

callmehiphop
callmehiphop reviewed Sep 12, 2016
View reviewed changes
packages/speech/src/index.js
* }
*
* //-
* // <h3>Run speech detection over a local file</h3>

This comment was marked as spam.

Sign in to view

@stephenplusplus
Copy link
Contributor

Pulled this out from the now-squashed commit comment:

IMO we should do stuff in this order

  • Merge Speech PR
  • Release Speech API (v0.1.0)
  • Add dependency for Speech on umbrella package
  • Release umbrella package.

@callmehiphop many changes made, PTAL!

@callmehiphop
Copy link
Contributor

Looks like there are still some linting issues lingering about.

@jmdobry
Copy link
Contributor Author

jmdobry commented Sep 19, 2016

Any idea when this will get merged?

@jmdobry
Copy link
Contributor Author

jmdobry commented Sep 19, 2016

Assuming the tests pass after my most recent commit, I think we can:

  1. Merge to master
  2. Cut a 0.1.0 release
  3. Open a new PR that adds the autogen layer for Speech
    1. Re-write hand-written methods as necessary to use the autogen layer instead of the grpc stuff in google-cloud-common
  4. Cut a 0.2.0 once the autogen PR is merged

callmehiphop
callmehiphop reviewed Sep 19, 2016
View reviewed changes
packages/speech/src/index.js
* // Run speech recognition over raw file contents.
* //-
* speech.recognize({
* content: fs.readFileSync('./bridge.raw')

This comment was marked as spam.

Sign in to view

@callmehiphop
Copy link
Contributor

@jmdobry I might have misunderstood, but I thought we agreed to wait until next Monday to cut the release so we could try and get vtk in as well.

@jmdobry
Copy link
Contributor Author

jmdobry commented Sep 19, 2016

Maybe I misunderstand too. Are we talking about just adding the autogen layer, or adding the autogen layer and changing the hand-written layer to use autogen?

@stephenplusplus
Copy link
Contributor

I obviously wasn't at the meeting, but just a thought... if we can get this out now, let's just do that :)

@callmehiphop callmehiphop merged commit d24209a into googleapis:master Sep 20, 2016
@jmdobry jmdobry deleted the speech branch September 28, 2016 17:38
sofisl pushed a commit that referenced this pull request Jan 24, 2023
@jmdobry @callmehiphop
speech: initial support (#1407)
72d0af0
sofisl pushed a commit that referenced this pull request Jan 25, 2023
@jmdobry @callmehiphop
speech: initial support (#1407)
885720d
This was referenced Jul 11, 2023
Open
Open
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@callmehiphop callmehiphop callmehiphop left review comments

Assignees

@callmehiphop callmehiphop

Labels
api: speech Issues related to the Speech-to-Text API. cla: no This human has *not* signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jmdobry @stephenplusplus @jgeewax @coveralls @callmehiphop @googlebot @pbogle