Subject: [rtcweb] Review of draft-ietf-rtcweb-jsep-06

From: tshields <[email-masked]>
Date: Wed, 28 May 2014 22:07:32 -0600

Below are some comments that I have regarding
[draft-ietf-rtcweb-jsep-06]. Overall it is very good, understandable,
and concise, but I've provided my thoughts on certain sections that
were not entirely clear to me.

I guess my one comment addressing the document as a whole would be
that the examples used in the draft are helpful in explaining certain
aspects and therefore even more examples would be even more helpful.

+ Section 1.1, Paragraph 3, Last Sentence

> "This mechanism effectively removes the browser almost completely 
> from the core signaling flow; the only interface needed is a way 
> for the application to pass in the local and remote session 
> descriptions negotiated by whatever signaling mechanism is used, 
> and a way to interact with the ICE state machine."

I think this can be reworded to be clearer and more powerful, something
such as:

"JSEP removes the browser almost entirely from the core signaling flow.
The Javascript application needs interfaces for only (1) passing in
local and remote session descriptions and (2) interacting with the ICE
state machine."

Perhaps the exact wording I've suggested is not ideal, but something
that more clearly shows that JSEP removes the browser from the signaling
flow and that the application (and therefore, the application's
developer) needs to do only two things for establishing a session. The
way it is worded now is unclear at first read and does not adequately
persuade me why JSEP is awesome (IMO).

+ Section 3.4.2, Paragraph 1

> "However, to accelerate cases where the browser knows the number of
> media streams to use ahead of time"

An example of a situation where the browser might know the number of
media streams ahead of time might be useful for context here, if one is
easily explained.

+ Section 3.5.1, Paragraph 2

> "... the application can choose to apply it either as a provisional
> answer, leaving open the possibility of using a different answer in
> the future, or apply it as a final answer, ending the setup flow."

How does the application decide whether to apply as provisional or
final? Does JSEP care at all or is this left entirely up to the
application to decide? This is expounded on somewhat in the 2 paragraphs
that follow, but I still don't feel like these questions are completely
answered.

+ Section 3.6, Paragraph 3 (Open Issue)

> "[OPEN ISSUE: EKR proposed an alternative rehydration approach 
> where the actual internal PeerConnection object in the browser was 
> kept alive for some time after the web page was killed and
> provided some way for a new page to acquire the old PeerConnection 
> object.]"

Having some mechanism for keeping the PeerConnection object in the
browser would be awesome and I agree that it should be explored further.
However, I also think it should not replace the current rehydration
method described as this allows the session state to be saved and
restored from a server to the Javascript application running in the
browser, correct?

+ Section 4.1.2, Paragraph 2

> "the generated SDP will contain all desired functionality for the 
> session (certain parts that are supported but not desired by 
> default may be omitted)"

I think this sentence would be clearer if "certain parts that are" was
replaced with "functionality that is".

+ Section 4.1.2, Paragraph 3

> "For each existing stream, the generation of each SDP line must 
> follow the process defined for generating an updated offer from the
> document that specifies the given SDP line."

This sentence was too hard to compute in my opinion, and I'm still not
quite sure what "the document" refers to. I think rewording would be
beneficial.

+ Section 4.1.4, Paragraph 5

> "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)."

So is it entirely up to the receiver of an answer to decide if an answer
should be treated as provisional or final, or does the offerer get to
make a suggestion on this matter? Is there any recommendation or
requirement on how long an application should keep a pranswer. My point
is that an application may set a pranswer anticipating another answer
that never comes. Does JSEP care at all about this situation?

+ Section 4.1.7, Paragraph 2 & Section 4.1.8, Paragraph 2

> "TODO: Do we need to expose accessors for both the current and 
> proposed local description?"

I don't think I can offer much on this issue except to say that it
would be nice if you could offer all the necessary functionality
without having to expose these accessors to keep it as simple as
possible for application developers.

+ Section 5.1.2, bullet R-2

> "R-2  ICE, as specified in [RFC5245], MUST be used.  Note that the 
> remote endpoint MAY use a Lite implementation."

This shouldn't be MAY because it is not referring to an optional
implementation specification of JSEP but rather a possible scenario that
JSEP needs to support. It should read as just a warning and possibly
specify that JSEP MUST be equipped to interface with an endpoint that is
using a Lite ICE implementation.

+ Section 5.2.1, bullet 3

> "   o  The third SDP line MUST be a "s=" line, as specified in 
> [RFC4566], Section 5.3; to match the "o=" line, a single dash 
> SHOULD be used as the session name, e.g. "s=-"."

Is there a reason to ignore the following SHOULD from RFC4566?

> "If a session has no meaningful name, the value "s= " SHOULD be 
> used (i.e., a single space as the session name)."

It seems like "-" and " " are both equally meaningless, so why deviate
from the standard? (I'm also not sure why we need to match it with the
username in the o= line). There may be good reason for both of these,
but it is not clear to me from reading the draft so I wanted to bring
it up.

+ Section 5.2.1, Paragraph 3

