Skip to content

srt: Fixes and improvements to stats handling

Mart Raudsepp requested to merge leio/gst-plugins-bad:srt-stats3 into master

This MR takes !1664 and moves over to an action signal approach, as follows:

  • The "srt: Pass SRT's statistics on verbatim" commit was edited to not reset the stats.
  • The "srt: Report callers' remote address in the stats" commit is dropped, as the same thing in a slightly different way has since gone in via !1772 (merged)
  • The "srt: Replace G_TYPE_VALUE_ARRAY with GST_TYPE_ARRAY" commit has been made to cleanly apply on top of master after the !1772 (merged) changes
  • Separate commits added to add a get-stats element action and drop the "stats" property as redundant. I kept them separate commits for now, in case we don't want to drop the property, despite this being in gst-plugins-bad for a new stable release cycle. If we do drop it, the two last commits could also be squashed together, if desired.

Open questions for me:

  • Should we drop the "stats" property? g_signal_emit_by_name (element, "get-stats", FALSE, FALSE, &stats) is now equal to previous g_object_get (element, "get-stats", &stats, NULL);
    • It was actually shadowing application/x-gst-base-sink-stats from GstBaseSink before, so dropping it makes those appear instead from under the stats property.
  • Should we mirror srt_bistats exactly like this, or clean it up a bit?
    • SRT upstream has some plans on changing the stats API in some ABI breaks in 1.4 or later, and it might become hard to keep ABI compat on our side afterwards, depending on which way they go.
    • One alternative idea is to drop the instantaneous boolean argument and instead always make a srt_bistats (srtsock, &istats, 0, 1); call before a srt_bistats (srtsock, &stats, clear, 0); call and then expose the few stats that change their meaning based on instantaneous value under different keys in the returned GstStructure. This feels like a bit more future-proof, at least as long as we assume that consumers make key presence checks in the returned structure, but slight overhead when the user only needs the instantaneous or moving average stats, not both, or no interest in stats that depend on this at all — currently only pktSndBuf, msSndBuf, pktRcvBuf, msRcvBuf have a "instantaneous" value, which is going to be different depending on this boolean. It would then mean some extra keys that don't have a matching SRT stats name, which goes contrary with the "pass stats on verbatim" goal that I strongly agree with otherwise.
Edited by Mart Raudsepp

Merge request reports