medooze / media-server

WebRTC Media Server
GNU General Public License v2.0
1.36k stars 295 forks source link

Support RTMPS client #297

Closed harryz2000 closed 3 months ago

murillo128 commented 5 months ago

Please use CamelCase for method names

murillo128 commented 4 months ago

@mildsunrise ptal

mildsunrise commented 4 months ago

also @harryz2000 sorry for the late review, I'm very bad at organizing my time :_

harryz2000 commented 3 months ago

have not checked the specific code for TLS, the non-tls related changes seems good to me, although I would prefer a class abstracting the client connection and don't have to do if (tls)

That would involve more change, including need to create different instances in the node js code. But that was my original thought too. I would rethink about that.

harryz2000 commented 3 months ago

Created another class RTMPSClientConnection for rtmps

harryz2000 commented 3 months ago

can't we avoid the queuing/dequeing of memory in TlsClient? it is going to kill performance.

I think we would need some sort of buffer to store the data. I will see if I can use a circular buffer. In case of the buffer is full, we may have to drop chunks, which should rarely happen we normally pop them immediately.

harryz2000 commented 3 months ago

I changed to use callbacks instead of queue for performance.