Review of draft-ietf-payload-melpe-04

Brian Carpenter <brian.e.carpenter@gmail.com> Mon, 26 December 2016 03:02 UTC

Return-Path: <brian.e.carpenter@gmail.com>
X-Original-To: ietf@ietf.org
Delivered-To: ietf@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 0107012957E; Sun, 25 Dec 2016 19:02:48 -0800 (PST)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Brian Carpenter <brian.e.carpenter@gmail.com>
To: gen-art@ietf.org
Subject: Review of draft-ietf-payload-melpe-04
X-Test-IDTracker: no
X-IETF-IDTracker: 6.40.3
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <148272136799.28234.15989368384638455485.idtracker@ietfa.amsl.com>
Date: Sun, 25 Dec 2016 19:02:47 -0800
Archived-At: <https://mailarchive.ietf.org/arch/msg/ietf/Lak7Ao-DPoaLAAiCq5fBQifLQwU>
Cc: draft-ietf-payload-melpe.all@ietf.org, ietf@ietf.org, payload@ietf.org, brian.e.carpenter@gmail.com
X-BeenThere: ietf@ietf.org
X-Mailman-Version: 2.1.17
List-Id: IETF-Discussion <ietf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ietf>, <mailto:ietf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ietf/>
List-Post: <mailto:ietf@ietf.org>
List-Help: <mailto:ietf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ietf>, <mailto:ietf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 26 Dec 2016 03:02:48 -0000

Reviewer: Brian Carpenter
Review result: Ready with Issues

Gen-ART Last Call review of draft-ietf-6tisch-minimal-17

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
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Document: draft-ietf-payload-melpe-04.txt
Reviewer: Brian Carpenter
Review Date: 2016-12-26
IETF LC End Date: 2017-01-13
IESG Telechat date:  

Summary: Ready with minor issues
--------

Major Issues: None
-------------


Minor issues:
-------------

> 3.1  MELPe Bitstream Definition
>
>   The total number of bits used to describe one frame of 2400 bps
>   speech is 54, which fits in 7 octets (with two unused bits). For
the
>   1200 bps speech the total number of bits used is 81, which fits in
11
>   octets (with seven unused bits). For the 600 bps speech the total
>   number of bits used is 54, which fits in 7 octets (with two
unused
>   bits).  Unused bits are coded as described in 3.3 in support of
>   dynamic bit-rate switching.

It would help the reader if the last sentence said something like:

   Unused bits, shown below as RSVA, RSVB, etc., are coded as
described
   in 3.3 in support of dynamic bit-rate switching.

> 3.3  Multiple MELPe frames in a RTP packet
...
>   When dynamic bit-rate switching is used and more than one frame
is
>   contained in a RTP packet, it is recommended to inspect the coder
>   rate bits contained in the last octet.  If the coder bit rate
>   indicates a Comfort Noise frame, then inspect the third last
octet
>   for the coder bit rate.  All MELPe speech frames in the RTP
packet
>   will be of this same coder bit rate.

I agree with the AD review that this should be RECOMMENDED.

> 4.2  Mapping to SDP
...
>   Alternative encoding name types,
>   "MELP2400", "MELP1200", and "MELP600", may be used in SDP to
convey
>   fixed bit-rate configurations. 

I think that should be MAY. If you really want to discourage this
usage,
you would need to say SHOULD NOT. Lower-case "may" is unclear in this
case.

> 6  Packet Loss Concealment

The "may" and "recommended" in this section are unclear - should they
be MAY and RECOMMENDED?

According to the writeup "There are existing implementations." It's a
shame
that there is no Implementation Status section (RFC 6982).

Nits:
-----

"declaritive" should be "declarative" (twice)

> 3.4  Congestion Control Considerations
>
>   The target bitrate of MELPe can be adjusted at any point in time,
>   thus allowing efficient congestion control.  Furthermore, the
amount

I would rephrase "efficient congestion control", because at present
the
word "efficient" isn't really justified by the text. How about
"thus allowing straightforward congestion management"?

>   of encoded speech or audio data encoded in a single packet can be
>   used for congestion control, since the transmission rate is
inversely
>   proportional to the packet duration.

Make that "the packet rate", because "transmission rate" could refer
to the
bit rate or the packet rate. At the moment the reader might miss that
until
the following sentence.