Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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 :)

# for free to join this conversation on GitHub. Already have an account? # 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