Skip to content

Conversation

@dpopp07
Copy link
Contributor

@dpopp07 dpopp07 commented Nov 29, 2018

Adds a new method - synthesizeUsingWebSocket - that returns a ReadableStream that the user can pipe into a new file or over the speakers. Returning a stream seemed to be the best user experience since the point of websocket is to allow the user to receive data in chunks in real time without reading all data into memory at once.

I tried to keep the design similar to recognizeUsingWebSocket for a consistent experience, but did not adopt any code that seemed unnecessary.

There might be more features that are needed but this provides the basic needs of the user with simple code and a simple interface.

I still need to write tests and an example but wanted to start getting feedback on the implementation.

cc @ehdsouza @jsstylos @jeffpk62

Todo:

  • Write tests
  • Write an example

This is what the experience looks like for the user:

const tts = new TTS({
  username,
  password,
});

const params = {
  text: 'Hello world.',
  accept: 'audio/ogg;codecs=opus',
};

// create the stream
const synthStream = tts.synthesizeUsingWebSocket(params);

// pipe the stream into a new audio file
synthStream.pipe(fs.createWriteStream('./speech.ogg'));

// optionally, the user can listen to data as it is returned from the service
synthStream.on('message', (message, data) => {
  console.log(data);
});

@germanattanasio
Copy link
Contributor

How can you see when the websocket was closed? can we have an on('close')?

@dpopp07 dpopp07 changed the title feat(text-to-speech): add support for using synthesize feat(text-to-speech): add support for using synthesize - WIP Nov 30, 2018
@dpopp07
Copy link
Contributor Author

dpopp07 commented Nov 30, 2018

There is a close event emitted when the connection closes, on Line 124 of synthesize-stream.ts.

self.emit('close', event.code, event.reason);

I just didn't include it in the demo.

@codecov-io
Copy link

codecov-io commented Dec 3, 2018

Codecov Report

Merging #807 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #807   +/-   ##
=======================================
  Coverage   95.65%   95.65%           
=======================================
  Files           2        2           
  Lines          46       46           
  Branches        2        2           
=======================================
  Hits           44       44           
  Misses          2        2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2f2e96...062179d. Read the comment docs.

@dpopp07 dpopp07 changed the title feat(text-to-speech): add support for using synthesize - WIP feat(text-to-speech): add support for using synthesize Dec 3, 2018
@dpopp07
Copy link
Contributor Author

dpopp07 commented Dec 3, 2018

@germanattanasio documentation, example, and test added

// the 'code' parameter is the status code. 1000 is the code for a normal termination
// the 'reason' parameter provides a string description of how the connection closed
synthesizeStream.on('close', (code, reason) => {
console.log(code);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 🎉

@germanattanasio
Copy link
Contributor

When running in node we use the ws package but do we create a bundle for the browser without it? and is the ws interface the same as the WebSocket interface in a browser?

The questions are not related to this PR which is good

Copy link
Contributor

@germanattanasio germanattanasio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Good job @dpopp07

@dpopp07 dpopp07 merged commit b48a85c into master Dec 6, 2018
@dpopp07 dpopp07 deleted the tts-websockets branch December 6, 2018 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants