Subject: [payload] OPS-Dir review of draft-ietf-payload-g7110-04

From: "Black, David" <[email-masked]>
Date: Wed, 24 Dec 2014 05:42:00 +0000

The -04 version of this draft is an improvement, but does not
resolve all the issues from the Gen-ART and OPS-Dir review of
the -03 version.  This is now reduced to an OPS-Dir review
(and email distribution), as Joel's holding the relevant "Discuss"
position.

The following issues are still open:

-- Major -- (these four are all now minor issues in -04)

> [A] Section 4.2.3 specifies a detailed decoding algorithm covering
> how G.711.0 decompression interacts with received RTP G.711.0 payloads.
> A corresponding encoding algorithm specification is needed on the
> sending side for G.711.0 compression interaction with RTP sending.
> The algorithm will have some decision points in it that cannot be
> fully specified, e.g., time coverage of the generated G.711.0 frames.

Section 4.2.2.1 does most of this.  It needs to include generation of
the "Prefix Code" octet that is described in Section 3.3.

> [B] The G.711.0 frame format is not specified here, making it very
> difficult to figure out what's going on when G.711.0 frames are
> concatenated.  A specific example is that the concept of a "prefix
> code" that occurs at the start of a G.711.0 frame is far too important
> to be hidden in step H5 of the decoding algorithm in Section 4.2.3.

This is mostly done in Section 3.3, but is a bit unclear:

   The first octet of a G.711.0 frame is called the
   "Prefix Code" octet; the value of this octet conveys how many G.711
   symbols the decoder is to create from a given G.711.0 input frame.
   The Prefix Code value of 0x00 is used to denote zero G.711 source
   symbols, which allows the use of 0x00 as a payload padding octet (to
   be described later).

The word "conveys" is problematic, as the "Prefix Code" octet cannot
contain the number of G.711 symbols, as that number could be 320,
which is larger than 255.  I'm guessing that the "Prefix Code" 
is an encoded value - if so, here's some suggested text:

OLD
   the value of this octet conveys how many G.711
   symbols the decoder is to create from a given G.711.0 input frame.
NEW (fill in value of 0xNN below)
   the encoded value in this octet indicates how many G.711
   symbols the decoder is to create from a given G.711.0 input frame
   (e.g., the value 0xNN indicates that 320 G.711 symbols are expected).

If there are invalid values of the "Prefix Code" octet, that will affect
step H5 in Section 4.2.3 where finding an invalid value should cause
decoding to stop.

