[Gen-art] Genart last call review of draft-ietf-mmusic-rid-14

Robert Sparks <rjsparks@nostrum.com> Tue, 27 February 2018 20:54 UTC

Return-Path: <rjsparks@nostrum.com>
X-Original-To: gen-art@ietf.org
Delivered-To: gen-art@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 24B3712D87F; Tue, 27 Feb 2018 12:54:27 -0800 (PST)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Robert Sparks <rjsparks@nostrum.com>
To: gen-art@ietf.org
Cc: ietf@ietf.org, draft-ietf-mmusic-rid.all@ietf.org, mmusic@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 6.73.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <151976486707.28513.8133879591048964555@ietfa.amsl.com>
Date: Tue, 27 Feb 2018 12:54:27 -0800
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/gEJYSG9UOHluCqZl3ZwW7wlSJ1c>
Subject: [Gen-art] Genart last call review of draft-ietf-mmusic-rid-14
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.22
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 27 Feb 2018 20:54:27 -0000

Reviewer: Robert Sparks
Review result: Ready with Nits

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-mmusic-rid-14
Reviewer: Robert Sparks
Review Date: 2018-02-27
IETF LC End Date: 2018-03-30
IESG Telechat date: Not scheduled for a telechat

Summary: Ready with nits

Nits:

I could see implementor disagreement around what's allowed when modifying a
session driven by the statement in the introduction that "They do not relax any
existing descriptions." (where "They" here are the restrictions communicated
with this attribute). Please look for a way to make it clear that an updating
offer-answer round _can_ result in fewer restrictions than the original round
did, just not fewer restrictions than the description places on the media if
the "rid" extension is not present.

(Micronit): I don't think the "To be clear" in the first paragraph on page 5
helps. I also worry that "it" may not have a clear meaning in "Such
implementations must send it" later in that paragraph.

At the description of max-bpp on page 7, the last sentence is awkward. Do you
perhaps mean "These values MUST NOT be encoded with more than four digits to
the right of the decimal point."?

In the second paragraph of section 6, "its own unique 5-tuple" is arcane if the
reader hasn't read the rest of the rtcweb work. Could you provide a description
or a pointer to a description here?

At section 6.3, where you say 'For each "a=rid" line:', should you say 'For
each "a=rid" line that has not been discarded by the requirements in section
6.2:'?

I found "a non-empty union" to be a confusing description of the condition you
are trying to convey in section 8. (A union of two sets, at least one of which
is not empty is going to be non-empty). I'm not sure intersection is the right
word here either. Perhaps you could find a different way to characterize the
condition?

The BNF for rid-id allows rid-ids like "This-is_my-favorite" or
"Gm-Cqzkj4VHVD". But all the examples use single digits for rid-ids. I see the
statement that "the actual identifiers used for RIDs are expected to be
opaque". I strongly encourage putting some opaque ones in the examples.

Consider reminding people that ABNF quoted strings are case-insensitive, and
that the grammar as written will allow things like "MaX-WiDtH" and "rECv". If
that's not what you want, consider bringing in RFC7405.