add simple nodejs example

[dsteinman]

Added a simple example using NodeJS bindings, single script, single wav file.

[ghost]

No Taskcluster jobs started for this pull request

The `allowPullRequests` configuration for this repository (in `.taskcluster.yml` on the
default branch) does not allow starting tasks for this pull request.

[lissyx]

@dsteinman Thanks for that, though I don't see a huge difference from the native_client/javascript/client.js, besides removal of the argparse part.

[lissyx]

@dsteinman Would you mind elaborating a bit on the added value of this ?

[dsteinman]

The code in the native client directory is not really a proper example that someone can quickly download and run. Fortunately the devs who answered my questions were able to point me in the right direction and get it working:

#1898

So I've simplified the code into a single example file with no build process and no external dependencies (except deepspeech and the model files). The command line arguments aren't necessary for a simple example because it's very likely that a NodeJS developer is building a web service rather than a command line tool, so the args have to be stripped out anyways.

I also published the example code here if you don't want to include it in the project:

https://github.com/dsteinman/deepspeech-simple

[lissyx]

@dsteinman I don't want to hurt anyone's feeling, but it's really exactly the same code. Again, except the removal of argparse and loading of the LM, it's just copy / paste, so I'm really having a hard time understanding how it improves understanding how to use the libdeepspeech bindings.

[dsteinman]

No worries, it was suggested by someone in the thread to make a pull request. But feel free to close it if it's unnecessary.

[lissyx]

No worries, it was suggested by someone in the thread to make a pull request. But feel free to close it if it's unnecessary.

Yeah, I even suggested sending a PR, I'm just wondering what's the added value right now, besides being another set of examples to maintain. Given it looks to be exactly the same code as the client.js in the bindings, I'm unsure it's really useful. Maybe I am missing something? Or maybe writing it helped you?

[dsteinman]

it looks to be exactly the same code as the client.js in the bindings

Have you ever actually tried to download client.js, just start completely from scratch, and write down each thing you need to do to get it working? Although it may seem obvious to those who are already familiar with DeepSpeech, it was not obvious to me. When I looked at the code, it wasn't clear whether compiling the code in /native_client was necessary and it didn't make sense why an NPM package would also require me to download and compile the same code just to get a simple example working.

There are a few important steps of work required between npm install deepspeech downloading client.js and actually get it working. So my pull request reflects the changes I had to make.

[lissyx]

it looks to be exactly the same code as the client.js in the bindings

Have you ever actually tried to download client.js, just start completely from scratch, and write down each thing you need to do to get it working? Although it may seem obvious to those who are already familiar with DeepSpeech, it was not obvious to me.

Can you understand that I'm asking questions in a way that is not to trick or diminish the value of your PR ? As you said, we are very used to the code and it's harder for us to have a newcomer point of view.

What is unclear to me is how complex it is to follow our current codebase. This native_client/javascript/ is only here to produce the deepspeech npm package. That's why there's all the logic. But the content of client.js also documents how to use it, since it's making use of it.

So, again, I'm asking because besides the argparse changes, as expected, your example is really very close to the same code (which is expected), and thus I don't understand how it improves the first newcomer experience.

When I looked at the code, it wasn't clear whether compiling the code in /native_client was necessary and it didn't make sense why an NPM package would also require me to download and compile the same code just to get a simple example working.

There are a few important steps of work required between npm install deepspeech downloading client.js and actually get it working. So my pull request reflects the changes I had to make.

Right, so maybe we should rather improve our documentation ?

Don't get me wrong, I like PR and I like that you took time to send it and discuss it, and I'd really like to understand what's missing / unclear in our current codebase.

Maybe @reuben or @kdavis-mozilla could weight their opinion or maybe they understand it better than I do?

[dsteinman]

I'd really like to understand what's missing / unclear in our current codebase.

What's missing is what "exactly" to do with "client.js". There should be a step-by-step list of commands to get the code running. Getting the example code running should be the first step of any documentation.

