GNOME Bugzilla – Bug 752774
srtpenc: remove unnecessary rtp/rtcp checks to improve performance
Last modified: 2015-08-16 13:37:43 UTC
Created attachment 307984 [details] [review] srtpenc: add "check-input-buffers" prop Now, srtpenc is checking if every input buffer is RTP or RTCP before encryption. This adds an overhead that can be avoided if the source element ensures that each buffer is RTP or RTCP.
How much of a difference does it make? rtpsession in rtpbin also does the same thing.
These checks seem weird to me, can't we just assume trusted/already-verified RTP/RTCP input here? And if it's not trusted/checked, then we probably shouldn't error out but just drop the buffer?
In my benchmark case, it implies the 7% of the srtpenc processing for doing something unnecessary. About rtpsession I do not see where this kind of check is done.
Those checks seem to be legacy, they date from the time where we parsed the ssrc out of the packets. We should probably just remove them entirely.
OK, I will update the patch to remove these checks instead of adding a property. Moreover, there an overhead allocating and moving memory before/after the encryption. I have seen this into gst_srtp_enc_process_buffer: 7.5% - gst_buffer_new_allocate 5.9% - gst_buffer_extract 2.0% - gst_buffer_set_size 2.0% - gst_buffer_copy_into 2.2% - gst_buffer_map Could be a way of reduce this? Can we avoid some of this operations? Having a buffer pool or something like that? (for gst_buffer_new_allocate) Do you think if this function can be improved?
To remove that copy, we'd need a way to request padding from the element that allocated the buffer, but we currently don't have that implemented anywhere. We could possibly do that with the allocation params in the reply to the allocation query. That said, I don't think this is implemented by any of the payloaders. They could in turn request a prefix from the encoder, so that the whole thing can be done in place.. But none of this is trivial, and it may not be worth it. Another option would be to make the srtp library understand scattered memory and just append another buffer for the suffix.
Created attachment 308057 [details] [review] srtpenc: do not check input buffers
@olivier, thanks for these ideas, I will think about them ;). For the moment I have uploaded a patch that remove input buffer checks.
commit 7db723831ddcc07f506e64e027b504ac16a067f7 Author: Miguel París Díaz <mparisdiaz@gmail.com> Date: Fri Jul 24 09:42:53 2015 +0200 srtpenc: do not check input buffers With this we avoid an unnecessary and considerable overhead. https://bugzilla.gnome.org/show_bug.cgi?id=752774