Add support for sending std::vector<uint8_t> type over DC>

[jeffRTC]

Since send method only support std::byte and string types, I have to make a copy to cast into std::byte and if there is a direct support for std::vector it would improve performance. The case is I read from a Stream and send the data through DataChannel and I use convertJSArrayToNumberVector to get close to zero copy from JS side.

it would be also good to remove casting again to char*type when you are passing this back to the JS side since uint8_t type already match with JS Uint8

        auto rawBuffer = emscripten::convertJSArrayToNumberVector<uint8_t>(reader["value"]);

        std::vector<std::byte> buffer(rawBuffer.size() + 1);

        std::transform(rawBuffer.begin(), rawBuffer.end(), buffer.begin(), [] (uint8_t i) { 
            return std::byte(i); 
        });

        if (auto channel = dc.lock())
        {
            channel->send(buffer);
        }

[jeffRTC]

See my edit, I've removed this one Utilities::DocumentToString and it's part of RapidJSON. I wanted to avoid it on the minimal example but forgotten to do so.

ws://paullouisageneau.org:8090/mySocket Example.

[paullouisageneau]

You forgot to mention that you renamed some stuff compared to the example client, like mid to sdpMid...

So I had a look, and I have no idea how to connect it, since it only accepts answers, not offers, yet it sends its signaling information to a remote random id called localID at startup.

You use a global peerConnectionMap (in network) and a global pc pointer at the same time, and you override pc depending of the last incoming message. This doesn't make sense to me. I wouldn't be surprised if a connection was replaced by another one by mistake.

I think you should revise your signaling setup and make it cleaner.

However, there is an error with:

