Copy raw RTP data, customizable settings, custom PayloadMap

[vany-egorov]

1. AddRemoteNonStrict method to Session;
2. Make Session configurable with function arguments;
3. Allow copy RTP data via SetBuffer, SetInUse, SetIsFree methods;
4. Make buffer sizes customizable. Buffers are reused via sync.Pool;
5. Allow custom and multiple PayloadMap's;

[wernerd]

As for the 'AddRemoteNonStrict': this violates RFC 3550.

RTP relies on the underlying protocol(s) to provide demultiplexing of RTP data and RTCP control
streams. For UDP and similar protocols, RTP SHOULD use an even destination port number and the
corresponding RTCP stream SHOULD use the next higher (odd) destination port number.

Note the use of SHOULD here which means no derivation from this rule. Other RTP libraries depend on this.

Well, buffers bigger than the maximum size of usual hardware buffers (usually little bit over 1500 bytes) would lead to IP packet split and this is an issues with throughput etc.

The changes in WriteData(...) are not necessary. Either all remotes wrote all bytes of the whole function returns en error. This was the reason the ignore the 'written' in the first play and just return the unaltered n

Because I'm not familiar with sync.pool functions I cannot yet comment on these changes.

What's the reason to add functions to set payload maps? The Standard defines a well established way to define media formats which are not part of the well defined payload formats. Changing payload formats, the numbers, indices, etc would probably not work with other RTP implementations.

[vany-egorov]

Thanks for your review and fast reply! 😊❤

As for the 'AddRemoteNonStrict': this violates RFC 3550.
Note the use of SHOULD here which means no derivation from this rule. Other RTP libraries depend on this.

Maybe you are right. But I didn't make any changes to AddRemote method. AddRemote acts the same way.

Well, buffers bigger than the maximum size of usual hardware buffers (usually little bit over 1500 bytes) would lead to IP packet split and this is an issues with throughput etc.
Because I'm not familiar with sync.pool functions I cannot yet comment on these changes.

I agree. Using sync.Pool and heap allocations here is overkill.
I think better solution is to allocate 1500 bytes on stack, but use only 1300 or 1472 or 1488 bytes.
1300 bytes limit need to be configurable (but not more than 1500).

The changes in WriteData(...) are not necessary. Either all remotes wrote all bytes of the whole function returns en error. This was the reason the ignore the 'written' in the first play and just return the unaltered n

Ok. I will fix it. But why "n" here as return variable? Why not to just WriteData(rp *DataPacket) (err error). Or if we want to act like io.Writer we need to Write(data []byte) (int, error).

What's the reason to add functions to set payload maps? The Standard defines a well established way to define media formats which are not part of the well defined payload formats. Changing payload formats, the numbers, indices, etc would probably not work with other RTP implementations.

For example i got two video streams. One from google-chrome, one from firefox. I want to just resend then to ffmpeg via rtp for example. Chrome and firefox cound have the same dynamic payload format (i.e. 98). But for Chrome it could be VP9 and for firefox it could be OPUS.

I want to handle this streams in one process but in different RTP Sessions. But i cann't set PayloadFormatMap individually for each Session. PayloadFormatMap is global variable.

I will try to note your comments and will split this big merge request into multiple smaller.