> "The next step is to generate m= sections for each 
> MediaStreamTrack..."

Consider adding: "...as specified in [RFC4566] Section 5.14..." The
draft references specific sections for the other SDP fields and I think
it is useful here as well.

+ Section 5.2.1, Paragraph 6, bullet 1 AND Paragraph 7, bullet 11

> "...as indicated in [I-D.ietf-mmusic-trickle-ice], Section 
> 5.1...."

Section 5.1 of [I-D.ietf-mmusic-trickle-ice] did not really clear this
point up for me at all. Is using a 'null' port value the same as setting
the connection address to "IP 6 ::" (which is mentioned in Section 5.1)?
If we wanted to set two m= sections to null port values, how will we do
so while maintaining this uniqueness constraint?

+ Section 5.2.3, Paragraph 1

> "The createOffer method takes as a parameter a MediaConstraints 
> object.  Special processing is performed when generating a SDP 
> description if the following constraints are present."

I think an example of when this would be useful might be nice for context.

+ Section 5.2.3.1 AND Section 5.2.3.2, Paragraph 1

> "If the "OfferToReceiveVideo" constraint is specified, with a value
> of "N", the offer MUST include N non-rejected m= sections with
> media type "video",... "

The use of "N" is confusing. Is "N" referencing an integer value or a
string with value "N". I think removing the quotes around "N" would make
it entirely more understandable.

+ Section 5.3.1, Paragraph 5

> "For each offered m= section of a given media type, if there is a 
> local MediaStreamTrack of the specified type which has been added 
> to the PeerConnection via addStream and not yet associated with a 
> m= section, and the specific m= section is either sendrecv or 
> recvonly, the MediaStreamTrack is associated with the m= section
> at this time."

This sentence is long and hard to parse. I think rewording and/or
breaking up would be beneficial.

+ Section 5.3.1, Paragraph 10

> "Note that regardless of the presence of "a=bundle-only" in the 
> offer, no m= sections in the answer should have an "a=bundle-only" 
> line."

Should this be a SHOULD NOT or is it more just informational?

+ Section 6, Paragraph 3

> "The following modifications, if done by the browser to a 
> description between createOffer/createAnswer and the 
> setLocalDescription, MUST be honored by the browser:"

Does this sentence just mean that a browser will honor its own changes
to a session description? Perhaps an example of when this might happen
would help make the situation a little clearer.

- --Nits:

+ Section 3.2, Paragraph 4

> "...the exact media parameters are only known only after a offer 
> and an answer have been exchanged..."

Should read "only after an offer".

+ Section 3.4.2, Paragraph 1

> "...the number of media streams to gather candidates for."

"... the number of media streams for which to gather candidates."

+ Section 3.5, Paragraph 1

> "...the media plane which is relevant."

Should say, "... the media plane that is relevant."

+ Section 4.1.5, Paragraph 2 and 3

> "This API changes the local media state"

Maybe I'm alone on this but I prefer the following: "Calling this API
changes the local media state." The API doesn't do anything on its
own, but is a mechanism for calling some action.

> "This API indirectly controls..."

(Same issue as above) "The candidate gathering process is indirectly
controlled through this API."

+ Section 5.2.1, Paragraph 4

> "Otherwise, it MUST use the same ICE credentials and candidates 
> that were used in the m= section that it is being bundled into."

"into which it is being bundled."

+ Section 5.2.1, Paragraph 5

> "as a result, they MUST all have the same [RFC4572]fingerprint 
> value."

Space needed after RFC reference.

+ Section 5.2.1, Paragraph 7, bullet 6

> "... that references the payload type fo the primary codec ..."

"of"

+ Section 5.2.1, Paragraph 7, bullet 11

> "...as specific in [RFC5245]..."

"specified"

+ Section 5.3.1, Paragraph 7

> "...each m= section in the answer should then generated..."

"...should then be generated..."

+ Section 5.3.1, Paragraph 7, bullet 4

> "... and there a local MediaStreamTrack has been associated ..."

"... and there is a local MediaStreamTrack that has been associated ..."

+ Section 5.3.1, Paragraph 7, bullet 5

> "... Section 3, MUST be be supported."

"... Section 3, MUST be supported."

+ Section 5.3.1, Paragraph 7, bullet 6

> "...the payload type fo the primary..."

"... the payload type of the primary..."

+ Section 5.3.1, Paragraph 7, bullet 11

> "... as specific in [RFC5245] ..."

"... as specified in ..."

Thanks!

- -- 
Troy Shields
Developer of Awesome | Jive Communications, Inc.
Jive.com | [email-masked]

---

From: Justin Uberti <[email-masked]>
Date: Fri, 30 May 2014 11:52:59 -0700

Thanks for the detailed and thorough review. Most of these suggestions are
simple and straightforward and we will adjust the document accordingly. I
addressed each comment individually below.

Look for a new rev of the document in github next week.

On Wed, May 28, 2014 at 9:07 PM, tshields <[email-masked]> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
>
>
> Which particular examples were most useful - the inline hand-wavy
descriptions, the API flows, or the example SDP?

