meetecho / janode

A Node.js adapter for the Janus WebRTC server
ISC License
98 stars 36 forks source link

Add WebRTC Data Channel Support to streaming-client.js #38

Closed guilherme-marcello closed 8 months ago

guilherme-marcello commented 8 months ago

Description

This pull request adds support for WebRTC data channels in the streaming-client.js file. It enables the creation of a data channel named "channel" in the PeerConnection and includes the necessary event handling for the data channel's onopen and onmessage events. Additionally, binary data received through the data channel is properly decoded using a TextDecoder instance.

Changes Made

Testing

Tested the changes by integrating them with a Janus instance and verified that the data channel works as expected. Verified proper logging of both regular and binary data in the browser console.

atoppi commented 8 months ago

@guilherme-marcello are you still interested in merging this?

guilherme-marcello commented 8 months ago

Hello @atoppi !

Thank you for pointing out areas for improvement with such detail, I totally agree with your suggestions!

I've added the suggested changes into the code, and I believe the enhancements will positively impact the functionality and maintainability of the WebRTC data channel handling in streaming-client.js, as I needed myself during a project.

Thank you once again for your review!

atoppi commented 8 months ago

Sorry if maybe I was not clear enough.

Check how janus.js handles dcs:

https://github.com/meetecho/janus-gateway/blob/master/html/demos/janus.js#L1650

Basically you need to do both things if data are going to be negotiated:

For both channels, set up onopen/message/error/close callbacks with proper logging (local/remote and label)

guilherme-marcello commented 8 months ago

Hello!

Thank you for the explanation. Hope is working properly right now.

Cheers!

atoppi commented 8 months ago

Thanks, looking good now. Have you tested it?

If it's working I'll merge and then fix some small issues (basically linting and variable/function naming) by myself.

guilherme-marcello commented 8 months ago

Yes, it is working; I tested before pushing. Same tests as before, with the multistream version of Janus, checking both regular and binary data in the browser console.