Add a method to the direct api to retrieve the sctp max message size#996
Add a method to the direct api to retrieve the sctp max message size#996wdouglass wants to merge 3 commits into
Conversation
dfa346f to
dd10d3e
Compare
|
Thanks for the approval!
|
Thank you!
Squash merge is my new way. Try to keep the PR description correct, since that will be the commit message.
Try! I can reproduce locally. Can't you? |
|
Ok, i reproduced the issue. It doesn't happen with It comes from the recursive call at lib.rs:1731 I'm currently doing a slight refactor of that function to avoid the recursive call (there's experimental forced-tail-recursion in rust 1.91 but i think you probably want to avoid experimental features) I'll rebase and push a patch when i've got it sorted |
dd10d3e to
c4bb63f
Compare
There was a problem hiding this comment.
I'm not entirely sold on where this PR is going.
max-message-size is the maximum SCTP user message size. It can be larger than MTU because SCTP fragments and reassembles the message. But I don't think that means str0m needs to reserve enough internal buffering for one or more max-sized messages.
Sending a large data channel message this way is mostly a convenience feature. The app gets to treat it as one logical message and the receiver gets one ChannelData event after reassembly. It is not really a wire efficiency thing. It still becomes a bunch of MTU-sized packets underneath.
So I think the right model is still backpressure:
Channel::write()returnsfalsewhen str0m doesn't have buffer space right now- the app uses
set_buffered_amount_low_threshold()to choose when it wants to resume, then retries onChannelBufferedAmountLow
The backpressure API already exists with buffered_amount, set_buffered_amount_low_threshold() and ChannelBufferedAmountLow.
I'm ambivalent about the getter of the negotiated max message size setting because I can't see it have any advantage to know it beyond convenience.
As of master right now, Channel::write() will always return false if the message is greater then the size of str0m's buffer space. There's no way to Channel::write part of a message, or to submit pre-fragmented sctp messages. Maybe the |
Yes
As long as what you're writing is below the threshold?
Right now it doesn't, the question is how strong the case is for making it possible to write/receive large messages.
Yes, this is why the current sized buffer is quite low. Is the case that a user would reasonably expect to push large messages through the SCTP part of a WebRTC implementation? |
The sensor i'm working on sends data over the sctp channel that's associated with each frame that gets sent over the h264 channel. That data is 16 bits per pixel, of a 1920x1200 image, that losslessly compresses to about 250k per image at 30fps. Right now, after compression, i'm fragmenting that data to 32k chunks, and then sending via sctp, which fragments it again to MTU sized packets. The client (a webassembly program running in firefox) reports a max-message-size that's 262144 bytes, so i suspect that if i got it to negotiate through the sctp implementation i could save time fragmenting my extra data frames twice. I've tried this with a patched str0m, and the performance benefit is very small, so if you'd like to reject this change i understand, but it seems logically that it should be permissable to send max-message-size messages. I also understand that head-of-line blocking becomes an issue with large messages, so there are some tradeoffs there. |
…lling itself recursively. This avoids a stack overflow in the `data_channel_flood` unit test
c4bb63f to
c247846
Compare
|
I'd like to park this one for now. Overall the question about using str0m as a parking place for data keeps coming up. When I wrote str0m I was very clear on that, generally, buffers should be outside of str0m. However this have in turn resulted in issues like the repeating problem of the calling pattern (one mutation followed by poll-to-timeout). If we are to be more liberal on storing data in str0m, I'd like to do this as a bigger initiative also affecting the media sending. |
This allows the calling program to use this information when generating messages.
As a side effect of allowing bigger messages from the calling program, the max_buffered_across_streams value has to increase to accommodate at least 1 message. The second commit in this PR allows for 2 messages to be buffered, although if there's some other equation we should use let me know.