[AVT] Comments on draft-ietf-avt-rtp-ac3-03.txt

[Magnus Westerlund <[e-mail]>]

Hi,

I have reviewed the AC3 payload format draft and have a few comments. In 
general I think the solution is exemplary in its simplicity.

Please remember that these are my opinions and suggestions. Text can be 
changed, or I can be wrong. So please discuss any of the proposed 
changes that you are not comfortable with. If you don't understand my 
comment or motivations, please send my a email.

1. Status of this memo: You need to update the boilerplate. Please use 
the text from RFC 3978 and 3979.

2. In section 2.1 I think it would be good with some additional text 
enumerating or a table showing how the frame size in time varies with 
the possible sampling rates. That so one easier can understand this 
without the need to calculate. Especially considering that you have the 
ptime and maxptime attributes that can be used with multiples of these time.

3. Section 3, timestamp: I think the enumeration of the sampling rate 
should using "or" instead of "and". The reason I comment on this is that 
a defined payload type and thus a single payload can't use different 
sampling frequencies . If different sampling frequencies needs to be 
supported on the same connection multiple payload types must be defined.

4. Section 4. I would propose that a normative statement is added saying 
how the payload is structured. I think it easily can be done by 
modifying the third sentence in the first paragraph:

OLD:
"To simplify the implementation of RTP receivers,
each RTP packet MUST contain an integral number of complete AC-3
frames, or one fragment of an AC-3 frame. "

NEW:
"Each RTP payload MUST start with the 2-byte payload header followed by 
  an integral number of complete AC-3 frames, or a single fragment of an 
AC-3 frame."

5. Section 4.1: MBZ:
"Must Be Zero (MBZ): Bits marked MBZ MUST have the value zero and are
reserved."
First, move it up before Frame Type so that the fields comes in the 
order they are seen in the figure.
Secondly: If these are going to be useful the definition should be 
tighten. Because now it is not specified that a receiver should ignore 
these fields. Thus the moment one tries to use these bits for something 
else which is backwards compatible but provides enhancements it fails. 
But if you point out that these bits must really be ignored with a 
normative "SHALL" then things are fine:

New Text:
"Must Be Zero (MBZ): Bits marked MBZ SHALL be set to the value zero and 
SHALL be ignored by receivers. The bits are reserved for future extensions."

6. Section 4.2:
"If the size of an AC-3 frame exceeds the MTU size, the
frame SHOULD be fragmented."

Could be clarified to:
"If the size of an AC-3 frame exceeds the MTU size, the
frame SHOULD be fragmented at RTP level."

7. Section 4.2, second paragraph: Must it be fragmented at 5/8th frame 
length exactly or anywhere on or beyond the 5/8th marker.

8. Section 4.2: I think it needs to be clarified a few things on how one 
perform the fragmentation:
"The fragmentation MAY be performed at any byte boundary in the frame. 
RTP packets containing fragments of the same audio frame SHALL be sent 
in consecutive order, from first to last fragment. This enables a 
receiver to assembly the fragments in correct order upon reception of 
all fragments."

9. The media types registration is using an old template. I would 
recommend using the new template in 
http://www.ietf.org/internet-drafts/draft-freed-media-type-reg-04.txt. 
The reason is that we are registering media types, not MIME media types.

10. Section 5.1, subtype name: Are there is a reason why this is 
registered in the vendor space, instead of in the general IETF tree? As 
we are having the specification of the payload format in IETF they are 
generally approved registered in the standards tree. I am especially 
concerned how we are going to handle the ownership/change controlling of 
this type.

11. Section 5.1: I think the channels parameter should point out 
explicitly that it doesn't follow the channel order scheme in RFC 3551 
and instead point at the actual scheme used to assign channel order.

12. Section 5.1: Ptime: Why isn't the normal definition in SDP (RFC 
2327) referenced here?

13. Section 5.1: Maxptime: Why isn't the definition in RFC 3267 
referenced? IF sdp-new is published before this document, one could 
reference that.

14. Section 5.1: I think we should add the "Restriction on usage" 
heading in the template and there specify "This type is only defined for 
transfer via RTP." instead of having it in the encoding consideration 
section.

