[Bug 733265] srtpenc: remove get-rollover-counter signal and add stats property

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=733265

Olivier Crête <olivier.crete at ocrete.ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #289703|none                        |needs-work
             status|                            |

--- Comment #5 from Olivier Crête <olivier.crete at ocrete.ca> ---
Review of attachment 289703:
 --> (https://bugzilla.gnome.org/review?bug=733265&attachment=289703)

::: ext/srtp/gstsrtpenc.c
@@ +611,3 @@
+
+  if (filter->session) {
+    stream = srtp_get_stream (filter->session, htonl (filter->current_ssrc));

Why only the current SSRC? Why not keep an array of seen SSRCs since the key
was last changed and just return the ROC for all of them?

@@ +639,3 @@
+
+  s = gst_structure_new ("application/x-multiplexed",
+      "streams", G_TYPE_ARRAY, streams, NULL);

a GstStructure containing a GArray of GstStructures?? Why such a monstruosity?
Also, you can'T use a GArray in element properties as they're sadly not
introspectable. You may want to us a GValueArray of GStructures instead.

@@ +807,2 @@
   if (HAS_CRYPTO (filter))
+    gst_structure_set (ps, "roc", G_TYPE_UINT, 0, NULL);

Why put roc=0 in the caps? What's the point?

@@ +1023,3 @@
   if (filter->first_session) {
+    GstCaps *caps = gst_pad_get_current_caps (pad);
+    err_status_t status = gst_srtp_enc_create_session (filter, caps, is_rtcp);

You should take the SSRC from inside the packets, not from the caps. The SSRC
in the caps is kind of meaningless because of RTX, FEC, etc. There could be
more than one SSRC in the stream.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.