Add optional sctpStreamId to SCTP-based data consumers #1378
Conversation
| this.#sctpStreamIds![sctpStreamId] = 1; | ||
| sctpStreamParameters.streamId = sctpStreamId; | ||
| sctpStreamParameters.streamId = this.getNextSctpStreamId(sctpStreamId); | ||
| ortc.validateSctpStreamParameters(sctpStreamParameters); |
There was a problem hiding this comment.
Why this call to ortc.validateSctpStreamParameters()?
There was a problem hiding this comment.
It seemed like an oversight as this call is also present for validating the SCTP stream parameters for the producer (https://github.com/versatica/mediasoup/pull/1378/files#diff-3253c176609c972f1279e2a6c6ca4c52cbfac8423efcc8b2891b9d8dbd92c358R1016).
There was a problem hiding this comment.
Please remove it. There is no need for this. These sctpStreamParameters were already validated in transport.produceData() and they are literally a copy of them.
There was a problem hiding this comment.
You're right, only streamId is now user input but its value (besides type) is not validated in validateSctpStreamParameters anyways. I'll remove it.
|
@threema-lenny, how are you taking benefit of choosing the steam ID for a DataConsumer? Can you share a use case? |
Sure. We're using OOB negotiated data channel IDs (using In our case, data is relayed via the SFU using this fixed SCTP stream ID which is why we set the corresponding SCTP stream ID for both producer and consumer. |
|
I get the point of this feature but need some time to review it. |
| await ctx.webRtcTransport2!.consumeData({ | ||
| dataProducerId: ctx.dataProducer!.id, | ||
| sctpStreamId: 123, | ||
| }); |
There was a problem hiding this comment.
Please, create const dataConsumer here and assert that its sctpStreamParameters.streamId matches the given sctpStreamId.
|
@threema-lenny are you working on this PR? |
|
@ibc Not right now but I'll definitely need to come back to getting this done. |
Second try (after #1111). This makes it possible to use a specific SCTP stream ID / data channel ID for forwarding data.
Could you give me feedback on whether this looks good for the Node API? Then I'll see if I can add the change to the Rust API, too.
TODO
sctpStreamIdinDataConsumerOptionsin the website. This task is for mediasoup authors once this PR is merged.