Subject: [payload] AD Evaluation: draft-ietf-avt-rtp-isac-04

From: Richard Barnes <[email-masked]>
Date: Mon, 1 Apr 2013 13:06:13 -0400

Overall, this document looks in pretty good shape.  A couple of comments I
would like to get resolved before IETF LC:

Section 3.1., "Consult source code for details"
If the details are only specified in the source code, then the source needs
to be a normative reference.  And I would prefer to avoid that  :)  Better
to specify here how the BEI and FL fields are encoded.   Alternatively, if
the codec really does expect a combined BEI/FL field, you could specify
such a field here, opaque to the RTP layer.  However, you would at least
need to say how the recipient knows where this field starts and stops.

Section 3.2., "The length of the encoded data is variable and depends on the
signal characteristics and the target bit rate."
However, there's nothing in this section that tells a recipient how to
determine what this length is.

Section 3.3., "... verifying the CRC checksum ..."
Please specify which CRC function is to be applied, and how the CRC value
field is formatted.

Section 3.3., "If this value would exceed 255 at encoding..."
It sounds like the LEN field is a single octet unsigned integer.  Please
state that explicitly.

Section 9.2. Informative References.
Better path to source code would be "webrtc/
modules/audio_coding/codecs/isac"

Thanks,
--Richard

---

From: "Roni Even" <[email-masked]>
Date: Mon, 1 Apr 2013 21:03:14 +0300

Hi Richard,

As a general comment, for RTP payload specifications we do not require a
normative reference to the codec. We only require enough information that
will allow an RTP receiver to parse the information and move it to the
decoder that can be a black box. This allows us to publish RTP payload
specification also for codecs whose specifications are not publically
available.

Still in this case more information can be supplied.

As for the comments the authors should address them

Roni Even

From: [email-masked] [mailto:[email-masked]] On Behalf
Of Richard Barnes
Sent: 01 April, 2013 8:06 PM
To: [email-masked]
Cc: [email-masked]
Subject: [payload] AD Evaluation: draft-ietf-avt-rtp-isac-04

If the details are only specified in the source code, then the source needs
to be a normative reference.  And I would prefer to avoid that  :)  Better
to specify here how the BEI and FL fields are encoded.   Alternatively, if
the codec really does expect a combined BEI/FL field, you could specify such
a field here, opaque to the RTP layer.  However, you would at least need to
say how the recipient knows where this field starts and stops.

Better path to source code would be
"webrtc/modules/audio_coding/codecs/isac"

Thanks,

--Richard

---

From: Richard Barnes <[email-masked]>
Date: Mon, 1 Apr 2013 14:46:35 -0400

Sure. I didn't mean to imply that we need a reference to the codec, in fact
the opposite.  I was looking for this document to tell me how to parse out
the fields I would feed into the black box of the codec.  They describe the
fields, but it seems to me that they need more detail in order for people
to know how to parse them out of the payload.

--Richard

On Mon, Apr 1, 2013 at 2:03 PM, Roni Even <[email-masked]> wrote:

> Hi Richard,****
> available.****
> Still in this case more information can be supplied.****
>
> ** **
>
> As for the comments the authors should address them****
>
> ** **
>
> Roni Even****
>
> ** **
>
> *From:* [email-masked] [mailto:[email-masked]] *On
> Behalf Of *Richard Barnes
> *Sent:* 01 April, 2013 8:06 PM
> *To:* [email-masked]
> *Cc:* [email-masked]
> *Subject:* [payload] AD Evaluation: draft-ietf-avt-rtp-isac-04****
>
> ** **
>
> would like to get resolved before IETF LC:****
>
> ** **
>
> Section 3.1., "Consult source code for details"****
>
> If the details are only specified in the source code, then the source
> needs to be a normative reference.  And I would prefer to avoid that  :)
>  Better to specify here how the BEI and FL fields are encoded.
> Alternatively, if the codec really does expect a combined BEI/FL field, you
> could specify such a field here, opaque to the RTP layer.  However, you
> would at least need to say how the recipient knows where this field starts
> and stops.****
>
> ** **
>
> Section 3.2., "The length of the encoded data is variable and depends
> on the signal characteristics and the target bit rate."****
>
> determine what this length is.  ****
>
> ** **
>
> Section 3.3., "... verifying the CRC checksum ..."****
>
> field is formatted.****
> ** **
> Section 3.3., "If this value would exceed 255 at encoding..."****
>
> state that explicitly. ****
>
> ** **
>
> Section 9.2. Informative References.****
>
> modules/audio_coding/codecs/isac"****
>
> ** **
>
> Thanks,****
>
> --Richard****
>

---

From: "Roni Even" <[email-masked]>
Date: Tue, 2 Apr 2013 00:44:43 +0300

Hi Richard,

These fields are not part of the RTP header but part of the iSAC payload
header and I agree that it will be better to clarify them.

Roni

From: Richard Barnes [mailto:[email-masked]] 
Sent: 01 April, 2013 9:47 PM
To: Roni Even
Cc: [email-masked]; [email-masked]
Subject: Re: [payload] AD Evaluation: draft-ietf-avt-rtp-isac-04

Sure. I didn't mean to imply that we need a reference to the codec, in fact
the opposite.  I was looking for this document to tell me how to parse out
the fields I would feed into the black box of the codec.  They describe the
fields, but it seems to me that they need more detail in order for people to
know how to parse them out of the payload.

--Richard

Hi Richard,

Roni Even

Thanks,

--Richard