Subject: [xrblock] Last Call Review: draft-ietf-xrblock-rtcp-xr-post-repair-loss-count-07

From: Tom Taylor <[email-masked]>
Date: Wed, 24 Dec 2014 21:41:51 -0500

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.

Document: draft-ietf-xrblock-rtcp-xr-post-repair-loss-count-07
Reviewer: Tom Taylor
Review Date: 2014-12-24
IETF LC End Date: 2014-12-26
IESG Telechat date: 2015-01-08

Summary: This document is mostly to publish as a Proposed Standard, with 
minor issues verging on editorial corrections.

Major issues: None.

Minor issues:

1) Second paragraph of Introduction: last sentence introduces the 
"repaired loss count". I found this confusing. Figure 1 shows you really 
meant "unrepaired loss count".

2) First paragraph of Section 3, third sentence: the ending currently reads:
    "Some packets will not be repaired in the current
    RTCP interval."
I think your point is that some of these could be repaired later, so I 
suggest the text should be extended:
    "Some packets will not be repaired in the current
     RTCP interval, but could be repaired later."

3) I question whether RFCs 4588 and 5109 are normative for this 
document, but I know this sort of thing is a difficult decision and I'll 
accept whatever the final outcome is.

4) Appendix A: The "Units of Measurement" are packets.

Nits/editorial comments:

1) First paragraph of introduction:
       s/contains/contain/ at end of first line.

Fourth sentence:

OLD

    However, this metric is measured on media stream before
    any loss repair mechanism, e.g., retransmission [RFC4588] and Forward
    Error Correction (FEC) [RFC5109], is applied.

NEW

    However, this metric is measured on the media stream before
                                        ^^^
    any loss repair mechanism, e.g., retransmission [RFC4588] or Forward
                                                              ^^
    Error Correction (FEC) [RFC5109], is applied.

I would break this paragraph into three parts. The first three sentences 
introduce the existing metric. The next two sentences, from "However, 
..." to "... [RFC3550].", introduce the problem. The following two 
sentences, from "Consequently ..." through "... higher overhead.", give 
an attempted solution. The final sentence, I think, is the motivator for 
this document and I would merge it into the next paragraph.

2) First paragrpah of Section 3: again, I suggest it be broken into 
three parts. The second part would begin with "Thus it is RECOMMENDED 
...", and the third with "The sequence number range ...".

3) The sentence preceding Figure 1 should identify "Figure 1" explicitly 
(via XML2RFC cross-reference if that is what you are using). It should 
also note that bit positions are given in octal -- the first time I've 
seen that in a document, but it's OK with me.

5) Section 5, third line: s/confidentially/confidentiality/

---

From: Tom Taylor <[email-masked]>
Date: Thu, 25 Dec 2014 08:46:25 -0500

Replies with [PTT] below.

On 25/12/2014 3:21 AM, Huangyihong (Rachel) wrote:
> Hi Tom,
>
> Thank you for the nice Gen-ART review. Please see my replies inline starting with "[Rachel]".
>
> BR,
> Rachel
>
>
>> -----Original Message-----
>> From: Tom Taylor [mailto:[email-masked]]
>> Sent: Thursday, December 25, 2014 10:42 AM
>> To: Gen Art; [email-masked];
>> [email-masked]; [email-masked]; Dan Romascanu
>> Subject: Last Call Review: draft-ietf-xrblock-rtcp-xr-post-repair-loss-count-07
>>
>> I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART,
>> please see the FAQ at
>>
>> Please resolve these comments along with any other Last Call comments you
>> may receive.
>>
>> 1) Second paragraph of Introduction: last sentence introduces the "repaired
>> loss count". I found this confusing. Figure 1 shows you really meant "unrepaired
>> loss count".
>
> [Rachel]: This report block introduces two metrics : "unrepaired loss count" which is specified as the total number of packets finally lost after applying loss-repair methods. And "repaired loss count" is specified as the total number of packets which are fully repaired after applying loss-repair methods. And in this sentence, I meant "repaired loss count" because this metric is used to help calculating pre-repair loss count (pre-repair loss count = unrepaired loss count + repaired loss count). Maybe the name of "unrepaired loss count" is confusing. Would it better changing to "post-repair loss count"?

