CHANGES: Implementation feedback

Hi

We have been implementing the WebRTC spec for a while and would like to
bring forward some proposals for changes. We have based our comments on
the 14 September 2011 editor's draft and therefore some of the proposed
changes may have to be adopted to ongoing changes in the spec.

4. Peer-to-peer connections

The "ICE started flag" is currently used to determine if the initial
offer has been processed (produced or handled). I suggest we rename it
to "initial offer processed flag" to make this clearer.

* PeerConnection (constructor section)

I propose to add a new step 16 to the algorithm to make it simpler:

- 16. If the PeerConnection's "initial offer processed flag" is false,
      then abort these steps.

This will not change the behavior since the operations below this point
do nothing if the flag is false. As a result, we can remove the "ICE
started flag" condition in current step 16, entire step 17 and make 18
unconditional.

* "When a user agent starts receiving media..."

In step 2, "then associate the component with that media stream and
abort these steps", could perhaps be changed to "then associate the
component with the corresponding MediaStreamTrack object in that media
stream and abort these steps" since it then made clearer that all
tracks are created when the MediaStream object is created in step 3. 

4.1.2 Methods

* addStream and removeStream

addStream and removeStream needs to know if the initial offer hasn't
been processed yet, in which case they can back off and have their
pending streams included in the initial offer, or if the operations
should result in a separate signaling message. The new step 6 below
proposes how this could be done.

 - 6. If the "initial offer processed flag" is true, then the next
      time the user agent provides a stable state, it must have the
      PeerConnection's PeerConnection ICE Agent add a media stream for
      stream. Any other pending stream additions and removals must be
      processed at the same time. [ICE]

I would also like to add the following text as a note or to the last
step in the algorithms that describe both addStream and removeStream:

"A pending stream addition and a pending stream removal that concerns
the same stream should cancel each other and not cause a signaling
message."

The reason is that the behavior will be different if the pending streams
cancel each other out or are processed as an add as well as a remove.
The first case will not cause a signaling message to be dispatched if
these two streams are the only ones to be processed, while the other
case will.

* processSignalingMessage

The string comparison in step 4 should explicitly state if it's
case-sensitive or not to be consistent with the rest of the spec.
My suggestion is to make it case-sensitive. That's also how we
implemented it. Section 4, where the SDP-prefix is added, should also be
updated to match.

/Adam

Received on Wednesday, 28 September 2011 10:07:43 UTC