search

pion / srtp

A Go implementation of SRTP
MIT License
110 stars 46 forks source link

`EncryptRTP` API change #9

Open kixelated opened 5 years ago

kixelated commented 5 years ago

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 commented 5 years ago

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

sirzooro commented 4 months ago

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.