[xiph-rtp] Updated Vorbis and new Theora I-D's

[Phil Kerr]

Hi Tor,

Tor-Einar Jarnbjo wrote:

>Tirsdag, 01 februar 2005, skrev Phil Kerr <phil at plus24.com>:
>
>  
>
>>If everything is ok I'll submit them to the IETF either at the end of 
>>this week or early next week.
>>    
>>
>
>As usual, I have a few comments :)
>  
>

Super!

>* I already pointed out, that it is not defined if the Vorbis packets 
>(both audio and header) should include the initial packet type byte.
>The data is already contained in the VDT field in the payload header,
>so although this is part of the actual packet data, it must not 
>necessarily be repeated.
>  
>
Do you mean that it isn't clear that each Vorbis payload needs a VDT 
octet?  There is only one VDT per packet, but after working on the 
Theora I-D (which uses an 8 bit field) it became clear that it is 
possible to have a VDT for each payload packet.  What do other think, 
stick with only allowing one payload type per packet or allow mixed?  
There will be complications for fragmented packets though of the mixed 
route is taken.

>I also suggest that the VDT description in section 2.2 is changed to:
>
>      0 = Vorbis Audio payload
>      1 = Vorbis Setup payload
>      2 = Vorbis Codebook payload
>      3 = Vorbis Metadata payload
>  
>
This is a good change.

>* Section 4, paragraph 2: The meta data header is not needed to decode 
>the Vorbis stream.
>  
>

Slaps forehead!  This should have been fixed in the last version, done now.

>* Section 4.1.2: The codebook length field is in the text described 
>as 16 bits, but in figure 10, it is 32 bits.
>  
>
Fixed, should be 16 bits.

>* Section 4.1.2.1: I see no reason why a library "SHOULD" use the 
>included C code, as this would impose a recommendation to use a specific 
>programming language. I suggest to replace the second paragraph with:
>
>
>The following C code function may be used by implementations. If 
>not, the code responsible for generating the CRC32 value MUST be 
>functionally identical.
>  
>
Good change.

>It may also be relevant, that the datatype sizes are implementation 
>dependent and that the quoted C function may produce wrong results 
>on systems where ints are not 32 bits or chars not 8 bit.
>  
>
Good point.  The biggest issue is 32 bit ints as I don't think there are 
many platforms which use a different size for unsigned chars (might be 
worth checking though).

>* Section 4.1.3: It should be specified if the comment length fields 
>refer to number of bytes or number of characters.
>
>* Section 5.1: It should be specified which URL schemes MUST be supported 
>by any client. HTTP should be sufficient.
>  
>
Yes.  I think HTTP should be enough, I'll add the reference.

>* Section 3.2 and 6: In 3.2 it's stated that "feedback reports on 
>lost and dropped packets MUST be sent back via RTCP". In section 
>6, this requirement is reduced to that "Vorbis clients SHOULD send 
>regular receiver reports detailing congestion." At least in a multicast 
>scenario, it's mostly pointless for the client to report packet loss 
>to the server, as there is no way to fix the situation.
>  
>
I know the AVT are keen on having this kind of info feeded back for 
monitoring.  Perhaps the first MUST can be reduced to a SHOULD?

Cheers

-P

>Tor
>
>
>
>
>
>  
>