> [C] The discussion of use of the SDP ptime parameter is spread out and
> imprecise (is SDP REQUIRED?, when is ptime REQUIRED, RECOMMENDED, or
> recommended? - it's not obvious).
>
[... snip ...]
> 
> I would suggest that a subsection be added, possibly at the end of
> Section 3, to gather/summarize all of the relevant ptime discussion
> in one place.  I suspect that the contents of this draft are mostly
> correct wrt ptime, but it's hard to figure out what's going on from
> the current spread-out text.  

Not done, although the sentence added (for [D]) on negotiation being
out of scope helps.  The first mention of "ptime" is in A4 in Section
3.2, with effectively no explanation of what ptime is:

         A G.711.0 decoder need not
         know what ptime is, as it is able to decompress the G.711.0
         frame presented to it without signaling knowledge.

A definition would help - somewhere before that first use of
ptime, define "ptime" using the words from RFC 4566:

         ptime: length of time in milliseconds represented by
         the media in a packet.  This is an attribute that can
	   be signaled via SDP [RFC 4566].

> [D] Backwards compatibility.
> 
> The problem here is that it's not clear that negotiation (e.g., via SDP)
> is required.  This sentence in Section 3.1 is a particular problem:
> 
>    G.711.0, being both lossless and stateless, may also be employed as a
>    lossless compression mechanism anywhere between end systems which
>    have negotiated use of G.711.
> 
> That's definitely wrong.  Use of G.711.0 when only G.711 has been
> negotiated will fail to interoperate correctly.

That problematic sentence is still there - I suggest a minor change to
remove the word "negotiated":

OLD
   G.711.0, being both lossless and stateless, may also be employed as a
   lossless compression mechanism for G.711 payloads anywhere between
   end systems which have negotiated use of G.711.
NEW
   G.711.0, being both lossless and stateless, may also be employed as a
   lossless compression mechanism for G.711 payloads anywhere between
   end systems that support use of G.711.

Then the added sentence on negotiation being out of scope (end of
paragraph that begins with that problematic sentence) makes more sense.

-- Minor --

> [F] Section 4.1:
> 
>       PT - The assignment of an RTP payload type for the format defined
>       in this memo is outside the scope of this document.  The RTP
>       profiles in use currently mandate binding the payload type
>       dynamically for this payload format.
> 
> Good start, but not sufficient - cite the "RTP profiles currently
> in use" and I would expect those citations to be normative references.

Not done.  Just listing those profiles with RFC citations will suffice.

> [G] Framing errors
> 
> Section 4 generally assumes that the G.711.0 decoder gets handed frames
> generated by the G.711.0 encoder and can't get disaligned.  I'm not
> convinced that this "just works" based on the text in the draft - major
> issue [B] is a significant reason why, and explaining that should help.

Not done - the concern here is what happens when the decoder reads a
G.711.0 payload octet (encoded from G.711 symbols) as a "Prefix Code"
octet?  I think resolving [B] will address most of this, especially if
that results in a "Not a valid Prefix Code value" error.

Thanks,
--David

> -----Original Message-----
> From: Black, David
> Sent: Wednesday, October 22, 2014 11:44 AM
> To: [email-masked]; Paul E. Jones ([email-masked]);
> [email-masked]; [email-masked]; [email-masked];
> General Area Review Team ([email-masked]); [email-masked]
> Cc: [email-masked]; [email-masked]; Black, David
> Subject: Gen-ART and OPS-Dir review of draft-ietf-payload-g7110-03
> 
> This is a combined Gen-ART and OPS-DIR review.  Boilerplate for both follows
> ...
> 
> I am the assigned Gen-ART reviewer for this draft. For background on
> Gen-ART, please see the FAQ at:
> 
> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
> 
> Please resolve these comments along with any other Last Call comments
> you may receive.
> 
> I have reviewed this document as part of the Operational directorate's ongoing
> effort to review all IETF documents being processed by the IESG.  These
> comments
> were written primarily for the benefit of the operational area directors.
> Document editors and WG chairs should treat these comments just like any other
> last call comments.
> 
> Document: draft-ietf-payload-g7110-03
> Reviewer: David Black
> Review Date: October 22, 2014
> IETF LC End Date: October 27, 2014
> IESG Telechat date: October 30, 2014
> 
> Summary: This draft is on the right track, but has open issues
>  		described in the review.
> 
> Process note: This is the second draft that I've reviewed recently
> that has been scheduled for an IESG telechat almost immediately
> following the end of IETF Last Call.  The resulting overlap of
> IETF LC with IESG Evaluation can result in significant last-minute
> changes to the draft when issues are discovered during IETF LC.
> 
> This draft describes an RTP payload format for carrying G.711.0
> compressed G.711 voice.  The details of G.711.0 compression are
> left to the ITU-T G.711.0 spec (which is fine), and this draft
> focuses on how to carry the compressed results in RTP and conversion
> to/from uncompressed G.711 voice at the communication endpoints.
> I found a few major issues and a couple of minor ones, although
> a couple of the major issues depend on a meta-issue, - the intended
> relationship of this draft be to the ITU-T G.711.0 spec.
> 
> In general, I expect IETF RFCs to be stand-alone documents that make
> sense on their own, although one may need to read related documents
> to completely understand what's going on.  For this draft, I would
> expect the actual compression/decompression algorithms to be left to
> the ITU-T spec, and this draft to stand on its own in explaining how
> to deploy G.711.0 compression/decompression with RTP.  If that
> expectation is incorrect, and this draft is effectively an RTP Annex
> to G.711.0 that must be read in concert with G.711.0, then the
> first two major issues below are not problems as they should be
> obvious in the G.711.0 spec, although the fact that this draft is
> effectively an Annex to G.711.0 should be stated.  Otherwise, those
> two major issues need attention.
> 
> -- Major Issues (4):
> 
> [A] Section 4.2.3 specifies a detailed decoding algorithm covering
> how G.711.0 decompression interacts with received RTP G.711.0 payloads.
> A corresponding encoding algorithm specification is needed on the
> sending side for G.711.0 compression interaction with RTP sending.
> The algorithm will have some decision points in it that cannot be
> fully specified, e.g., time coverage of the generated G.711.0 frames.
> 
> [B] The G.711.0 frame format is not specified here, making it very
> difficult to figure out what's going on when G.711.0 frames are
> concatenated.  A specific example is that the concept of a "prefix
> code" that occurs at the start of a G.711.0 frame is far too important
> to be hidden in step H5 of the decoding algorithm in Section 4.2.3.
> 
> [C] The discussion of use of the SDP ptime parameter is spread out and
> imprecise (is SDP REQUIRED?, when is ptime REQUIRED, RECOMMENDED, or
> recommended? - it's not obvious).
> 
> A specific example is that this sentence in Section 4.2.4 is an
> invitation to interoperability problems ("could infer" - how is that
> done and where do the inputs to that inference come from?):
> 
>    Similarly, if the number of
>    channels was not known, but the payload "ptime" was known, one could
>    infer (knowing the sampling rate) how many G.711 symbols each channel
>    contained; then with this knowledge determine how many channels of
>    data were contained in the payload.
> 
> I would suggest that a subsection be added, possibly at the end of
> Section 3, to gather/summarize all of the relevant ptime discussion
> in one place.  I suspect that the contents of this draft are mostly
> correct wrt ptime, but it's hard to figure out what's going on from
> the current spread-out text.  It looks like "ptime" could provide a
> cross-check on correctness of G.711.0 decoding - see minor issue [G]
> below.
> 
> This major issue [C] is independent of the relationship between this
> draft and the G.711.0 spec.
> 
> [D] Backwards compatibility.
> 
> The problem here is that it's not clear that negotiation (e.g., via SDP)
> is required.  This sentence in Section 3.1 is a particular problem:
> 
>    G.711.0, being both lossless and stateless, may also be employed as a
>    lossless compression mechanism anywhere between end systems which
>    have negotiated use of G.711.
> 
> That's definitely wrong.  Use of G.711.0 when only G.711 has been
> negotiated will fail to interoperate correctly.
> 
> A subsection of section 3 on negotiation and SDP usage would help here.
> 
> This major issue [D] is independent of the relationship between this
> draft and the G.711.0 spec.
> 
> -- Minor issues (3):
> 
> [E] Section 4.1:
> 
>    The only significant difference is that the
>    payload type (PT) RTP header field will have a value corresponding to
>    the dynamic payload type assigned to the flow.  This is in contrast
>    to most current uses of G.711 which typically use the static payload
>    assignment of PT = 0 (PCMU) or PT = 8 (PCMA) [RFC3551] even though
>    the negotiation and use of dynamic payload types is allowed for
>    G.711.
> 
> I would change "will have" to "MUST have" and add the following sentence:
> 
>    The existing G.711 PT values of 0 and 8 MUST NOT be used for G.711.0
>    content.
> 
> I'm suspect that this is obvious to the authors, but it'll help a reader
> who's not familiar with the importance of the difference between G.711
> and G.711.0 .
> 
> [F] Section 4.1:
> 
>       PT - The assignment of an RTP payload type for the format defined
>       in this memo is outside the scope of this document.  The RTP
>       profiles in use currently mandate binding the payload type
>       dynamically for this payload format.
> 
> Good start, but not sufficient - cite the "RTP profiles currently
> in use" and I would expect those citations to be normative references.
> 
> Would that be just RFC 3551 and RFC 4585 (both are already normative
> references), or are there more RTP profiles?
> 
> [G] Framing errors
> 
> Section 4 generally assumes that the G.711.0 decoder gets handed frames
> generated by the G.711.0 encoder and can't get disaligned.  I'm not
> convinced that this "just works" based on the text in the draft - major
> issue [B] is a significant reason why, and explaining that should help.
> 
> Some discussion should be added on why the G.711.0 decoder can't get
> disaligned wrt frame boundaries this can't happen, or what the G.711.0
> decoder will do when it discovers that it wasn't handed a complete G.711.0
> frame.  For example, this error case and how to deal with it are not
> covered by the algorithm in Section 4.2.3.
> 
> -- Nits/editorial comments:
> 
> Section 3.2:
> 
>    A6  Bounded expansion: Since attribute A2 above requires G.711.0 to
>          be lossless for any payload, by definition there exists at
>          least one potential G.711 payload which must be
>          "uncompressible".
> 
> The "by definition" statement assumes that every possible bit string is
> a valid G.711 input.  If that is correct, it should be explicitly stated.
> 
>    A8  Low Complexity: Less than 1.0 WMOPS average and low memory
>          footprint (~5k octets RAM, ~5.7k octets ROM and ~3.6 basic
>          operations) [ICASSP] [G.711.0].
> 
> Expand WMOPS on first use, and check for other acronyms that need to be
> expanded on first use.
> 
> Section 3.3:
> 
>    Since the G.711.0 output frame is "self-describing", a G.711.0
>    decoder (process "B") can losslessly reproduce the original G.711
>    input frame with only the knowledge of which companding law was used
>    (A-law or mu-law).
> 
> "companding law"?  The term "compression law" is used elsewhere in
> this draft, including two paragraphs earlier in this section - I suggest
> using "compression law" consistently.
> 
> Section 6:
> 
>    We note that something must be stored for any G.711.0 frames that not
>    received at the receiving endpoint, no matter what the cause.
> 
> "that not" -> "that are not"
> 
> Section 6.2:
> 
>    An entire frame of value 0++ or 0-- is expected to be
>    extraordinarily rare when the frame was in fact generated by a
>    natural signal (on the order of one in 2^{ptime in samples, minus
>    one}), as analog inputs such as speech and music are zero-mean and
>    are typically acoustically coupled to digital sampling systems.
> 
> This doesn't explain where the 2^{ptime in samples, minus one} order
> of magnitude estimation came from.  What assumption(s) is(are) being
> made about randomness and distribution thereof in the analog input?
> It might be simpler to delete the parenthesized text.
> 
> Section 11: Congestion Control
> 
> This section is mis-named, as it basically (correctly) says that there
> is nothing useful that can be done in G.711.0 compression to respond
> to congestion.  I would retitle this to "Congestion Considerations".
> 
> Are there opportunities to respond to congestion elsewhere, e.g.
> dynamically change the sampling rate?  If so, a sentence mentioning
> them would be good to add.
> 
> idnits 2.13.01 didn't find anything to complain about ;-).
> 
> --- Selected RFC 5706 Appendix A Q&A for OPS-Dir review ---
> 
> Most of these questions are N/A as this draft specifies a payload format
> for RTP, so most of the operations and management concerns are wrt RTP
> and SDP.
> 
> A.1.3.  Has the migration path been discussed?
> 
> No, see major issue [D] above.
> 
> A.1.4   Have the Requirements on other protocols and functional
>        components been discussed?
> 
> Only in part - major issues [C] and [D] call out shortcomings in the
> discussion of SDP interactions.
> 
> A.1.8   Are there fault or threshold conditions that should be reported?
> 
> Yes, the likelihood and consequences of framing problems at the G.711.0
> decoder (decoder is handed octet strings that are not G.711.0 frames
> generated by the encoder) should be discussed.  Major issue [B] needs
> to be resolved first, and then see minor issue [G].
> 
> A.2.  Management Considerations
> 
> I would expect that the media type registration (Section 5.1 of this draft)
> results in this new G.711.0 media type being usable in any relevant
> management model and/or framework that has some notion of media type.
> 
> A.3 Documentation
> 
> By itself, this compressed payload format does not look like a likely
> source of significant operational impacts on the Internet.
> 
> The shepherd's writeup indicates that an implementation exists.
> 
> Thanks,
> --David
> ----------------------------------------------------
> David L. Black, Distinguished Engineer
> EMC Corporation, 176 South St., Hopkinton, MA  01748
> +1 (508) 293-7953             FAX: +1 (508) 293-7786
> [email-masked]        Mobile: +1 (978) 394-7754
> ----------------------------------------------------