> persuade me why JSEP is awesome (IMO).

Agree your wording is more direct. We'll rework this part.

> media streams ahead of time might be useful for context here, if one is
Sure, that makes sense. The typical case here is for an endpoint that wants
to be able to quickly set up a received call. The caller has the ability to
just start the call and leave it in the local-offer state instead.

> that follow, but I still don't feel like these questions are completely

This is an application decision; in JSEP's opinion, forking should overall
be avoided.

> restored from a server to the Javascript application running in the

The rehydration stuff has proved hard to get right. In the interest of
finishing this document, we have removed this section from JSEP. If there
is still sufficient interest in this in the future, we can revisit
rehydration in an extension document.

> replaced with "functionality that is".

Agreed.

> quite sure what "the document" refers to. I think rewording would be

Agreed.

> that never comes. Does JSEP care at all about this situation?

The sender of an answer can mark it as pranswer or answer, as can the
receiver. In practice though, I expect this to be done by the receiver, as
it deals with multiple SIP replies. In the end this is up to the app, and
JSEP is This is really up to the app. The things you mention are all issues
the app needs to be aware of.

> possible for application developers.

As accessors, I don't think this is really forcing any additional
complexity on the developer. And I think they are needed. See
https://github.com/rtcweb-wg/jsep/issues/16

> using a Lite ICE implementation.

Agree. We'll adjust accordingly.

> It's entirely arbitrary. Since they are meaningless, I figured using
similar meaningless values made this more clear than individual meaningless
values. We can add some text to make this clear.

> it is useful here as well.

Sounds good.

> so while maintaining this uniqueness constraint?

This is being tracked in https://github.com/rtcweb-wg/jsep/issues/41

> I think an example of when this would be useful might be nice for context.

I assume you are asking for when each of the individual constraints
parameters would be useful. That sounds helpful.

> it entirely more understandable.

Agreed. Constraints have been replaced with dictionaries, so we can clearly
refer to this as an integer var.

> breaking up would be beneficial.

OK.

> Should this be a SHOULD NOT or is it more just informational?

It's really just informational, to avoid any confusion. The actual BUNDLE
spec will have the exact guidance on this matter.

> would help make the situation a little clearer.

This is being tracked in https://github.com/rtcweb-wg/jsep/issues/8

> Should read "only after an offer".

yep

> "... the number of media streams for which to gather candidates."

ok, not a fan of ending a sentence with a preposition, I see. I'll go along
with this one.

> Should say, "... the media plane that is relevant."

yep

> controlled through this API."

Will think this over. Overall, want to be consistent in how the term 'API'
is used.

> "into which it is being bundled."

ok

> Space needed after RFC reference.

yep

> > "... that references the payload type fo the primary codec ..."

yep

> > "...as specific in [RFC5245]..."

yep

> "...should then be generated..."

yep

> "... and there is a local MediaStreamTrack that has been associated ..."

indeed, not my best work

> "... Section 3, MUST be supported."

yep

> "... the payload type of the primary..."

yep

> "... as specified in ..."

yep

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG/MacGPG2 v2.0.22 (Darwin)
> Comment: GPGTools - https://gpgtools.org
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iQIcBAEBCgAGBQJThrKEAAoJEPpHBvvPaXlQjwEQAJ0gOz+wj6YEp++/GD/fbifB
> 6tA/jXvcF1HHqLLilfHZof7bnWwjYGTzbCFaT21xnouXEb0x1ZEuL2nVvK5CU801
> sOJtOJtEavwkbRQffp8ECE4SMZnEUocRFEro1QtV1Lay5EwV86Kn7gqoJCOf8AVJ
> UuFLrPktJFX9bbWQEXrjHXDjFUj+zvSj23UKAKEvMoLJKoXvuAKadU+UoZcuNBDo
> SFoBSCsu91F4tE3InUn3uZlMt+GogeikfnPbmc8qdw+p7XzsztZf1oHa6eC8AZiT
> 6YHEnNHqG7klc9EO/Gr1lfdwTkbnyybbqRUEpQUNURlmqMOD8+Z3V8t3IWFxkdbU
> 61QMt8TG0N1AVEuIGGU4JaQfqF0kCVgb9H2+UE7hg3jXbT1lavmWLuOwecB79LTQ
> 4Iemyygz1DG4GmJ/vhpVy++azMQqjEWeRQbaXg5j5gvkUyjb1iAnNlNcl98ZqPli
> drcsvWraBldIEpxR3cEA3Cgj+dCbJXuGA6SOy6C6xciwib7ZRAeKL40XPdeaSnbg
> hDXFwuAMOPycpfzpmUIoTQVtAQt8iM2bNyiuVoPnOSFyx/YEh7FyBZg4C/8LW8ZT
> WiW08Jujr96l2b+8J4ajzRoHgGa7iPCajO3DLxJxncAEZyvQk8+nGNTTVVBoso1Q
> XstBoLY6o9nE91hnOqle
> =rkNA
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> rtcweb mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/rtcweb
>