Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

atoppi
Copy link
Member

@atoppi atoppi commented Jul 25, 2024

Attempt to fix #3411

@atoppi atoppi requested a review from lminiero July 25, 2024 12:33
…nditions to drop packets in relay_rtp_packet
@atoppi atoppi marked this pull request as draft July 26, 2024 08:24
@atoppi
Copy link
Member Author

atoppi commented Jul 26, 2024

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 atoppi marked this pull request as ready for review July 26, 2024 20:10
@atoppi
Copy link
Member Author

atoppi commented Jul 26, 2024

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[1.x] databuffermsg/buffermsg not working for streaming plugin
2 participants