Re: draft-ietf-rmt-bb-fec-ldpc-06 LC comments

... and these comments for the LDPC I-D.

  Vincent


Alfred Hoenes a écrit :
> Hello,
> after studying the Internet-Draft in IETF LC authored by you,
>                 draft-ietf-rmt-bb-fec-ldpc-06,
> I'd like to submit a few comments, pointing out a few textual
> flaws I found in that memo that should be addressed before
> publicaton as an RFC.
> 
> Due to the observed text sharing with the related I-D,
> draft-ietf-rmt-bb-fec-rs-04, some overlap of issues arises.
> For brevity, I quote from my comments on that draft sent yesterday.
> 
> The items below are presented in textual order.
> To give more context, sometimes I quote larger blocks of text
> literally and show the replacement proposed using the shorthand
> notation:
> 
>    <original draft text>
> ---
>    <modified text>
> 
> I use change bars ('|' in column 1) and occasionally
> up/down pointing marker lines ('^^^'/'vvv') to emphasize
> the location of textual issues and/or proposed corrections.
> 
> 
> (1)  Section 4.2.2 -- typo
> 
> Correct the first line of that section:
> 
>    The following elements MUST be defined with the present FEC Scheme:
> ---
>    The following elements MUST be defined with the present FEC Schemes:
>                                                                      ^
> Rationale:
>   Multiple schemes in this draft, given unified treatment in this
>   section!  -- May be legacy from text sharing :-)
> 
> 
> (2)  Section 4.2.4.2 -- clarification of potentially misleading text
> 
> ---> same as for draft-ietf-rmt-bb-fec-rs-04  <---
> 
>> The last paragraph of Section 4.2.4.2 says:
>>
>> |  After Base64 encoding, the 2 bytes of the FEC OTI Scheme Specific
>>    Information are transformed into a string of 4 printable characters
>> |  (in the 64-character alphabet) and added to the FEC-OTI-Scheme-
>>    Specific-Info attribute.
>>
>> a) In the first line, "After" is misleading; "During" would be
>>    appropriate; I propose to use "Via" for brevity.
>> b) In the third line, "and added" does not give the sentence the
>>    proper sense; I porpose to use "that is added" instead.
>>
>> Taking altogether, the paragraph should say:
>>
>> |  Via Base64 encoding, the 2 bytes of the FEC OTI Scheme Specific
>>    Information are transformed into a string of 4 printable characters
>> |  (in the 64-character alphabet) that is added to the FEC-OTI-Scheme-
>>    Specific-Info attribute.
> 
> 
> (3)  Section 5.6 -- typo in code for 'rand()'
> 
> Please correct the following code fragment (shown with some context):
> 
>            lo = 16807 * (seed & 0xFFFF);
>            hi = 16807 * (seed >> 16);  /* binary shift to right */
> |          lo += (hi & 0x7FFF) < < 16; /* binary shift to left */
> ---                            ^^^
>            lo = 16807 * (seed & 0xFFFF);
>            hi = 16807 * (seed >> 16);  /* binary shift to right */
> |          lo += (hi & 0x7FFF) << 16;  /* binary shift to left */
> 
> 
> (4)  Section 6.2 -- inconsistency in code
> 
> In the code block with the headline comment,
> 
>       /* Add extra bits to avoid rows with less than two "1s".
>        * This is needed when the code rate is smaller than 2/5. */
> 
> the line,
> |             e = matrix_insert_entry(i, j);
>               ^^^^
> comes to surprise and is inconsistent with other parts of that code:
> matrix_insert_entry() is used as void otherwise.
> So please change that line to say:
> 
> |             matrix_insert_entry(i, j);
> 
> 
> (5)  Section 7.2 -- surprising code -- please check
> 
> In the inner loop of the code in Section 7.2, the line,
> 
>               j = rand(j);
> 
> is a bit surprising due to its 'contracting' effect
> (but it might well be correct).
> I do not have the time now to go 'back to the roots' for
> verification, so please check again.  Thanks!
> 
> 
> (6)  Section 11.1 -- outdated reference
> 
> Apparently, ref. [2] has been published as RFC 5052 in September.
> Thus, this ref. should now be updated accordingly, offloading work
> for the RFC-Ed.
> 
> 
> Best regards,
>   Alfred H�nes.
>