srtp icon indicating copy to clipboard operation
srtp copied to clipboard

`EncryptRTP` API change

Open kixelated opened this issue 6 years ago • 2 comments

current:

EncryptRTP(dst []byte, plaintext []byte, header *rtp.Header) ([]byte, error)

proposed:

EncryptRTP(dst []byte, packet *rtp.Packet) ([]byte, error)

pros:

  1. Simpler API when callers use *rtp.Packet.
  2. Avoids an extra Unmarshal for callers when use rtp.Packet. For example: WriteRTP.
  3. Avoids a memcpy of the payload prior to encryption.
  4. Removes the implicit assumption that cap(dst) == cap(plaintext) will avoid an allocation.

cons:

  1. Worse API when callers use byte slices. Requires Unmarshal, but net performance is still the same/better.
  2. Removes the option to fill the packet headers after sending. I can't think of a case when this would be practical.

kixelated avatar Feb 17 '19 14:02 kixelated

ditto for the WriteRTP and ReadRTP methods. Should combine Header, []byte into Packet everywhere.

kixelated avatar Mar 10 '19 04:03 kixelated

I am against this. In my case RTP/RTCP packets are generated by other parts of the system, and I use pion/srtp to encrypt generated packets. The same case is for receiving, I need to decrypt them to []byte and pass elsewhere for processing.

sirzooro avatar Feb 23 '24 17:02 sirzooro