Re: [rtcweb] Ben Campbell's Yes on draft-ietf-rtcweb-jsep-24: (with COMMENT)

[Justin Uberti <[e-mail]>]

Thanks for the detailed comments, will send a full response shortly.

On Mon, Dec 11, 2017 at 7:49 PM, Ben Campbell <[e-mail]> wrote:

> Ben Campbell has entered the following ballot position for
> draft-ietf-rtcweb-jsep-24: Yes
>
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
>
>
> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
>
>
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-rtcweb-jsep/
>
>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> I'm balloting "yes", but I have some minor comments and nits:
>
> Substantive:
>
> - General:
> I still find the edges around where SDP is and isn't required a bit vague.
> Section 3.3 says that the JSEP implementation internal representation is
> SDP.
> While I have trouble imagining implementing this otherwise, do we really
> mean
> to mandate the internals? Is there an assumption that said internal
> representation is _serialized_ SDP vs some abstract internal
> representation?
>
> Section 3.1 says that the signaling model is not specified. Is there an
> implicit assumption that, however things are signaled, the signaling
> process
> moves around serialized SDP? Or is it envisioned that one could write an
> application without serialized SDP occurring anywhere above the API?
>
> -5, throughout: There are a number of normative keywords used in
> constructions
> like "... MUST foo, as described in RFCXXX". That construction is ambiguous
> about whether it defines a normative requirement to do something, and the
> doing
> of that something is described in the RFC, or if describes the referenced
> RFC
> as defining the MUST.
>
> In general, it's best to avoid using 2119/8174 keywords to talk about
> external
> requirements, except as a direct quote. I think this is especially true
> here
> given the interdependencies between drafts in cluster 238. If in the
> future we
> update a dependency to change a normative requirement, we risk creating
> conflicts, and we make it unclear which text is authoritative.
>
> -5.1.1: I think the operative requirement is "MUST support" rather than
> "MUST
> indicate support". (While indication may also be required, it seems a
> consequence of "MUST support".)
>
> -5.1.2: " Any profile in the offer matching one of the following MUST be
> accepted:" Isn't the real requirement that any of the following must be
> interpreted the same, or otherwise in some specified way? I assume there
> may be
> completely unrelated reasons to reject an m-section, but the "MUST be
> accepted"
> language seems to overrule that.
>
> -5.4: "The SDP returned from createOffer or createAnswer MUST NOT be
> changed
>    before passing it to setLocalDescription."
> Is that a requirement on the JSEP implementation, or the javascript
> application? If the latter, is it appropriate to put normative
> requirements on
> the application? It seems like it would be better to normatively state the
> JSEP
> implementations's behavior when the application does something incorrect.
> (Which seems to be done further down the page.)
>
> -5.7: " The effect of rollback MUST be the same regardless of whether
> setLocalDescription or setRemoteDescription is called." Does that mean if I
> call setLocalDescription, then rollback with a call to
> setRemoteDescription,
> _both_ are rolled back?
>
> -5.8.3: The MUSTs in this section seem to be putting requirements on the
> SDP
> being parsed. Shouldn't they instead describe the parser's behavior based
> on
> that SDP input? The correctness of the SDP being parsed seems beyond the
> control of the implementation.
>
> -8, second paragraph: When describing how the javascript is untrusted, it
> may
> be worth mentioning that it may have been downloaded from an untrusted
> source.
>
> Editorial and Nits:
>
> -1.1: Please expand ICE and MCU on the first mention.
> s/config/configuration
> Sentence starting with "Through its abstraction of signaling...": Should
> that
> say "Though it abstracts signaling..."
>
> -2: Please consider using the boilerplate from RFC 8174. There are a
> number of
> instances of lowercase versions of normative keywords.
>
> -3.2, paragraph 2: The MUST seems like a statement of fact.
>
> -3.4.1, 2nd paragraph: Please expand or explain "mid" on first mention.
> (It is
> explained 3.5.2.1)
>
> -3.5.1, last paragraph: Inconsistent capitalization: "MID"
>
> -3.5.2.1: Please expand (or define) "ufrag" on first mention.
>
> -3.6.1, first paragraph: is "intersect" the correct verb here? Missing
> conjunction in "hardware decoder capababilities, local policy".
>
> -3.6.2, 2nd paragraph: s/"may be producing"/"may produce"
>
> -4.1.2: Please expand LS on first mention. (I can guess from context.
> Please
> don't make me :-)  )
>
> - 4.1.6: s/"codec/RTP/RTCP"/"codec, RTC, or RTCP"
> "for each SDP line, the generation of the SDP will follow the process
> defined
> for generating an initial offer from the document that specifies the given
> SDP
> line." While I worked out what that means, I found "document" to be
> ambiguous.
> At first I interpreted it as the "SDP document" rather than "the RFC".
> (Note
> this occurs several times in subsequent sections.)
>
> -4.1.8, third paragraph: ""pranswer" indicates that a description should be
> parsed as an
>    answer, but not a final answer"
> I think it would be more clear to say "... parsed as a provisional answer,
> rather than a final answer".
>
> -5.2.1: Paragraph starting with: " Each m= section, provided it is not
> marked
> as bundle-only, MUST generate..." The m= section is not the real actor
> here, is
> it?
>
>
> _______________________________________________
> rtcweb mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/rtcweb
>