Re: WebRTC spec errata (regarding states)

Hi

I've collected the comments from github in this mail as well.

Github version can be viewed online here (note that links doesn't work):

http://htmlpreview.github.com/?https://github.com/fluffy/webrtc-w3c/blob/master/webrtc.html

The github spec is not updated with these changes yet.

>> From spec:
>> The gathering process is done.

> juberti@github:
> I suggest that we should transition the gathering
> state to completed here, before dispatching this callback (which
> indicates that that state has changed).

Agreed.

>> From spec:
>> This can happen, e.g., if there are insufficient resources to apply
>> the SDP. The user agent MUST then roll back as necessary if the new
>> description was partially applied when the failure occurred.
>
> juberti@github:
> Rollback may not always be possible in this case. I think this should
> be treated as a fatal error.

What's the result of a fatal error? Close the PeerConnection?

What should errorType be in this case, INCOMPATIBLE_SESSION_DESCRIPTION?

>> From spec:
>> When the method is invoked, the user agent must follow the
>> processing model of setLocalDescription() .

> juberti@github:
> I think it would be better to spell this out explicitly here. For
> example, the ICE-related processing is different - this doesn't
> trigger any gathering changes.
>
> Also, if the remotedesc ICE credentials are different, this means
> that createAnswer should generate new ICE credentials in the next
> call (so that a local ICE restart can be performed). LMK if you'd
> like me to write some text for this part.

I wrote this as a shared processing model since most of the text would 
be the same for both methods and it's a whole lot easier to maintain one 
text at this stage.

The points you mention above are taken into consideration since the 
current model sometimes talks about setting a specific type of 
description. For example:

"4. If the local description was set with content that caused an ICE 
restart, then set connection's ice gathering state to gathering."

We could move entire bullets, that only deals with either local or 
remote descriptions, out of the shared model, but I would, at least for 
now, keep a shared model for the error and success cases.

On 2013-03-10 18:54, Justin Uberti wrote:
> Thanks for making these changes. A few comments inline:
>
>
> On Wed, Mar 6, 2013 at 10:04 AM, Adam Bergkvist
> <adam.bergkvist@ericsson.com <mailto:adam.bergkvist@ericsson.com>> wrote:
>
>               4. 4.3.2.2 <http://4.3.2.2>: setLocalDescription
>
>                  The description for this function should indicate that it
>                  triggers a signaling state change. It should also
>         indicate that
>                  an INVALID_STATE exception is triggered if the wrong
>                  SessionDescription.type is supplied for the current state.
>               5. 4.3.2.2 <http://4.3.2.2>: setRemoteDescription
>
>                  The description for this function should indicate that it
>                  triggers a signaling state change. It should also
>         indicate that
>                  an INVALID_STATE exception is triggered if the wrong
>                  SessionDescription.type is supplied for the current state.
>
>
>     Updated with the modifications we talked about in a prior reply to
>     this mail. I've tried to address some stuff that was unspecified
>     around these functions. We have a bit of redundancy with both a
>     callback and an event when a description is applied successfully.
>     I've picked an order for these which is event first, and callback
>     second. Please review.
>
>
> That ordering sounds good to me. I would expect that when I got the
> success callback, I should be able to call other functions, so all
> states should already be up-to-date.
>
> I do think we need to have separately-specified processing for local and
> remote descriptions. For example, the local description may influence
> the gathering process, whereas the remote description does not.

Addressed above.

>               2. 4.3.2.1 <http://4.3.2.1>: onicecandidate
>
>                  The description for this callback should indicate that
>         it is
>                  invoked with a null argument when gathering completes.
>         Prior to
>                  dispatching this null callback, the iceGatheringState
>         will be
>                  set to "completed".
>
>
>     I've seen that we have descriptions on event handler attributes in
>     the form "this callback should be invoked when...". There are other
>     ways to listen for events so the attribute handler shouldn't be
>     called for any special reason other then the event being fired.
>     Anyways, to address this particular problem, I've added a paragraph
>     about how the ICE Agent notifies the script about the candidate
>     gathering process (new candidate or gathering done). Please review.
>     Perhaps the ICE Agent should have it's own sub-section in the "4.3.1
>     Operation" section where we collect this kind of stuff.
>
>
> I didn't quite follow what you were saying here.

My point was that I didn't want to extend the description of the 
callback because it's the wrong place do document this kind of behavior. 
That was basically the motivation for the new text about how the ICE 
Agent talks to the script.

> I do think we need some
> way to specify when the gathering state transition to "completed" occurs.

Addressed above.

/Adam

Received on Monday, 11 March 2013 12:10:50 UTC