AirenSoft / OvenLiveKit-Web

OvenLiveKit for Web is a JavaScript-based Live Streaming Encoder
MIT License
49 stars 24 forks source link

connectionClosed callback doesn't work if stream is stopped from client side #4

Open brootle opened 2 years ago

brootle commented 2 years ago

Testing WebRTC at https://ovenplayer.com/docs/demo_input.html

connectionClosed callback is never called when I stop stream on client side. It works when connection is closed from server side.

Obviously socket connections is not getting closed properly from client side as it should happen here https://github.com/AirenSoft/OvenLiveKit-Web/blob/17e7387782bc63485ea6694cda18a2b2d7161ab9/src/OvenLiveKit.js#L367

So webSocket.onclose never happens and that's strange as instance.webSocket.close(); is called here https://github.com/AirenSoft/OvenLiveKit-Web/blob/17e7387782bc63485ea6694cda18a2b2d7161ab9/src/OvenLiveKit.js#L741

Can you please double check on your side? Thanks!

SangwonOh commented 2 years ago

@brootle Hi. Yes, I will check it as well.

And please be careful site https://ovenplayer.com/docs/demo_input.html has been consolidated into https://demo.ovenplayer.com/demo_input.html

SangwonOh commented 2 years ago

@brootle connectionClosed callback is designed to only occurs when there is an abnormal disconnection situation. If you use 'instance.remove' to stop the stream on the client side, this code will not fire the 'connectionClosed' callback.

https://github.com/AirenSoft/OvenLiveKit-Web/blob/17e7387782bc63485ea6694cda18a2b2d7161ab9/src/OvenLiveKit.js#L369

brootle commented 2 years ago

@SangwonOh thanks! I was playing with closing connection from the client side and after some testing it looks like it works best when I send 'stop' command via socket connection, this way server closes both peer and socket connection, there is also one funny thing, browser for some reason doesn't send command right away, there is ping/pong (like every 30 seconds or so) between browser and server during socket connection, so I am sending 'stop' command and one second after that send empty message, this way there is no delay and command is delivered, server sends error to client and I have confirmation on client that connection is closed. I noticed that there was 'stop' command in OvenPlayer client, but this logic didn't get into OvenLiveKit-Web, see https://github.com/AirenSoft/OvenPlayer/blob/master/src/js/api/provider/html5/providers/WebRTCLoader.js#L875

SangwonOh commented 2 years ago

@brootle Yes. As you said, it might be a good idea to send the `stop' command. We will implement it and patch it in the next update. Thanks!

Ahmatmufid commented 7 months ago

Good