search

meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.17k stars 2.47k forks source link

Fix handling of "data" stream parameters in the streaming plugin #3412

Closed atoppi closed 3 weeks ago

atoppi commented 2 months ago

Attempt to fix #3411

atoppi commented 2 months ago

Converted the PR into draft since the changes have become more error prone and I'd like to have a review first.

In the current status the sending of the buffered message happens in the data_ready callback. In the callback we iterate the session, looking for streams that have the buffermsg flag set to true. I'm not sure if that callback might be issued multiple times, in particular when multistream and renegotiation come into play. Also the condition to drop packets in relay_rtp_packet has been changed to let early data packets come through.

atoppi commented 2 months ago

Updated the patch in order to send a buffered message only the first time data_ready is being called. I'm still not sure of potential side-effects / bad behaviors in case of renegotiation or restart.

lminiero commented 2 months ago

I'm still not sure of potential side-effects / bad behaviors in case of renegotiation or restart.

The data_ready callback is sent any time the buffers in the SCTP stack empty, and so it's safe to send again. That's why we also use it as a trigger for when we can start sending the first time. This has nothing to do with renegotiations or restart, since it has to do with the SCTP stack, and using the dataready volatile int should be safe since it's only reset when hangup_media hits.

atoppi commented 3 weeks ago

I have reverted a condition that was meaningless. Merging since it seems to work.