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.

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

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

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
@@ -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.

This comment was marked as spam.

@stephenplusplus stephenplusplus added the api: speech Issues related to the Speech-to-Text API. label Jul 6, 2016
baseUrl: 'speech.googleapis.com',
projectIdRequired: false,
service: 'speech',
apiVersion: 'v1',

This comment was marked as spam.

This comment was marked as spam.

@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.

// We must establish an authClient to give to grpc.
this.getGrpcCredentials_(function(err, credentials) {
if (err) {
setImmediate(function() {

This comment was marked as spam.

This comment was marked as spam.

* }
*
* //-
* // <h3>Run speech detection over a local file</h3>

This comment was marked as spam.

@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

* // Run speech recognition over raw file contents.
* //-
* speech.recognize({
* content: fs.readFileSync('./bridge.raw')

This comment was marked as spam.

@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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

7 participants