Last Call Review of draft-ietf-payload-g7110-03
review-ietf-payload-g7110-03-genart-lc-black-2014-10-22-00
Request | Review of | draft-ietf-payload-g7110 |
---|---|---|
Requested revision | No specific revision (document currently at 06) | |
Type | Last Call Review | |
Team | General Area Review Team (Gen-ART) (genart) | |
Deadline | 2014-10-27 | |
Requested | 2014-10-16 | |
Authors | Michael A. Ramalho , Paul Jones , Noboru Harada , Muthu Arul Mozhi Perumal , Miao Lei | |
I-D last updated | 2014-10-22 | |
Completed reviews |
Genart Last Call review of -03
by David L. Black
(diff)
Secdir Last Call review of -03 by Steve Hanna (diff) Opsdir Last Call review of -03 by David L. Black (diff) |
|
Assignment | Reviewer | David L. Black |
State | Completed | |
Request | Last Call review on draft-ietf-payload-g7110 by General Area Review Team (Gen-ART) Assigned | |
Reviewed revision | 03 (document currently at 06) | |
Result | On the right track | |
Completed | 2014-10-22 |
review-ietf-payload-g7110-03-genart-lc-black-2014-10-22-00
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 david.black at emc.com Mobile: +1 (978) 394-7754 ----------------------------------------------------