Re: Question: how should addTrack work in relation to createOffer et. al.?

On 5/9/15 12:45 AM, Stefan Håkansson LK wrote:
> On 08/05/15 23:23, Jan-Ivar Bruaroey wrote:
>> On 5/8/15 9:43 AM, Stefan Håkansson LK wrote:
>>> Trying to summarize this discussion:
>>>
>>> It seems that there is agreement:
>>>
>>> * the offer created by createOffer will only take into account the
>>> senders (and their respective settings) that exist at the time when
>>> createOffer is called - anything that happens after will not be reflected
>>>
>>> Is this right?
>> Not quite I fear. Depends on your definition of "the time when
>> createOffer is called".
>>
>> Lets call this A:
>>
>> Since createOffer is queued in the operations chain, what I realized in
>> this thread, and agree is needed at a minimum, is that, to avoid
>> non-determinism, createOffer must take into account the senders that
>> exist at the time when it comes off the queue and is executed, rather
>> than any time later.
>>
>> This would simply require wording under createOffer, ideally as steps in
>> a processing model, which is sorely needed anyways. It would not require
>> createOffer-specific changes to the description of the operations chain.
>>
>> It would not change much else, which means it still requires accepting
>> PR 222 to fix the "sometimes y is added, sometimes it is not" surprise.
>>
>> E.g. for pc.addTrack(X); pc.createOffer(); pc.addTrack(Y) the offer
>> would always contain X and Y (regardless of what's on the queue).
>>
>> I agree with Martin where he says [1]: "One thing that I like about this
>> is that it removes the synchronous step. Having a function run
>> synchronously or asynchronously depending on something that has happened
>> elsewhere is a very good way to ensure that surprises happen. Allowing
>> the current execution context to run to completion before executing any
>> potentially asynchronous operation is always better than having surprises."
>>
>> (Martin, sorry for quoting you here, but I thought it was a good point
>> that got lost in the older PR [1]).
>>
>> I could see us doing this. I suspect it is not what you meant.
>>
>> Lets call this B:
>>
>> I think what you mean is have createOffer take into account the senders
>> that exist at the time when the content JS calls createOffer. The only
>> benefit I see to this (and correct me if I'm wrong) would be to try to
>> have the offer never contain Y in the common (not queued) case, and
>> ignore the non-common (queued) case, since essentially "people, quit
>> doing that!"
>>
>> It seems to me this would require createOffer-specific changes to the
>> description of the operations chain (not to mention the other methods we
>> haven't talked about). And you still have the "sometimes y is added,
>> sometimes it is not" surprise.
>>
>> You would therefore not want PR 222 here.
>>
>> Lets call this C:
>>
>> Same as B, plus rip out the operations queue, and have methods throw if
>> called at the wrong time. This removes the need for the queue and solves
>> the "offer never contains Y" problem that way.
>>
>> I think I like A and C the best.
> Thanks for outlining the options.
>
> I interpreted the conversation as that you and Harald had agreed on B,
> and that is what I meant. That does not seem to be the case :).

I think we're close.

Lets call this AB:

How about this: If we remove PR 222, then A is indistinguishable from B 
in the common (not-chained) case. I.e. createOffer runs right away, thus 
uses the senders that exist at the time of the content-JS-call (like B).

We get the same benefit as B (in the common not-chained case) with less 
surgery on the spec text (no need for createOffer-specific text in the 
operations algorithm).

The remaining difference is in the chained case, where I'm torn between 
feeling that ignoring track additions/removals since the method was 
chained, is wrong, and not caring at all.

To illustrate, say you relied on the implicit chain:

     log(pc.signalingState); // have-remote-offer
     pc.setLocalDescription(answer);
     log(pc.signalingState); // have-remote-offer
     pc.addTrack(X);
     pc.createOffer() // wont run tillstable!
     .then(offer => {
       log(pc.signalingState); // stable(createoffer doesn't change state)
       // In AB, offer contains X and Y.
     })
     .catch(failed);
     log(pc.signalingState); // have-remote-offer
     pc.addTrack(Y); // <-- discerning intent here is a crapshoot!

whereas if you didn't:

     log(pc.signalingState); // have-remote-offer
     pc.setLocalDescription(answer)
     .then(() => {
       log(pc.signalingState); // stable
       pc.addTrack(X);
       var p = pc.createOffer(); // runs now
       pc.addTrack(Y);
       return p;
     }).then(offer => {
       log(pc.signalingState); // stable
       // offer contains Xonly
     })
     .catch(failed);

The reason I'm coming around is a realization that I like C (getting rid 
of the chain), but that it is probably too late to deprecate it fully. 
The second best then seems to be to ignore it as much as possible and 
not let it influence the design further, in spite of surprises using it.

.: Jan-Ivar :.

PS: I think we should describe the operations chain with promises 
regardless. I'll see if I can make a reasonable PR that retains today's 
behavior.

Received on Saturday, 9 May 2015 19:15:12 UTC