search

shinyoshiaki / werift-webrtc

WebRTC Implementation for TypeScript (Node.js), includes ICE/DTLS/SCTP/RTP/SRTP/WEBM/MP4
MIT License
485 stars 32 forks source link

remove @peculiar/webcrypto as webcrypto has been available since node… #394

Closed koush closed 3 months ago

koush commented 3 months ago

… 15. minimum version of node for werift is 18.

shinyoshiaki commented 3 months ago

The test "negotiationneeded event should not fire if signaling state is not stable" is failing.

This is purely speculative, the cause might be that @peculiar/webcrypto is an async function, but it might be creating the key synchronously in reality.

On the other hand, node-webcrypto actually creating the key asynchronously, which could be causing a timing discrepancy.

If there isn't a strong motivation to remove "@peculiar/webcrypto," it might be better to leave it as is.

koush commented 3 months ago

This is purely speculative, the cause might be that @peculiar/webcrypto is an async function, but it might be creating the key synchronously in reality.

I'm not sure, but that would make sense.

If there isn't a strong motivation to remove "@peculiar/webcrypto," it might be better to leave it as is.

Removing peculiar/webcrypto brings my bundle size down like 300k. Less dependencies, especially in the crypto paths, is preferable in my opinion. Furthermore, if peculiar/webcrypto is in fact synchronous (ie, blocking js main thread), that would mean that each key generation is interrupting sending of RTP packets from other sessions. I'm currently mitigating this issue in general by using per connection workers though.

koush commented 3 months ago

I'll see if I can work out the new timing issues in the tests. It is not having adverse effects in my production code as far as I can see.

koush commented 3 months ago

test is fixed. negotiation was finishing before the next tick that runs the promise.then.