Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Describe when an RTCSctpTransport is created/set to null. #996

Merged
merged 2 commits into from
Jan 26, 2017

Conversation

taylor-b
Copy link
Contributor

Fixes #979.

An SctpTransport represents a unique SCTP association. It's created from
applying a local description, and set to null when applying an answer
with a rejected data m= section.

SCTP-SDP reference will be added by this PR: tobie/specref#327

Fixes w3c#979.

An SctpTransport represents a unique SCTP association. It's created from
applying a local description, and set to null when applying an answer
with a rejected data m= section.
@taylor-b taylor-b force-pushed the issue_979_sctptransport_lifetime branch from 1e296a1 to 53a0ffe Compare January 19, 2017 18:57
and it creates a new SCTP association, as defined in
[[!SCTP-SDP]], set the value of <var>connection</var>'s
[[<code><a>sctpTransport</a></code>]] internal slot
to a newly created <code><a>RTCSctpTransport</a></code>.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe that a new SCTP association can be created until setRemoteDescription is called, because until then the remote port isn't known. So you might say, "creates a new RTCSctpTransport object.".

Copy link
Contributor Author

@taylor-b taylor-b Jan 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would "initiates a new SCTP association" be better wording? What I'm trying to capture is:

  • data "m=" section negotiated for the first time
  • data "m=" section going from rejected to non-rejected
  • local port changed (which should only happen if this is an answer to a remote offer that changed the port)

Copy link
Contributor

@aboba aboba Jan 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to initiate a new SCTP association before setRemoteDescription is called? If the local port and remote port are assumed to be the same, I guess it is, but otherwise you need the remote port provided by setRemoteDescription.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to initiate a new SCTP association before setRemoteDescription is called?

Yeah, "initiate" doesn't quite work either, especially given that SCTP uses an "INIT" message.

I'd like to not say something as vague as "If the description is associated with a new SCTP association"... How about "If the description will lead to a new SCTP association being established?"

Another option would be to only create a new object when applying an answer or pr-answer. The more I think about it, the more that seems like the more correct thing to do. For one, how is "maxMessageSize" set without any remote description?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the SctpTransport is created when calling setLocalDescription then maxMessageSize needs to be nullable since it will be null prior to setting a remote description. Perhaps a state attribute would clarify whether the SctpTransport was new, connecting, connected or closed.

This simplifies things, and means we don't need to make maxMessageSize
nullable.

Also, started referencing the specific sections of sctp-sdp that deal
with this, and changed "closes/establishes" to "initiates the closure"/
"initiates the establishment", which is more accurate.
@aboba
Copy link
Contributor

aboba commented Jan 26, 2017

LGTM

@alvestrand alvestrand merged commit 290c25a into w3c:master Jan 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants