Subject: RE: [rohc] RE: CDRs for WGCLdraft-ietf-rohc-rfc3095bis-rohcv2-profiles-02

From: "Ghyslain Pelletier" <[email-masked]>
Date: Tue, 13 Nov 2007 17:15:07 +0100

Hello,
Thanks for your review. Sorry for the late reply ...
See inline.
Cheers,
///Ghyslain

Jin, Haipeng wrote:
> Hi, All,
> 
> My review for ROHCv2 text portion is attached to this message.
> 
> Best Regards,
> Haipeng
> -------------------------------------------------------------
> Document: draft-ietf-rohc-rfc3095bis-rohcv2-profiles-02.txt
> Reviewer: Haipeng Jin
> Status: WG Last Call
> 
> (convention used in comments:
> ---word---: remove the word identified,
> +++word+++: insert word suggested)
> 
> Technical ones:
> 1. Section 6.6.9, "The value of the field is decoded by calculating an
> approximation of the ---uncompressed--- +++scaled+++ value,", reason
> for the change is that "uncompressed" gives the impression that the
> value is the unscaled uncompressed value.

[GHPE] Ok
 
> 2. Section 6.6.9, "   - tsc_ref_advanced is an approximation of the
> ---uncompressed--- +++scaled+++ value of the field.", same reason.

[GHPE] Ok
 
> 3. Section 6.6.9, "
> The control field time_stride controls whether or not the
>    timer_based_lsb method is used in the
> ---CO header---
> +++compressed headers+++.  The decompressor

[GHPE] Ok

>    SHOULD send the CLOCK_RESOLUTION option if it receives a non-zero
>    time_stride value and it has not previously informed the compressor
>    that it supports timer-based compression using the CLOCK_RESOLUTION
>    option with a non-zero value.
> ---Whether the CLOCK_RESOLUTION is set to
>    a non-zero value or to a zero value is up to the implementation.---
> +++If the decompressor supports timer-based compression, then the
> decompressor includes a non-zero value when sending the
> CLOCK_RESOLUTION option. Otherwise, if the decompressor does not
> support timer-based compression, the decompressor sets the value of
> CLOCK_RESOLUTION option to zero. +++

[GHPE] Not Ok. TBC is optional to implement, and it is optional to use.
This was agreed on the list, and is no different than for RFC3095. There
is thus no reason to bind the use of TBC based on whether is it
implemented or not. Furthermore, from a pure protocol design
perspective, such a statement would not make sense.

[GHPE] My proposal is to leave the text unchanged.

>    The support and usage of timer-based compression is optional for
>    both the compressor and the decompressor;
> ---the compressor is never required
>    to set the time_stride control field to a non-zero value when it
>    has received a non-zero value for the CLOCK_RESOLUTION option.---
> +++The compressor only sets the time_stride control field to a
> non-zero value when it supports timer-based compression and wants to
> compress the timestamp value based on this mechanism. +++
> "

[GHPE] Not Ok. Same reasons as above. In addition, the wording
"supports" and "wants" does not have a meaning from a protocol
perspective, these are implementation-related choices.

[GHPE] My proposal is to leave the text unchanged.
 
> 4. Section 6.6.11, for ctrl_data_value, the text says msn is not
> included for RTP profiles (0x0101 and 0x0107). This is also true for
> ESP profile, so please add 0x0103.

[GHPE] Ok

> 
> 5. Section 6.9.2.2, "
> The ACKNUMBER-NOT-VALID option MUST NOT appear more than once in the
>    FEEDBACK-2 format ---and MUST NOT appear in the same feedback
> element as
>    the MSN option---, otherwise the compressor MUST discard the entire
>    feedback element.
> ", we don't have the MSN option any more.

[GHPE] This is now fixed.
 
> 6. Appendix B.3, "
> In addition to this, the compressor needs to estimate an upper bound
>    for the jitter between compressor and decompressor (Max_Jitter_CD).
>    This information may for example come from lower layers
> ---, but if such
>    information is not available, the compressor should use an upper
>    bound which is significantly higher than the link roundtrip
> time---. ". The reason for remove the extra statement is that I don't
> think there is a correlation between the link roundtrip time and the
> max jitter, so we should not be giving any incorrect guidance here.

[GHPE] Ok

> 
> Editorials:
> 1. Section 6.3, last sentence, the reference should be RFC4997 instead
> of RFC4995.

