[xiph-rtp] [vorbis-rtp] Updates

[Luca Barbato]

Ralph Giles wrote:
> 
> Here's my review. I also checked in some minor wording changes. I think
> we can still make a lot of the text more clear, but let's get the 
> details worked out first. :)
> 
> The spec doesn't give a payload type. Do we assign that, or do we need 
> the WG to do that? I note we've been using 96 in the reference 
> implementations and 98 in the SDP examples.

dynamic numbers are dynamic. The fixed payload number will be decided on 
a second time IIRC

> 
> In section 2.3, I'm confused by the paragraph on payload packing 
> referencing RFC 3551. Is this a reference to bit/byte order? When
> I first read it I thought it meant the ordering of vorbis packets
> within a packed RTP packet. If it's that it has to be a MUST, since
> there's no way to reorder the vorbis packets for correct decode.

I think that MUST is better.

> 
> We should also clarify that the vorbis packet should be zero-padded
> out to the end of the last octet, and so the length field for the next
> vorbis packet in a packed RTP payload will start on an octet boundary.
> The vorbis spec implies this, but it's good to clarify the rules here.

Integrated in the payload description.

> 
> Also in section 2.3, a reference is made to an ITU spec for channel
> mapping. Maybe we should give some examples since that spec isn't
> freely available? (and so we can make sure it matches the vorbis
> mapping!) 

What about ftp://ftp.sgi.com/sgi/aiff-c.9.26.91.ps.Z  ?
the rfc3551 reports this document and has a channel mapping description 
in itself.

 > Is there a way to distinguish dolby-style 5.1 from other six channel
 > mappings?

I didn't document myself to reply properly yet. Will be one of the items 
in my todo list tomorrow.

> 
> Section 3: I can see the value of defining the packed header formats
> here so we can refer to them from the descriptions of separate 
> out-of-band retrieval methods, but I still don't see any particular
> advantage to packing them into a fragmented series of RTP packets.

If they are sent in-band they will obey the same rules for the other 
vorbis data, thus they could be fragmented (they will be for sure).

> Using the same mapping we do for data packets, just like in Ogg,
> seems simpler to me and would let us eliminate the payload type
> field.

It would allow mixing packets of different type (should be forbidden).
I we could consider the issue but if it lasted for that many revision of 
the rfc there could be a reason I don't figure now.

> 
> We should also explicitly say somewhere that the ident field in the 
> payload header maps a given data packet to a given decoder 
> configuration, and that for out-of-band retrieval methods, that
> mapping is outside the scope of the current document.

I'll add the notices tomorrow.

> 
> I also think we should mention some of the examples of oob methods we've 
> come up with in this document, e.g. URI in the SDP or equivalent 
> pointing to an HTTP or RTSP url for unicast use, another SDP for a 
> separate multicast channel containing just the headers for a multicast
> session, or even a private fixed mapping for broadcast applications. 
> Also, a hash could be included to allow caching. This helps illustrate 
> what we're thinking even if we don't give any details.

I'll try to write something tomorrow. A 24bit hash doesn't look so great 
considering that we discarded a 32bit solution because of collisions.



> 
> In section 4: please remove "This packet SHOULD NOT be sent". We have 
> people on both sides of that idea; being neutral seems the best course.

Ok, I'll leave it just optional but not discouraged.

> 
> In section 5.2: we should probably ammend that to say that the client 
> MUST discard any remaining fragments in a vorbis packet after one 
> fragment is lost. Vorbis is designed to cope with packet truncation,
> just not with complete packet loss. There will be still be signal loss
> of course, but it's better than dropping the packet entirely.

Corrected.

> 
> I also think we should tone down the RTCP feedback references in this 
> section. This is already covered by other RFCs. How about changing the 
> last sentence to a SHOULD, or dropping it altogether.

I don't think is a good idea put a SHOULD, I'll reread tomorrow the 
pointed rfc to see if is ok to remove those part and just point the 
documents.

> 
> In section 6.1, but MIME subtype is still listed as "VORBIS" instead of 
> "vorbis".

Modified to be shown in the example uppercase and lowercase elsewhere 
(the note about case insensitivity is next to the example)

> 
> So far, so good,

I'm committing some changes now and the other will follow tomorrow.

lu

PS: I changed the timestamps production in the reference implementation, 
I hope I understood correctly the spec.

-- 

Luca Barbato

Gentoo/linux Developer		Gentoo/PPC Operational Leader
http://dev.gentoo.org/~lu_zero