Conversation
src/change/sdp.rs
Outdated
| if let Some(max_size) = app_line.max_message_size() { | ||
| rtc.sctp.set_remote_max_message_size(max_size as u32); | ||
| } | ||
| } |
There was a problem hiding this comment.
At row 106 we are initializing the SCTP subsystem, and I'm unsure whether we want to keep this change to the SCTP together with that. I'm also unclear on what should happen if you change this value in a follow-up SDP, is that valid?
There was a problem hiding this comment.
I was also wondering whether max-message-size should be set once or can be changed dynamically. I didn't see any reason why it wouldn't be dynamic in sctp-proto and if I understand RFC 8841 correctly, it can be included when modifying the session. Section 10.5 basically says that modifying the session works like the initial offer/answer procedure, with some exceptions. But those do not concern individual SDP attributes.
However, I haven't tried to figure out whether str0m supports modifying the session yet.
At row 106 we are initializing the SCTP subsystem, and I'm unsure whether we want to keep this change to the SCTP together with that.
Not sure I fully understand this. Are you saying you might prefer set_remote_max_message_size to be called in sctp_init? As far as I can tell, none of the other code in apply_answer modifies the rtc.sctp instance. But moving this to sctp_init would effectively disable changing this attribute in a SDP renegotiation, right?
There was a problem hiding this comment.
Yeah, that's what I mean. Keep it together with sctp_init unless we have strong reasons to think this is modifiable by subsequent negotiations.
There was a problem hiding this comment.
While trying to move the setting of remote_max_message_size to Rtc::init_sctp, I realized that I passed the wrong constant to TransportConfig::with_max_receive_message_size. This was fixed in 892488f.
I also noticed that for the server config, I missed setting max_send_message_size. The issue here is that the server config (which contains the TransportConfig) is created in RtcSctp::new, which is called before SDP negotiation takes place. I didn't find a way to access the existing ServerConfig and the contained TransportConfig, in order to set max_send_message_size, so instead I added a call to Association::set_max_send_message_size in RtcSctp::poll where the Connected event is handled.
LOCAL_MAX_MESSAGE_SIZE is what we are willing to receive. DEFAULT_REMOTE_MAX_MESSAGE_SIZE is what the remote is willing to receive, hence we are able to send.
2 participants
This PR fixes
max-message-sizehandling by separating local and remote limits and including them in SDP negotiation.Context: #844 (comment)