15. Section 5.1: Please remove the File extension, mac file type code, 
and OID registration. That is unless you intend to register a file 
format at the same time. However in that case you need to reference a 
file format definition and an analyzes if registration under the same 
name is appropriate or not must be performed.

16. Section 5.1: A question mark on the text for author/change 
controller depending on the registration space.

17. Section 5.2: Offer/Answer usage. Sorry, you will not get away this 
easy. You have one "negotiable" parameter in the media type, that is 
BSID. To my understanding the only thing that one needs to consider is 
that the receiving part declares the highest version it supports 
receiving. A sender can then send any version equal or lower than the 
declared. For sendonly, that indicates the version intended to be sent. 
For multicast a receiver may indicate a lower BSID than the offer, but 
not a higher.

My suggestion for specification text is:

"Certain considerations are need when performing offer/answer exchanges 
[RFC 3264]. The "rate" is a symmetric parameter and the answer MUST use 
the same value or remove the payload type.

In unicast usage the "BSID" parameter is declared by each entity. For 
recvonly or sendrecv the parameter indicates the highest version number 
that this receivers support, a media sender MAY use any version equal or 
lower than the indicated. For sendonly streams it indicates the version 
intended to use. If the answerer indicates a lower version, the sending 
entity SHALL send with a version equal or lower to the indicated in the 
answer or renegotiate the session. For multicast the answerer the 
version number MUST either be kept or reduced to the highest version 
supported by the answerer.

The "Channels" parameter is declarative and indicates, for recvonly or 
sendrecv the desired channel configuration to receive, for sendonly the 
intended channel configuration to transmit. All receivers are capable of 
receiving any number of channels and the parameter exchange can only 
help optimize the transmission to the number of channels usable by the 
receiver. "ptime" and "maxptime" are negotiated as defined for "ptime" 
in RFC 3264 [RFC 3264]."

18. Section 6. Please remove or correct the sentence
"Such an encryption scheme is harmless
to the encoded audio data presuming the data is decrypted before being
sent to the decoder."

The reason is that the sentence is strangely defined. Also in my opinion 
it is quite unnecessary. Unless people do not understand that encryption 
needs to happen sometime after AC-3 encoding and decryption somewhere 
before decoding. Combined with the fact that all the security mechanism 
works on RTP packets, or even lower layers, such as IPSec.

19. Section 6. Is there any any manipulation an attacker could perform 
to a audio frame that would result in that the codec crashes or uses 
significant more processing or memory to handle that frame. Examples of 
these would be to manipulate BSI, or dynamic range control. If the 
answer is yes, then this needs to be pointed out. This due to the Denial 
of Service threat that then exist.

20. Section 6. Integrity and source authentication isn't discussed. I 
would suggest that the security consideration section would read:

"The payload format described in this document is subject to the 
security considerations defined in RTP [4] and any applicable RTP 
profile (e.g. [RFC3551]). To protect the users privacy and any 
copyrighted material, confidentiality protection would have to be 
applied. To also protect against modification by intermediate entities 
and ensure the authenticness of the audio stream, integrity protection 
and authentication would be required. Confidentiality, integrity 
protection and authentication have
to be solved by a mechanism external to this payload format, e.g. SRTP [9]."

If you have any potential DoS risks these would be spelled out in a 
second paragraph and indicate that use of integrity protection and 
authentication would be required to protect one from this threat. Also 
other mitigations can be described.

21. I am missing a congestion control section. This should indicate what 
operations I can perform to reduce the bit-rate and/or packet rate of 
the transmission.

22. Section 8 and 9: Move RFC 2736 to the informative part. Add RFC 3551 
as an informative reference. Add RFC 3264 as an normative reference.
Add RFC 2327 as a normative reference (in regards to comment 12). Add 
RFC 3267 as a normative reference (in regards to comment 13).

23. Check the boiler plate if that requires any update with the release 
of RFC 3978 and 3979.



Cheers

Magnus Westerlund

Multimedia Technologies, Ericsson Research EAB/TVA/A
----------------------------------------------------------------------
Ericsson AB                | Phone [phone]
Torshamsgatan 23           | Fax   [phone]
S-164 80 Stockholm, Sweden | mailto: [e-mail]

_______________________________________________
Audio/Video Transport Working Group
[e-mail]
https://www1.ietf.org/mailman/listinfo/avt