[GHPE] Ok

> 
> 2. Section 6.7, "
> time_stride_value: The value of this parameter should be set to
> 	the expected inter-arrival time between consecutive packets for
> 	the flow, and the selected value is implementation-specific.
> 	--It MUST NOT be set to a non-zero value,--
> 	++It MUST be set to zero,++
> 	unless the compressor has received a feedback message with the
> CLOCK_RESOLUTION option set to a non-zero value.  See also Section
> 6.6.9.
> ", There are two occurrences; just to make the sentence reads better.
> 

[GHPE] Ok

> 3. Section 6.7, right above udp_baseheader description:
> "
> This flag may only be set to 1 --either-- for the "co_common" packet
> format in the different profiles.
> "

[GHPE] Ok

> x. Appendix A.5, "
>    Padding
>       The use of this field is application-dependent, but when payload
>       padding is used it is likely to be present in most or all
>       packets. The field is classified as RC to allow for the case
>       where the value of this field ---is--- changes.
> "

[GHPE] Ok

> 
> X. Appendix A.5, "
> In the video case:
>          Between two consecutive packets, the timestamp will either be
>          unchanged or increase by a multiple of a fixed value
>          corresponding to the picture clock frequency.  The timestamp
>          can also decrease by a multiple of the fixed value for
>          certain coding schemes.
> 	---This increase---
> 	+++The change in timestamp value+++, expressed as a multiple of
> the
>          picture clock frequency, is in most cases
> 	---very limited--- +++within a very limited range+++.
> "

[GHPE] Ok

> 
> X. Appendix A.9, "
>    Source and Destination Addresses
>       These fields can be used to define the flow +++and+++ are not
> expected to change.
> "

[GHPE] Ok

> 
> X. Appendix A.10, "
>       The ICV is expected +++ to +++ behave unpredictably and is
> therefore
>       classified as IRREGULAR.
> "

[GHPE] Ok

> 
> 4. Appendix B.2, first paragraph, "
> Exactly how the compressor selects packet format is up to the
> implementation to decide, but the following is an example of how this
> process can be    performed for LSB-encoded fields---, though---
> +++through+++ the use of Window-based LSB encoding (W-LSB).
> "

[GHPE] Ok

> 
> 5. Appendix B.3, first paragraph, "
> This section gives guidelines on how the
>    compressor should select how many bits of timestamp LSBs need to
> +++be+++
>    transmitted.
> "

[GHPE] Ok

> 
> 
> 
> 
> _______________________________________________
> Rohc mailing list
> [email-masked]
> https://www1.ietf.org/mailman/listinfo/rohc

_______________________________________________
Rohc mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/rohc

---

From: "Jin, Haipeng" <[email-masked]>
Date: Fri, 16 Nov 2007 06:16:54 -0800

Hi, Ghyslain,

Thanks for the response, I removed all the agreed ones and have some
comments on the remaining two items.

Best Regards,
Haipeng

[Haipeng] We both agree that TBC is optional to implement and use. The
rewording I provided is simply to clarify that if the decompressor
supports TBC, then it includes non-zero value. Note that this is
different from bind the use of TBC to whether it is implemented or not,
the non-zero value is just an indication that the decompressor supports
TBC. Whether TBC is used or not is decided by the compressor. 

[Haipeng] I am not sure what you mean by protocol design issue. We use
the term "support" in RFC4995 and RFC3241 plenty of times. For example,
the RFC talks about "Define the set of profiles supported by the
decompressor" instead of saying "whether the decompressor includes a
certain profile id or not is up to the implementation".  If the end
point "supports" (if you allow me to use the term :-)) a profile, why
wouldn't it indicate the support of the profile? Similarly, if an end
point supports TBC, why wouldn't it indicate the support of this
capability? Again, this does not mean that the end point needs to make
use of TBC even if it supports the capability.

[Haipeng] I think this is the same issue as above. Does "never required"
have more meaning than "support" from a protocol design perspective? A
different way of stating the sentence based on the existing text you
used on the compressor side is: "
The compressor sets the time_stride control field to a non-zero value to
request CLOCK_RESOLUTION feedback from the decompressor if it has not
previously received any CLOCK_RESOLUTION option from the decompresor"

So that we have some corresponding text to "
>    option with a non-zero value.
".