After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 776901 - srtp: Port to libsrtp2
srtp: Port to libsrtp2
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-01-05 14:03 UTC by Sebastian Dröge (slomo)
Modified: 2018-01-29 07:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
srtp: Support libsrtp2 (24.38 KB, patch)
2018-01-12 09:27 UTC, Jan Alexander Steffens (heftig)
none Details | Review
srtp: Support libsrtp2 (24.45 KB, patch)
2018-01-26 15:28 UTC, Jan Alexander Steffens (heftig)
none Details | Review
srtp: Support libsrtp2 (28.72 KB, patch)
2018-01-28 17:01 UTC, Jan Alexander Steffens (heftig)
committed Details | Review

Description Sebastian Dröge (slomo) 2017-01-05 14:03:56 UTC
Lot's of API we use seems to be gone, need to find alternatives or get them to expose more API again.
Comment 1 Sebastian Dröge (slomo) 2017-05-15 07:29:35 UTC
This is supposed to be fixed now, but someone will have to try: https://github.com/cisco/libsrtp/issues/270#issuecomment-300069623
Comment 2 Jan Alexander Steffens (heftig) 2018-01-12 09:27:22 UTC
Created attachment 366705 [details] [review]
srtp: Support libsrtp2

For libsrtp 1, add defines that translate the new namespaced identifiers
to the old unnamespaced ones. Also move the code for setting and getting
a stream's ROC into two compat functions that match libsrtp2's API.

It seems that libsrtp2 properly supports changing the ROC without having
to touch the sequence numbers afterwards, given that srtp_set_stream_roc
sets a pending_roc field, so the entire roc_changed dance should not be
needed anymore. The compat functions for libsrtp 1 just contain our
preexisting hacks, however, so it's still needed there.

libsrtp2 has no means of discovering the streams in the session, so to
create the stats structure we need to iterate over our own set of SSRCs.
For this we also need to re-add the previously removed ssrcs_set to the
encoder.
Comment 3 Jan Alexander Steffens (heftig) 2018-01-12 09:28:15 UTC
Test suite still passes, but I could not test further.
Comment 4 Sebastian Dröge (slomo) 2018-01-12 09:37:39 UTC
Another useful test would be gst-rtsp-server's "test-video" example (#define WITH_AUTH and WITH_TLS at the top) and testing that against rtspsrc.
Comment 5 Jan Alexander Steffens (heftig) 2018-01-26 12:57:27 UTC
That still works.

examples/test-video &

gst-launch-1.0 rtspsrc location=rtsps://user:password@127.0.0.1:8554/test tls-validation-flags=0 ! decodebin ! autovideosink
Comment 6 Sebastian Dröge (slomo) 2018-01-26 14:11:10 UTC
Sounds good to me then!
Comment 7 Sebastian Dröge (slomo) 2018-01-26 15:06:37 UTC
Review of attachment 366705 [details] [review]:

Looks good except for the version check :)

::: configure.ac
@@ +2472,3 @@
 AG_GST_CHECK_FEATURE(SRTP, [srtp library], srtp, [
+  HAVE_SRTP="no"
+  AG_GST_PKG_CHECK_MODULES(SRTP, libsrtp2)

I believe we need at least version 2.1, or not? See https://github.com/cisco/libsrtp/issues/270#issuecomment-287720060
Comment 8 Jan Alexander Steffens (heftig) 2018-01-26 15:28:40 UTC
Created attachment 367482 [details] [review]
srtp: Support libsrtp2

Require libsrtp2 >= 2.1.0.
Comment 9 Sebastian Dröge (slomo) 2018-01-26 16:08:51 UTC
Comment on attachment 367482 [details] [review]
srtp: Support libsrtp2

  CC       libgstsrtp_la-gstsrtp-enumtypes.lo
In file included from gstsrtp.h:49:0,
                 from gstsrtp-enumtypes.c:6:
../../config.h:63:0: error: "GST_LEVEL_DEFAULT" redefined [-Werror]
 #define GST_LEVEL_DEFAULT GST_LEVEL_ERROR
 
In file included from /home/slomo/Projects/gstreamer/head/gstreamer/gst/gst.h:55:0,
                 from gstsrtp-enumtypes.h:7,
                 from gstsrtp-enumtypes.c:4:
/home/slomo/Projects/gstreamer/head/gstreamer/gst/gstinfo.h:102:0: note: this is the location of the previous definition
 #define GST_LEVEL_DEFAULT GST_LEVEL_NONE
 

This is with libsrtp2
Comment 10 Jan Alexander Steffens (heftig) 2018-01-28 17:01:39 UTC
Created attachment 367550 [details] [review]
srtp: Support libsrtp2

Split enums off into gstsrtpenums.h so that gstsrtp.h can include config.h without messing up the
compilation of gstsrtp-enumtypes.c, which included gstsrtp.h after <gst/gst.h>, causing a
redefinition of GST_LEVEL_DEFAULT. With the enums in gstsrtpenums.h, gstsrtp-enumtypes.c no longer
includes gstsrtp.h.
Comment 11 Jan Alexander Steffens (heftig) 2018-01-28 17:04:37 UTC
By the way, the meson srtp_mkenum.py only includes <glib-object.h> in the generated header, so that problem was exclusive to the autotools build.
Comment 12 Sebastian Dröge (slomo) 2018-01-29 07:59:07 UTC
Attachment 367550 [details] pushed as e9aa117 - srtp: Support libsrtp2