std::string localID =  Utilities::createRandomID(5);
[...]
ws->onOpen([&]() {
[...]
            pc = createPeerConnection(config, ws, localID);
[...]

This can't work properly. localID is defined in the local scope of the method, and you use it in an asynchronous callback by reference. Please pay attention, this is the third time I fix a similar error for you.

[jeffRTC]

Oh yeah, I did rename mid to sdpMid to fix some bug I think.

I think you told me to put RTCTransport network; in global because to prevent it being deconstructed at exit of the main. Similarly, you told me to put pc in global.

[jeffRTC]

Well that's your problem, and it's always the same problem. At the end of main, network will be destroyed, so when the onOpen callback is called, it does not exist anymore, and references on it become invalid.

You must not use things allocated in the main function scope in callbacks, you have to allocate them globally.

https://github.com/paullouisageneau/datachannel-wasm/issues/4

[jeffRTC]

I actually had pc and dc locally but it was you who told me to move it to global.

Issue #4

[jeffRTC]

It actually send an offer to remote peer, I can see it on Websocket logs.

I think the offer happens at,

pc->onLocalDescription([ws, id](rtc::Description description) {
        nlohmann::json message = {{"id", id}, {"type", description.typeString()}, {"description", std::string(description)}};

[jeffRTC]

So it send offer and accept an answer. Simple as that because I don't want reverse way.

This is modeled with client-server style. It's centralized. The goal is to exploit UDP speed.

[jeffRTC]

I've modified ws->onOpen([&]() to ws->onOpen([&, localID]()

[paullouisageneau]

I think you told me to put RTCTransport network; in global because to prevent it being deconstructed at exit of the main. Similarly, you told me to put pc in global.

Sure but that's not the point. The point is, having both peerConnectionMap and pc that you change according to the last received message does not make sense. Either you handle multiple connections properly by id, or just one.

It actually send an offer to remote peer, I can see it on Websocket logs.

As I said, it sends an offer to a remote random id at startup. This can't work with the example signaling server of libdatachannel as you would have to know what id to set on the remote peer beforehand. So you told my to put my own server, but you seem to use a modified version which changed the meaning of the ids.

[jeffRTC]

The random id concept is used to make sure the server knows the thread of client.

It actually has a Non-Random UUID but I deleted it since I need to fix the binary thing before moving forward to threading.

I have attached the arch of the system

But I am puzzled about the ID thing. The client use Websocket to send the Random ID to server then Server do it's thing but the client never have to know the Server ID.

It really works fine with Strings. But I don't think this affect Binary at all.

Even if It did, it should only happen when testing multiple clients. Here I am just testing one client with the server.

[paullouisageneau]

OK but how can I run your code, if I need your custom server? I would like something that I can run on my side without guessing the entire architecture around it. That's what a minimal example is.

[jeffRTC]

The point is, having both peerConnectionMap and pc that you change according to the last received message does not make sense.

I tried to make pc local but the whole thing broke and nothing works. peerConnectionMap is really not global, its a public in a class.

you change according to the last received message

So what should I do here?

I am lost.

I think I do nothing with the map too other than emplacing stuff to it.

[jeffRTC]

OK but how can I run your code, if I need your custom server? I would like something that I can run on my side without guessing the entire architecture around it. That's what a minimal example is.

So you can create a static ID like paullouisageneauPC instead my random and use it on your server beforehand. That's the only change needed.

[paullouisageneau]

I'm not talking about changing globals to locals, I'm taking about having both peerConnectionMap and pc. Just remove peerConnectionMap if you don't use it. By the way, it is global since it is in the global network object.

So you can create a static ID like paullouisageneauPC instead my random and use it on your server beforehand. That's the only change needed.

Sure, that's only one change, but I've made a few changes like this already after guessing how it should work. I'd have liked you to ensure your code actually works outside of your setup, that's what I meant by a minimal self-contained example.

[jeffRTC]

Well no, if I remove, peerConnectionMap it breaks the ability of client to set the remote description, remote candidates.

If I remove pc, it breaks the ability of client to start the PeerConnection and DC.

I used everything exactly as it's from your example except the localID part.

But you are expecting a bug free version instead trying to look for the binary bug. It's not possible. I've did my best here.

You already have a server. You can start it with a static ID and send whatever binary. It will not able to see it.

The remote peer is a server not some any other browser.

Most of bugs in my code doesn't affect the ability of sending binary message. As I have already checked with strings, It works.

[jeffRTC]

I made a JavaScript version and it works, I see binary on server.

var pc = new RTCPeerConnection({
    iceServers: [
        { 'url': 'stun:stun.services.mozilla.com:3478' }
    ]
});

var ws = new WebSocket('ws://' + "YOURIP" + ':PORT/whatevernamegoeshere');
var dc = pc.createDataChannel("my channel");

let peerID = "Bill";

dc.onmessage = function (event) {
    console.log("received: " + event.data);
};

dc.onopen = async function () {
    console.log("datachannel open");

    let response = await fetch("https://webhook.site/0f1dd8a0-9cdf-4a73-8307-8d49520bfbd6");

    let stream = response.body.getReader();

    while(true) {
        let reader = await stream.read();

        if(reader.done) {
            console.log("Stream has been finished");
            break;
        }

        console.log(dc.bufferedAmount)

        dc.send(reader.value);

        console.log(dc.bufferedAmount)
    }
};

dc.onclose = function () {
    console.log("datachannel close");
};

dc.error = function () {
    console.log("datachannel close");
};

pc.onicecandidate = function (event) {
    if (!event || !event.candidate) return;

    ws.send(JSON.stringify({
        type: 'candidate',
        id: peerID,
        mid: event.candidate.sdpMid,
        candidate: event.candidate.candidate
    }));
};

ws.onopen = async function () {
    try {

        let offer = await pc.createOffer();

        await pc.setLocalDescription(offer);

        offer = offer.toJSON();

        ws.send(JSON.stringify({
            type: offer.type,
            id: peerID,
            description: offer.sdp
        }));

    } catch (e) {
        console.log(e);
    }
}

ws.onmessage = async function (input) {
    try {
        var message = JSON.parse(input.data);

        if (message.type == "offer") {
            var offer = new RTCSessionDescription(message);

            await pc.setRemoteDescription(offer);

            let answer = await pc.createAnswer();

            await pc.setLocalDescription(answer);

            answer = answer.toJSON();

            ws.send(JSON.stringify({
                type: answer.type,
                id: peerID,
                description: answer.sdp
            }));
        } else if (message.type == "answer") {
            let answer = new RTCSessionDescription(message);

            await pc.setRemoteDescription(answer);

            ws.send(JSON.stringify({
                type: "getCandidates",
                id: peerID
            }));

        } else if (message.type = "candidate") {
            try{
                await pc.addIceCandidate(message);
            } catch (err) {
                console.log(err)
            }
        }
    } catch (e) {
        console.log(e);
    }
}

[jeffRTC]

The buffered amount is 19 in my JS version!!!

It looks like something wrong with your wrapper.

Maybe if you can test void RTCTransport::worker part isolated with other parts, it would perhaps give you a clue.

Perhaps you can add void RTCTransport::worker to your own code and test

[jeffRTC]

Okay.

I made a hook on your JS wrapper,

        rtcSendMessage: function(dc, pBuffer, size) {
            var dataChannel = WEBRTC.dataChannelsMap[dc];
            if(dataChannel.readyState != 'open') return 0;
            if(size >= 0) {
                var heapBytes = new Uint8Array(Module['HEAPU8'].buffer, pBuffer, size);

                console.log(heapBytes.buffer);

                dataChannel.send(heapBytes.buffer);

                return size;
            } 

Now, it logs ArrayBuffer with 16777216 size.

I didn't caught this before because I trusted on the size your wrapper provides me.

[jeffRTC]

Funny enough, I converted this ArrayBuffer to text.

it seriously logs source code of WASM. I see parts...and other texts are just Unicode symbols.

[jeffRTC]

Looking at what I am seeing here,

Captured Args of new Uint8Array(Module['HEAPU8'].buffer, pBuffer, size) follows

Module['HEAPU8'].buffer size = 16777216; pBuffer value (This is actually value not a buffer) = 5285952 size value = 19

But it looks like this operation fails and resulting re-assignment of Module['HEAPU8'].buffer to var heapBytes thus leaving the size of var heapBytes as 16777216

[jeffRTC]

The actual function signature for Uint8Array is here

https://apireference.aspose.com/html/net/aspose.html/uint8array/constructors/2

Uint8Array Constructor (ArrayBuffer, Int32, Int32)

Do you seriously pass Int32 type value to this method?

[paullouisageneau]

Well no, if I remove, peerConnectionMap it breaks the ability of client to set the remote description, remote candidates. If I remove pc, it breaks the ability of client to start the PeerConnection and DC.

Remove it, also remove the local PeerConnection in onMessage, and use the global PeerConnection everywhere. You handle a single PeerConnection, only one global shared pointer will work fine.

I used everything exactly as it's from your example except the localID part. But you are expecting a bug free version instead trying to look for the binary bug. It's not possible. I've did my best here.

And mid. This is fine but I can't spend hours trying to reverse engineer what you changed without mentioning it.

Most of bugs in my code doesn't affect the ability of sending binary message. As I have already checked with strings, It works.

Well the local id passed by reference introduced a memory corruption that would send garbage, so it does affect the ability of sending messages. When I tried your code I got an exception from the nlohmann json conversion because the string was not valid UTF-8. You just got lucky on your machine somehow.

I made a hook on your JS wrapper,

      rtcSendMessage: function(dc, pBuffer, size) {
          var dataChannel = WEBRTC.dataChannelsMap[dc];
          if(dataChannel.readyState != 'open') return 0;
          if(size >= 0) {
              var heapBytes = new Uint8Array(Module['HEAPU8'].buffer, pBuffer, size);

              console.log(heapBytes.buffer);

              dataChannel.send(heapBytes.buffer);

              return size;
          } 

Now, it logs ArrayBuffer with 16777216 size.

I didn't caught this before because I trusted on the size your wrapper provides me.

This is expected, buffer is the wrapped buffer, so the wasm memory.

Funny enough, I converted this ArrayBuffer to text.

it seriously logs source code of WASM. I see parts...and other texts are just Unicode symbols.

Yes this is normal, since it is the entire wasm memory.

Looking at what I am seeing here,

Captured Args of new Uint8Array(Module['HEAPU8'].buffer, pBuffer, size) follows

Module['HEAPU8'].buffer size = 16777216; pBuffer value (This is actually value not a buffer) = 5285952 size value = 19

But it looks like this operation fails and resulting re-assignment of Module['HEAPU8'].buffer to var heapBytes thus leaving the size of var heapBytes as 16777216

Actually this is fine, the error is with

dataChannel.send(heapBytes.buffer);

It should be

dataChannel.send(heapBytes);

This is fixed in https://github.com/paullouisageneau/datachannel-wasm/commit/f00a7c59678dab6b354e12018a5feda1a3edcc4f Tell me if it solves your issue.

[jeffRTC]

Thank for the Fix. Actually, I told you that I no longer pass localID by reference.

I've modified ws->onOpen([&]() to ws->onOpen([&, localID]()

[jeffRTC]

I can confirm the Fix has fixed the issue.

[jeffRTC]

I also deleted the map thing.

Waiting for you to confirm if my local id thing is correct or not

[paullouisageneau]

I also deleted the map thing.

Waiting for you to confirm if my local id thing is correct or not

Yes if you copy it, it's correct.

[paullouisageneau]

I'm closing this is since the original issue is solved.