y-webrtc icon indicating copy to clipboard operation
y-webrtc copied to clipboard

extend simple peer (not peerjs, oops) to handle buffered/packet transmission; add raw dependency w/MIT license

Open disarticulate opened this issue 3 years ago • 3 comments

Closes https://github.com/yjs/y-webrtc/issues/20

  • Extends the simple peer class to override the send and _onChannelMessage functions to handle chunked data packets using custom defined packets encodePacket and decodePacket
  • Adds bundled & minified (maybe you want to include it somewhere else) dependency to use integers for header details (8 bytes) as discussed in the issue, numbering packets.

The custom packet setup could probably be simplfied as we're counting each full data (txOrd), each packet sent (index), how many packets (length), the size of the packet's data (chunkSize) and the total size of the data (totalSize)

Ran the tests and get some typescript errors that come from the SimplePeerExtended extends Peer where we're using something from the parent class that isnt available. Also, the minified dependency would either need to be brought in officially, or ignored.

disarticulate avatar May 21 '21 12:05 disarticulate

@disarticulate Is this patch the latest version you've been applying? I see errors when clients disconnect. Those seem to go away, if I include super.destroy() from you main branch.

Beyond that, a supported way to apply customized SimplePeer would be great :pray:.

datakurre avatar Apr 13 '22 07:04 datakurre

@disarticulate Is this patch the latest version you've been applying? I see errors when clients disconnect. Those seem to go away, if I include super.destroy() from you main branch.

Beyond that, a supported way to apply customized SimplePeer would be great 🙏.

Yes. I haven't been developing with it recently, and haven't followed any yjs updates since this was proposed.

check https://github.com/disarticulate/y-webrtc for when it was developed. it looks like simple-peer would need be moved to a peer dependency. I think the behavior is stable, but how it should be integrated takes some considerations that I haven't returned to.

disarticulate avatar Apr 28 '22 16:04 disarticulate

The boring answer is the encoder selected is encoding 64 bit integers.

The arbitrary answer is it made the header simple to decode and encode.

Someone could rightsize it all, some of those values are purely redundant. I considered a system that did packet routing so wanted space.

On Thu, Jul 28, 2022, 17:14 Vitor de Mello Freitas @.***> wrote:

@.**** commented on this pull request.

In src/SimplePeerExtended.js https://github.com/yjs/y-webrtc/pull/25#discussion_r932720000:

+}

+class SimplePeerExtended extends Peer {

  • constructor (opts) {
  • super(opts)
  • this._opts = opts
  • this._txOrdinal = 0
  • this._rxPackets = []
  • this._txPause = false
  • this.webRTCMessageQueue = []
  • this.webRTCPaused = false
  • }
  • encodePacket ({ chunk, txOrd, index, length, totalSize, chunkSize }) {
  • const encoded = concatenate(Uint8Array, [
  •  new Uint8Array(new Int64BE(txOrd).toArrayBuffer()), // 8 bytes
    

@disarticulate https://github.com/disarticulate Why do the values need to be padded to 8 bytes in the header?

— Reply to this email directly, view it on GitHub https://github.com/yjs/y-webrtc/pull/25#discussion_r932720000, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFHWPLDSXMKA2FL3GAULI3VWMA2TANCNFSM45JCHYTA . You are receiving this because you were mentioned.Message ID: @.***>

disarticulate avatar Jul 30 '22 00:07 disarticulate