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

binary data is not transmitted entirely #895

Closed
raminrad opened this issue Nov 16, 2016 · 5 comments
Closed

binary data is not transmitted entirely #895

raminrad opened this issue Nov 16, 2016 · 5 comments

Comments

@raminrad
Copy link

I am using WS version 1.1.1 and sending audio data (raw PCM) to a browser. But the number of bytes I receive on the client side (after I reassemble all the data in WS messages) is not equal to the original file size (in bytes). I am not sure if this is an issue with the code or the way I am using it.

             var pcmStream = fs.createReadStream(
                'data/win.pcm',
                {
                    'flags': 'r',
                    'encoding': 'binary',
                    'bufferSize': size
                });
            pcmStream.on('data', function(data) {
                ws.send(data, {'binary': true, mask: false});
            });

@lpinca
Copy link
Member

lpinca commented Nov 16, 2016

Hi,
can you try to disable the permessage-deflate extension and see if the issue still happens?

const wss  = new WebSocket.Server({
  perMessageDeflate: false,
  port: 8080
});

With current version (1.1.1) you can also pass a readable stream to WebSocket.prototype.send() and it should handle it for you. On the receiving end you'll get a single message so you don't have to rebuild the message yourself.

We are currently discussing if this feature should be removed and let the developer handle this, see #875 (comment).

The added benefit of the example posted there is that backpressure is correctly handled. Please note that it doesn't work with 1.1.1 as the fin option is overridden. If you want to try it you can install ws from the master branch:

npm install websockets/ws

@raminrad
Copy link
Author

Thank you for responding. After further investigation, I found that the server is in fact sending the correct number of bytes, and the issue is in fact in the browser, where the audio data is stored in memory and some of the bytes are word-aligned, and I have to unpack the data.

I have posted this on stackoverflow for additional help. http://stackoverflow.com/questions/40657827/

Sorry for suspecting that the issue was with your code.

@lpinca
Copy link
Member

lpinca commented Nov 17, 2016

@raminrad why are you using the binary encoding in fs.createReadStream() options? By doing that your chunks of data are strings encoded as latin1.

Those chunks are then converted to buffers using the utf-8 encoding in ws.send() which could be the cause of your issue.

TL;DR try to remove the encoding option when you use fs.createReadStream() so each chunk of data is a buffer.

@raminrad
Copy link
Author

You are absolutely right! I switched to a different Web Socket node package and the person on stack overflow suggested that I pass the 'binary' encoding to Buffer, and when I did that, it fixed the problem. But after reading what you wrote, I removed the encoding fromfs.createReadStream and also from Buffer and that ALSO worked! Thank you for taking the time to review this. I am switching back to your package. It's so simple to use. Great job and thank you again for your attention.

@lpinca
Copy link
Member

lpinca commented Nov 18, 2016

You have to send binary data so there is no reason to convert each chunk to a string and then back to a buffer again when sending the data.

The added benefit of using buffers directly is that performance is better. In your use case you are sending data from server to client so that data doesn't need to be masked.

You have the best possible scenario where data is a buffer and doesn't need to be copied :)

P.S. I looked at the SO post and noticed this line

conn.sendBytes(new Buffer(data));

You can avoid creating a new buffer from a buffer as that is only a useless copy.

Closing this, please comment or reopen if needed.

@lpinca lpinca closed this as completed Nov 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants