Subject: Re: [avtext] Comments on draft-ietf-avtext-mixer-to-client-audio-level-02

From: Emil Ivov <[email-masked]>
Date: Tue, 05 Jul 2011 00:47:42 +0200

Hey Magnus, Keith, all,

We have just submitted new versions of the level drafts that we hope
would address most of the comments expressed on the list.

Diff2 available here:
http://tools.ietf.org/rfcdiff?url2=draft-ietf-avtext-mixer-to-client-audio-level-03.txt
http://tools.ietf.org/rfcdiff?url2=draft-ietf-avtext-client-to-mixer-audio-level-03.txt

Comments inline:

Magnus Westerlund commented on mixer-to-client:
> Hi,
> 
> I reviewed the draft and have some comments.

Thanks!

> 1. The intended status says Informational. This document is intended for
> standards track.

Fixed.

> 2. Section 3, paqe 4:
> "   Note that in some cases a mixer may be sending an RTP audio stream
>    that only contains audio level information and no actual audio.
>    Updating a (web) interface conference module may be one reason for
>    this to happen."
> 
> My question would what value you stick into the payload type field if
> you have no payload only header? It is not obvious that you can do this
> without configuring a payload format that either is valid for a zero
> length payload or define a specific for that case.

Agreed. Removed.

> 3. Section 4 and 5. I find the definition confusing, especially as
> important rules of the actual format comes in later sections.
>  a) First of all it is not written in generic engough way to support
> both the short and long format.
>  b) The actual extension formats that is the payload is in the next
> section. Including important rules, like that the CSRC list shall be
> mapped against the level list.
> 
> I would recommend that Section 4 and 5 is merged and that a more ordered
> structure to the definition of the header extensions is made. Where
> field values are clearly defined including explicit length in bits. And
> where the rules around it is separated from the basic field definitions.

OK, we've merged sections 4 and 5 into a single "Audio Levels" section.
We've described use of the extension with both the one-byte and the
two-byte header formats and added example for both of them.

We've also modified the corresponding section in the client-to-mixer
draft so that both are now almost identical, which is also what Keith
advised in an earlier exchange.

> 4. Section 5. I think the dBov definition needs to be really clear. Yes,
> the convention is amplitudes are squared when doing dB calculation. But
> it is not clear if this is done for the single highest value in the
> measured range, or if it is done as an average over the measured range.
> Please provide a definition or a reference.

Well, we've mostly been using it as an average. A root mean square to be
precise. We've modified one of the paragraphs to read as follows:

    The audio level header extension only carries the level of the
    audio in the RTP payload of the packet it is associated with, with
    no long-term averaging or smoothing applied. That level is measured
    as a root mean square of all the samples in the measured range.

We hope that this text, combined with the reference implementation
should clear all doubts. However, to be perfectly honest, I am not
completely convinced that we should specify this. Maybe we could have
this as RECOMMENDED leaving the door open to alternative measurement
mechanisms?

> 5. Section 5 and Section 3:
> 
> I would note that the folllowing text in section 5:
>    The audio level header extension only carries the level of the audio
>    in the RTP payload of the packet it is associated with, with no long-
>    term averaging or smoothing applied.
> 
> Makes it impossible for the following to be done:
> Note that in some cases a mixer may be sending an RTP audio stream
>    that only contains audio level information and no actual audio.
>    Updating a (web) interface conference module may be one reason for
>    this to happen.
> 
> This as there is no duration of audio indicated, only a first sample.

Agreed. Text already removed as per comment 2.

> 6. Section 5:
>   "The audio level for digital silence, for example for a muted audio
>    source, MAY be represented as 127 (-127 dBov), regardless of the
>    dynamic range of the encoded audio format."
> 
> I don't like this MAY. To me it appears to only complicate
> implementations. Either you mandate the behavior so that one can trust
> sources muted by the application to always carry value 127. Or one
> anyway need to have noise level detector so that one can determine when
> there is signal above the noise level. One may have to have that anyway,
> but I think we should consider if we want clear MUTED indication here or
> not.

You are right. Mandating it would probably be best. Changed to MUST.

> 7. Section 6.
> "Presence of the above attribute in the SDP description of a media
>    stream indicates that some or all RTP packets in that stream would
>    contain the audio level information RTP extension header."
> 
> To me this sentence is a mix between a usage rule:
> "some or all packets may contain the header extension" and a signaling
> rule: The presence of the attribute indicates the usage of the audio
> level header extension.
> 
> In addition it not is the attribute per say that indicate it, it is the
> attribute with the given URI is used to indicate that a particular value
> is bound to identify the header extension defined in this doc.

OK. Changed to:

    Presence of the above attribute in the SDP description of a media
    stream indicates that RTP packets in that stream, which contain the
    level extension defined in this document, will be carrying them
    with an ID of 7.

If this doesn't work then we can simply remove it. This is, after all,
only comfort text meant to assist implementors. Rules of use and
signalling are inherited from 5285 anyway.

> 8. Section 6.
>   "This speicification does not define use of the audio level extensions
>    in video streams.  Therefore, the extension defined in this document
>    SHOULD NOT be advertised in anything but audio streams."
> 
> First spelling error of specification.

Fixed.

> Secondly, we also have media types like text, where I also don't see a
> logical mapping. Thus I wonder if this should be formulated in a way
> that avoids being media specific. I also wonder if there is any point in
> leaving the hole with "SHOULD NOT"? Either skip using that and formulate
> that it is only defined for audio streams, or some other way?

OK, so how about:
    This specification only defines use of the audio level
    extensions in audio streams. They MUST NOT be advertised with
    other media types such as video or text for example.

> 9. Section 7:
> I am missing a reference to the VBR consideration that I think contains
> a much better and complete explanation to the issue with just energy
> level or similar phonetic structure indicators.

Added.

Magnus Westerlund commented on mixer-to-client:
> 1. Section 3.
> "The two-byte header defined in RFC 5285 [RFC5285] may also be used."
> 
> I think that it should be stated up front that header extensions per
> definitions can be used with either of the header formats. In addition I
> think one needs to separate a bit clearer what is framework and what is
> the new parts.

Addressed as with the mixer-to-client draft.

> 2. Section 3. I think the dBov definition needs to be really clear. Yes,
> the convention is amplitudes are squared when doing dB calculation. But
> it is not clear if this is done for the single highest value in the
> measured range, or if it is done as an average over the measured range.
> Please provide a definition or a reference.

Addressed as with the mixer-to-client draft.

> 3. Section 5:
>   "The audio level for digital silence, for example for a muted audio
>    source, MAY be represented as 127 (-127 dBov), regardless of the
>    dynamic range of the encoded audio format."
> 
> I don't like this MAY. To me it appears to only complicate
> implementations. Either you mandate the behavior so that one can trust
> sources muted by the application to always carry value 127. Or one
> anyway need to have noise level detector so that one can determine when
> there is signal above the noise level. One may have to have that anyway,
> but I think we should consider if we want clear MUTED indication here or
> not.

Mandated as with the mixer-to-client draft.

> 4. Section 3:
>    In addition, a flag bit (labeled V) indicates whether the encoder
>    believes the audio packet contains voice activity (1) or does not
>    (0).  The voice activity detection algorithm is unspecified and left
>    implementation-specific.
> 
> Would it be of interest to have an signalling parameter for an
> implementation to indicate its algorithm?

My personal opinion is that, if this is deemed necessary, it can
probably be handled in a separate draft at a later stage. This would
also allow some level of negotiation, again, if people find this
necessary. Any other thoughts?

> 5. Section 8.2:
> I think both the encrypted header extension and the VBR audio is likely
> normative references as they either is recommend solution or a
> description of the issue.

I don't know. Header encryption is only one way of handling this.
There's also DTLS, IPsec and even VPN. So in that respect it seems to me
that this method is best seen here as an example.

Keith commented on mixer-to-client
> Section 6.
> 
>    This speicification does not define use of the audio level extensions
>    in video streams.  Therefore, the extension defined in this document
>    SHOULD NOT be advertised in anything but audio streams.
> 
> Typo: "specification"

Fixed.

Keith commented on client-to-mixer
> Section 3.
> 
>    Implementations MAY choose to measure audio levels prior to encoding
>    them in the payload carried in the RTP payload, e.g. on raw linear
>    PCM input.
> 
> "MAY" defines an option. Is this really defining an option or just indicating a possibility, as in "can".

Also raised by Magnus. Reply above.

Cheers,
Emil