You keep referring to how similar the code is, but what's different is the procedure to install everything and run it is simple and straightfoward:

git clone https://github.com/dsteinman/deepspeech-simple.git
cd deepspeech-simple
pip3 install deepspeech
wget https://github.com/mozilla/DeepSpeech/releases/download/v0.4.1/deepspeech-0.4.1-models.tar.gz
tar xvfz deepspeech-0.4.1-models.tar.gz
brew install sox
npm install
node index.js

No command line arguments, no editing, and it has a test audio.wav file already there. Anyone who follows these procedures can get it working and can immediately begin changing it to do whatever they want.

But now try to do the same thing with the native_client code client.js:

https://github.com/mozilla/DeepSpeech/blob/master/native_client/javascript/client.js

If I try to run it with node I get an error:

node client.js

node client.js 
module.js:550
    throw err;
    ^

Error: Cannot find module 'sox-stream'
    at Function.Module._resolveFilename (module.js:548:15)
    at Function.Module._load (module.js:475:25)
    at Module.require (module.js:597:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (.../client.js:4:13)
    at Module._compile (module.js:653:30)
    at Object.Module._extensions..js (module.js:664:10)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)

So right off the bat I am starting with what appears like "broken" code and I have to figure out what code does and how to "fix" it. And there's 120 lines of code to look through, links to other things, it has an environment script #!/usr/bin/env and references to command line arguments, and I have to install various packages like sox-stream, node-wav, and I have to replace the reference to const Ds = require('./index.js'); with an import of deepspeech.

It will take a great deal more than 8 commands to get it running, and every developer who tries the Deepspeech module will have to know or figure out what to do.

[lissyx]

What's missing is what "exactly" to do with "client.js". There should be a step-by-step list of commands to get the code running. Getting the example code running should be the first step of any documentation.

Well, again, because client.js is here to produce deepspeech npm package, as well as serve as an example, not meant to be run by hand.

So right off the bat I am starting with what appears like "broken" code and I have to figure out what code does and how to "fix" it. And there's 120 lines of code to look through, links to other things, it has an environment script #!/usr/bin/env and references to command line arguments, and I have to install various packages like sox-stream, node-wav, and I have to replace the reference to const Ds = require('./index.js'); with an import of deepspeech.

It will take a great deal more than 8 commands to get it running, and every developer who tries the Deepspeech module will have to know or figure out what to do.

And if you npm install deepspeech you get the bindings installed and the deps as well: https://github.com/mozilla/DeepSpeech/blob/master/README.md#using-the-nodejs-package

And we refer there to client.js as an example of code.

[lissyx]

There should be a step-by-step list of commands to get the code running

And we have that on the front page / README: https://github.com/mozilla/DeepSpeech/blob/master/README.md#getting-the-pre-trained-model https://github.com/mozilla/DeepSpeech/blob/master/README.md#using-the-model

[dsteinman]

Well, again, because client.js is here to produce deepspeech npm package, as well as serve as an example, not meant to be run by hand.

If that code cannot be run by hand, then why is it being provided as the starting point for newcomers? This is precisely why I was confused. It's like a catch-22 -- do I install the NPM module or do I need to build the module. It's impossible for a newcomer to know where to start because the procedure to run the code in that client.js file (or even what to do with it) is not provided.

[lissyx]

If that code cannot be run by hand, then why is it being provided as the starting point for newcomers? This is precisely why I was confused. It's like a catch-22 -- do I install the NPM module or do I need to build the module. It's impossible for a newcomer to know where to start because the procedure to run the code in that client.js file (or even what to do with it) is not provided.

Again, https://github.com/mozilla/DeepSpeech/blob/master/README.md#using-the-nodejs-package we document that exactly, we say the client.js file is an example of how to use the bindings. That should precisely address the use-case you describe.

Did you saw that when you struggled ? If so, then I guess we could welcome improvement on the wording if you think it's not clear enough or misleading.

[dsteinman]