[PTT] Yes, "post-repair loss count" sounds good.
>> I think your point is that some of these could be repaired later, so I suggest the
>> text should be extended:
> [Rachel]: Right. Will fix it in the new version.
>
>> 3) I question whether RFCs 4588 and 5109 are normative for this document,
>> but I know this sort of thing is a difficult decision and I'll accept whatever the
>> final outcome is.
>
> [Rachel]: I agree with you that these two should be informative.
>>
> [Rachel]: According to RFC6390, Section 5.4.5, there's an example to show how to define a metric. Here's how it defines the "Units of Measurement":
> "
> Units of Measurement:  This metric is expressed as a fixed-point number with the binary point at the left edge of the field.  For example, a metric value of 12 means a loss rate of approximately 5%.
> "
> I consulted it to specify mine.
>
[PTT] But what you are measuring according to your description and text 
in the preceding body of the document is number of packets, not a loss 
rate as shown in the RFC 6390 example. So what you should say is:

   "This metric is expressed as a 16-bit unsigned integer giving the 
number of packets ...".
> [Rachel]: Okay. All the above nits will be fixed.
>
>>
>> introduce the existing metric. The next two sentences, from "However, ..." to
>> "... [RFC3550].", introduce the problem. The following two sentences, from
>> "Consequently ..." through "... higher overhead.", give an attempted solution.
>> The final sentence, I think, is the motivator for this document and I would
>> merge it into the next paragraph.
>
> [Rachel]: Good suggestion.
>
>>
>> 2) First paragrpah of Section 3: again, I suggest it be broken into three parts.
>> The second part would begin with "Thus it is RECOMMENDED ...", and the
>> third with "The sequence number range ...".
>
> [Rachel]: Okay. So you're suggesting to break the big paragraph into 3 small ones, right?
>
[PTT] Yes.
>>
>> 3) The sentence preceding Figure 1 should identify "Figure 1" explicitly (via
>> XML2RFC cross-reference if that is what you are using). It should also note that
>
> [Rachel]: Okay. Will do.
>
>> bit positions are given in octal -- the first time I've seen that in a document, but
>> it's OK with me.
>
> [Rachel]: This draft is just an RTCP XR extension to augment the report blocks defined in RFC3611. So the format is just in a manner consistent with other RTCP XR packets. All the RFCs produced in XRBLOCK wg specifying new report blocks have the same pattern. That's why I think it doesn't need to be clarified in this draft.
>
>>
> [Rachel]: Okay.

---

From: Varun Singh <[email-masked]>
Date: Thu, 25 Dec 2014 19:32:34 +0200

Hi Tom,

Thanks for reading the draft and the feedback.

> On 25 Dec 2014, at 15:46, Tom Taylor <[email-masked]> wrote:
> 

This is a very good recommendation, we should replace other instances of unrepaired loss with post-repaired, as well. Including in the block format. 

---
http://www.netlab.tkk.fi/~varun

---

From: Jari Arkko <[email-masked]>
Date: Wed, 7 Jan 2015 12:42:11 +0200

Thanks for your review Tom  (and on Dec 25, of all days!)

Rachel et al - thanks for the responses and edits. Are we happy now, and are all the necessary changes in the new -08 version?

Jari

On 25 Dec 2014, at 04:41, Tom Taylor <[email-masked]> wrote:

> Summary: This document is mostly to publish as a Proposed Standard, with minor issues verging on editorial corrections.
> 
> Major issues: None.
> 
> Minor issues:
> 
> 1) Second paragraph of Introduction: last sentence introduces the "repaired loss count". I found this confusing. Figure 1 shows you really meant "unrepaired loss count".
> 
> I think your point is that some of these could be repaired later, so I suggest the text should be extended:
> 3) I question whether RFCs 4588 and 5109 are normative for this document, but I know this sort of thing is a difficult decision and I'll accept whatever the final outcome is.
> 
> I would break this paragraph into three parts. The first three sentences introduce the existing metric. The next two sentences, from "However, ..." to "... [RFC3550].", introduce the problem. The following two sentences, from "Consequently ..." through "... higher overhead.", give an attempted solution. The final sentence, I think, is the motivator for this document and I would merge it into the next paragraph.
> 
> 2) First paragrpah of Section 3: again, I suggest it be broken into three parts. The second part would begin with "Thus it is RECOMMENDED ...", and the third with "The sequence number range ...".
> 
> 3) The sentence preceding Figure 1 should identify "Figure 1" explicitly (via XML2RFC cross-reference if that is what you are using). It should also note that bit positions are given in octal -- the first time I've seen that in a document, but it's OK with me.
> 
> _______________________________________________
> Gen-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art