Properly handle max payload length of zero - #1966
Conversation
Take the same approach as rust: livekit/rust-sdks#1137
size-limit report 📦
|
🦋 Changeset detectedLatest commit: 87845eb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| // than LiveKit/pion can deliver end-to-end (~64 KiB), so we trust | ||
| // the answer up untilthe built in ceiling. | ||
| const maxPublisherMessageSizeBytes = Math.min( | ||
| this.pcManager?.getMaxPublisherMessageSize() ?? DEFAULT_MAX_MESSAGE_SIZE, |
There was a problem hiding this comment.
Realized a bit late that the react-native-webrt. didn't fill out the sctp field so the previous PR didn't actually work for it. Adding the default now covers react-native nicely 👍
| ); | ||
| if ( | ||
| typeof maxPublisherMessageSizeBytes !== 'undefined' && | ||
| maxPublisherMessageSizeBytes !== 0 /* 0 means "no limit" */ && |
There was a problem hiding this comment.
can maxPublisherMessageSizeBytes be 0 at all ? I think it is protected to be min(DEFAULT_MAX_MESSAGE_SIZE)
There was a problem hiding this comment.
Yep, it can be zero - the expression 0 ?? n will always be zero (more info, this is actually an important key difference between ?? and ||), so min(0 ?? n, n) where n > 0 will also always be 0.
As a follow up to #1962, I realized there were a few small discrepancies with the swift and rust versions.
min(maxMessageSize, 64_000)check rather than using themaxMessageSizevalue verbatim.0is a special value that means "no limit", the web implementation didn't do this