I'm not sure what else I can say other than the docs were not sufficient for me. Perhaps close the PR, and revisit at a later time if other developers have the same problems.

[lissyx]

I'm not sure what else I can say other than the docs were not sufficient for me.

Sorry if I feel to insist, but we are more than open to improvements on the docs, if I felt this was a useless request I would have closed the PR.

So if you say they were not sufficient, could you elaborate based on the links above ? Those were really put together with newcomer in mind, so if it's not clear, we are failing somehow, and it's not good.

Perhaps close the PR, and revisit at a later time if other developers have the same problems.

Except others may not take the time to give feedback, that's why I really want to understand what's missing / unclear in our current docs, because everything you explained so far is, at least from my point of view and understanding, covered by the docs. Maybe not at the place you would have expected, maybe wording unclear, maybe overwhelmed by too much infos, all those are understandable.

But you took time to test, to give feedback, so it's really important we avoid wasting your time.

[dsteinman]

at least from my point of view and understanding, covered by the docs.

There is no documentation, and the example provided doesn't work as-is.

npm install deepspeech
See client.js for an example of how to use the bindings.

That's it. That's the entire extent of the NodeJS documentation. And to a new user some "unknown" amount of work will be required to modify that client.js and figure out how to get it working. And who knows if they will succeed, or that it's possible to get it working?

It would be a lot more re-assuring to download code that is known to work, and has a clear step-by-step procedure, and then use that as the starting point. Right now, there is no starting point, the developers are forced to derive their own example based on one that they probably don't have working and maybe aren't sure they will be able to get working.

npm install deepspeech
wget https://github.com/mozilla/DeepSpeech/blob/master/native_client/javascript/client.js
[INSERT INSTRUCTIONS HERE]

If it's as easy as you think, why not try to complete the rest of instructions such that anyone can download it as-is, and run it, in as few steps as possible?

[lissyx]

at least from my point of view and understanding, covered by the docs.

There is no documentation, and the example provided doesn't work as-is.

npm install deepspeech
See client.js for an example of how to use the bindings.

That's it. That's the entire extent of the NodeJS documentation. And to a new user some "unknown" amount of work will be required to modify that client.js and figure out how to get it working. And who knows if they will succeed, or that it's possible to get it working?

Okay, now I get what you struggled with

It would be a lot more re-assuring to download code that is known to work, and has a clear step-by-step procedure, and then use that as the starting point. Right now, there is no starting point, the developers are forced to derive their own example based on one that they probably don't have working and maybe aren't sure they will be able to get working.

npm install deepspeech
wget https://github.com/mozilla/DeepSpeech/blob/master/native_client/javascript/client.js
[INSERT INSTRUCTIONS HERE]

If it's as easy as you think, why not try to complete the rest of instructions such that anyone can download it as-is, and run it, in as few steps as possible?

Well, that's where you feedback is useful, we did not got into thinking of that wget step, for us it was obvious "read the code and adapt".

[lissyx]

LGTM, but you need to remove the useless pip step, and it would be better you link your example doc from the main README.md, after the mention of client.js, so that others can find it easily :)

[dsteinman]

Okay I'm glad I was able to convince you this was worthwhile :) I added a link to the Readme as well.

[lissyx]

Sorry, a few things that seems not right :)

[lissyx]

LGTM, thanks! 👍

[lissyx]

@dsteinman Could you squash all commits into one before I merge? Thanks!

[dsteinman]

@dsteinman Could you squash all commits into one before I merge? Thanks!

No problem, it's done.

BTW I have another example file that I made based on that one which turns on the microphone and feeds the recording into DeepSpeech. Is that something you might want in the examples also? It would just add one more JavaScript file and one additional dependency.

[lissyx]

BTW I have another example file that I made based on that one which turns on the microphone and feeds the recording into DeepSpeech. Is that something you might want in the examples also? It would just add one more JavaScript file and one additional dependency.

I know we have some doing the same with Python and I think one with NodeJS + ffmpeg, but if yours adds any different value it's welcome.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.