RecvHandler::ReceiveDataChannel() is wrong

[ibc]

As stated in the forum, the RecvHandler::ReceiveDataChannel() implementation of libmediasoupclient is wrong. It does not read streamId from given sctpStreamParameters (there are no given sctpStreamParameters) but increments it locally.

• In mediasoup-client Chrome74.ts handler:

  async receiveDataChannel(
    { sctpStreamParameters, label, protocol }: HandlerReceiveDataChannelOptions
  ): Promise<HandlerReceiveDataChannelResult>
  {
    this._assertRecvDirection();

    const {
      streamId,
      ordered,
      maxPacketLifeTime,
      maxRetransmits
    }: SctpStreamParameters = sctpStreamParameters;

    const options =
    {
      negotiated : true,
      id         : streamId,
      ordered,
      maxPacketLifeTime,
      maxRetransmits,
      protocol
    };

    logger.debug('receiveDataChannel() [options:%o]', options);

    const dataChannel = this._pc.createDataChannel(label, options);

• In libmediasoupclient Handler.cpp:

  Handler::DataChannel RecvHandler::ReceiveDataChannel(
    const std::string& label, webrtc::DataChannelInit dataChannelInit)
  {
    MSC_TRACE();

    uint16_t streamId = this->nextSendSctpStreamId;

    dataChannelInit.negotiated = true;
    dataChannelInit.id         = streamId;

    /* clang-format off */
    nlohmann::json sctpStreamParameters =
    {
      { "streamId", streamId                },
      { "ordered",  dataChannelInit.ordered }
    };
    /* clang-format on */

    // This will fill sctpStreamParameters's missing fields with default values.
    ortc::validateSctpStreamParameters(sctpStreamParameters);

    rtc::scoped_refptr<webrtc::DataChannelInterface> webrtcDataChannel =
      this->pc->CreateDataChannel(label, &dataChannelInit);

    // Increase next id.
    this->nextSendSctpStreamId = (this->nextSendSctpStreamId + 1) % SctpNumStreamsMis;

In mediasoup-client we read streamId from the given sctpStreamParameters (as it should be). However, in libmediasoupclient we assume streamId is incremental based on a local count. It doesn't make any sense.

[jmillan]

This passed unnoticed when DataChannels were introduced into libmediasoupclient. Working on it.

[tdrz]

Sorry for that!

[jordaneckhardt]

Thank you, @jmillan! I am looking at your code so far and comparing it to the JS client, and I have two questions:

1. The JS client passes sctpStreamParams all the way down to the construction of the data channel. Instead, you pass in streamId and set the data channel's ID to equal streamId. See JS implementation: https://github.com/versatica/mediasoup-client/blob/v3/src/Transport.ts#L725-L734.
2. You've required streamId to be passed to both ProduceData and ConsumeData. The JS client manages the stream IDs for data producers: https://github.com/versatica/mediasoup-client/blob/v3/src/handlers/Chrome74.ts#L555-L625.

I understand this is still a work in progress, but is there a reason to deviate from the JS client?

[jmillan]

Hi @jordaneckhardt,

Thanks for the comments. It's a work in progress branch.

[jordaneckhardt]

I see the completed PR does change ProduceData. Thank you.