[rtcweb] Review of draft-ietf-rtcweb-jsep-07

[Paul Kyzivat <[e-mail]>]

I just did my first thorough read-through of JSEP in quite some time. I 
came away with a number of concerns:

Section 1.1:

When describing offers, modification by the application is mentioned:

    JSEP's handling of session descriptions is simple and
    straightforward.  Whenever an offer/answer exchange is needed, the
    initiating side creates an offer by calling a createOffer() API.  The
    application *optionally modifies that offer*, and then uses it to set
    up its local config via the setLocalDescription() API.

but when describing answers it is not:

    When the call is accepted, the callee uses the createAnswer() API to
    generate an appropriate answer, applies it using
    setLocalDescription(), and sends the answer back to the initiator
    over the signaling channel.  When the offerer gets that answer, it
    installs it using setRemoteDescription(), and initial setup is
    complete.

And Section 6 only talks about changing offers, not answers. It is 
equally important to be able to modify answers. (More on this in later 
comment on section 6.)

Section 4.1.4:

    The only difference between a provisional and final answer is that
    the final answer results in the freeing of any unused resources that
    were allocated as a result of the offer.  As such, the application
    can use some discretion on whether an answer should be applied as
    provisional or final, and can change the type of the session
    description as needed.  For example, in a serial forking scenario, an
    application may receive multiple "final" answers, one from each
    remote endpoint.  The application could choose to accept the initial
    answers as provisional answers, and only apply an answer as final
    when it receives one that meets its criteria (e.g. a live user
    instead of voicemail).

Issue: in this forking case, until the final selection is made it is 
unclear which one will need ICE completed. Only when a setlocal is done 
on one of the answers will ICE begin to be performed. Then, if another 
answer is provisionally set, won't that stop ICE for the prior one? And 
won't it require new candidates? What if one of the earlier ones is 
eventually chosen as final?

And what if ICE completes for one of the candidates? Can't media start 
to flow? Then what if a different candidate is set as the final answer?

Section 4.1.4.1:

I question the following:

    ...  While it is good practice to send an immediate
    response to an "offer", in order to warm up the session transport and
    prevent media clipping, the preferred handling for a web application
    would be to create and send an "inactive" final answer immediately
    after receiving the offer.  Later, when the called user actually
    accepts the call, the application can create a new "sendrecv" offer
    to update the previous offer/answer pair and start the media flow.

This is very unfriendly when receiving calls that might be forked. By 
immediately "answering" a call before knowing if the user will accept 
it, you preempt the possibility that it will be answered on some other fork.

For instance, if a call could come to your browser, or be sent to an 
answering service, and your broswer is on but you aren't present to 
accept the call, then the call will be accepted and then dropped rather 
than sent to the answering machine.

So this technique should not be used if there is any possibility that 
the request could be coming from a source that might try other 
possibilities.

Section 5.2.1:

    When createOffer is called for the first time, the result is known as
    the initial offer.

By this definition, if a peer connection initially *received* an offer 
and sent an answer, and then it later sends an offer then that offer 
would be considered an initial offer.

I think perhaps what is intended is:

    When createOffer is called before setLocal has been called with
    an offer,  the result is known as the initial offer.

The following doesn't support the "balanced" BUNDLE policy:

    Once all m= sections have been generated, a session-level "a=group"
    attribute MUST be added as specified in [RFC5888].  This attribute
    MUST have semantics "BUNDLE", and MUST include the mid identifiers of
    each m= section.  The effect of this is that the browser offers all
    m= sections as one BUNDLE group.  However, whether the m= sections
    are bundle-only or not depends on the BUNDLE policy.

Section 5.2.2 says:

    o  If any MediaStreamTracks have been added, and there exist recvonly
       m= sections of the appropriate media type with no associated
       MediaStreamTracks, or rejected m= sections of any media type,
       those m= sections MUST be recycled, and a local MediaStreamTrack
       associated with these recycled m= sections until all such existing
       m= sections have been used.  This includes any recvonly or
       rejected m= sections created by the preceding paragraph.

This fails to say anything about codec compatibility. SDP O/A requires 
the answer to be a subset of the offer in terms of PT/codec 
configuration. You should not use the same m-line unless there is a 
desire to use the same settings for the new track as are currently in 
use in the other direction.

Section 5.3.1:

    When createAnswer is called for the first time after a remote
    description has been provided, the result is known as the initial
    answer.

By this definition, if a peer connection initially sent an offer and 
received an answer, and then it later receives an offer then the answer 
to that first *received* offer would be considered an Initial Answer.

I think perhaps what is intended is:

    When createAnswer is called before setLocal has been called with
    an offer,  the result is known as the initial answer.

When specifying the mapping of local tracks to m-lines in a received 
offer, there is again no discussion of codec compatibility. It is quite 
possible that the m-line chosen by the algorithm in this section won't 
have compatible codec attributes, but some other m-line will.

Sections 5.3.2, 5.3.3, and 5.4-5.7:

Are these empty because the content is yet to be written?

Section 6:

Other likely changes are addition of extra attributes and maybe other 
parameters. For instance, CLUE will want to add another grouping construct.

	Thanks,
	Paul