Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

obs-ffmpeg: Add a stats dock for SRT with graphs #9027

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

pkviet
Copy link
Member

@pkviet pkviet commented Jun 5, 2023

Description

This adds:

  • a proc handler for srt stats.
  • an srt stats dock, including a live updated graph of the most important statistics.
    This depends on a companion obs-deps PR adding qtcharts to qt deps : deps.qt: Add qtcharts obs-deps#192 .
    The current PR will stay in draft state until the obs-deps PR is merged (if it is).

obs64_2023-06-05_21-05-12

Motivation and Context

I had many exchanges with broadcast engineers during the 2023 SRT Plugfest.
One thing missing from obs srt output are stats to monitor the network condition.
In particular, given the peculiarities of SRT (which relies on udp rather than tcp like rtmp), it is important to
be able to monitor:

  • resent packets,
  • dropped packets,
  • RTT,
  • latency.

I received also the suggestion to use live updated graph which allows to follow the
trends over time and are more informative than instantaneous stats.

The charts code could be easily ported to the current stats dock.
@derrod suggested that a chart of dropped frames could be useful for rtmp too.

How Has This Been Tested?

Tested on windows against YouTube SRT ingests and a variety of other SRT capable servers.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@pkviet pkviet marked this pull request as draft June 5, 2023 19:15
@pkviet pkviet force-pushed the srt-statsdock branch 2 times, most recently from 2f3fc5f to 1235cf7 Compare June 5, 2023 19:21
@tytan652 tytan652 added New Feature New feature or plugin UI/UX Anything to do with changes or additions to UI/UX elements. labels Jun 5, 2023
@Warchamp7
Copy link
Member

Is this something that should live in OBS at all? Can this information not be tracked/monitored with external applications or utilities?

@pkviet
Copy link
Member Author

pkviet commented Jun 5, 2023

Is this something that should live in OBS at all? Can this information not be tracked/monitored with external applications or utilities?

  1. I wouldn't formulate the question like that. There are a lot of features of obs that are optional but probe very convenient.
    Should it be included in obs ? Phrased like that, no.
    Is it convenient to SRT users ? Definitely.
    SRT users will live better with it than without. This improves their network monitoring.
  2. As to the second question, for instance YouTube doesn't provide any of these stats. Your mileage may vary depending on the ingest service.
    These stats are provided by libsrt on the sender side. Libsrt doesn't send stats from the receiver.
    Even if the receiver has a dashboard or tool giving useful stats, it's still interesting to cross-check the info with that on the sender side.

@gxalpha
Copy link
Member

gxalpha commented Jun 6, 2023

How difficult do you think it would be to provide API's for plugins to implement such windows? Looking at the code it seems that’s basically what it does already, albeit internally via proc handlers.
(And also, having the main obs-ffmpeg plugin link against Qt is going to be very controversial, I’d argue that if this should become part of OBS itself it should be a UI feature instead, at which point you’d need the same API's a plugin would use).

@pkviet
Copy link
Member Author

pkviet commented Jun 6, 2023

How difficult do you think it would be to provide API's for plugins to implement such windows? Looking at the code it seems that’s basically what it does already, albeit internally via proc handlers. (And also, having the main obs-ffmpeg plugin link against Qt is going to be very controversial, I’d argue that if this should become part of OBS itself it should be a UI feature instead, at which point you’d need the same API's a plugin would use).

it's probably not too difficult to make an API for that.
Is it worth it though since I only foresee such a usage for the stats dock ? But I lack imagination tbh.

I could certainly split the code into its own plugin indeed; i hadn't thought about it.
I did consider splitting srt / rist / mpegts in its own plugin since obs-ffmpeg has turned into a behemoth, including audio and video encoders, media source, recording output, ffmpeg output ...

@derrod
Copy link
Member

derrod commented Jun 6, 2023

I wonder if it would be reasonable to have some sort of generic metrics API in libobs that plugins could report various stats to, e.g. things like drops/RTT, RAM usage, frame rate (for async sources). Then the UI could register a callback or poll to receive metrics updates and can be then present such data in arbitrary forms, such as a graph.

@RytoEX
Copy link
Member

RytoEX commented Jun 6, 2023

The questions from @Warchamp7, @gxalpha, and @derrod are exactly in line with concerns I had previously raised.

Personally, I am not fond of the idea of adding more Qt components for the core distribution. I do see how this could help with debugging SRT streams, but I do not presently see the benefits for the larger subsets of our users. I think that it would be better to have the stream session data/metrics able to be extracted via appropriate APIs for any given stream session and read/parsed/interpreted/graphed/charted by external tools rather than us implementing charts in the OBS interface itself.

@derrod
Copy link
Member

derrod commented Jun 7, 2023

I do not presently see the benefits for the larger subsets of our users.

For SRT maybe not, but for general performance? Absolutely.

I think that it would be better to have the stream session data/metrics able to be extracted via appropriate APIs for any given stream session and read/parsed/interpreted/graphed/charted by external tools rather than us implementing charts in the OBS interface itself.

I'm definitely in favour of adding Qt Charts based stats displays to the stats dock, as it's currently not really possible to determine OBS performance/network stability over time. Relegating that to third-party tools would be pretty silly.

@@ -474,7 +478,6 @@ void obs_module_unload(void)
#if ENABLE_FFMPEG_LOGGING
obs_ffmpeg_unload_logging();
#endif

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an unnecessary and unrelated modification.

@mufunyo
Copy link

mufunyo commented Jun 11, 2023

It seems a bit silly to abbreviate "RTT", when "Retransmitted (pkts/sec)" is more characters than "Round Trip Time" would be.

@pkviet pkviet force-pushed the srt-statsdock branch 3 times, most recently from 934c46a to 2053a0a Compare June 30, 2023 19:52
@basisbit
Copy link

It seems this PR is not progressing because of the added qt component. Is there any chance to get this PR through without the chart? I am referring to this part from @pkviet 's screenshot:

image

This already would be a huge improvement, and imho this is very much needed improvement to resolve the "but stats show no dropped frames and no encoding lag" when using SRT in OBS when in fact there is for example 30% packet loss and tons of retransmits. Currently the streamer doesn't see that they chose a way too high bitrate for the used internet connection, except when watching the stream and seeing random fragments instead of the intended video stream.

@pkviet pkviet force-pushed the srt-statsdock branch 3 times, most recently from 8812556 to f2e83b5 Compare December 24, 2023 21:09
@pkviet pkviet added the Seeking Testers Build artifacts on CI label Dec 24, 2023
@pkviet pkviet force-pushed the srt-statsdock branch 3 times, most recently from 75dd7ad to aec0be3 Compare December 24, 2023 21:54
This splits the ffmpeg mpegts muxer output from obs-ffmpeg project.
There's some common code between obs-ffmpeg-output.c &
obs-ffmpeg-mpegts.c.
But most of it had already been factored out to allow for easier
maintainance and readability.
The goal is to allow simpler maintainance of the mpegts output.

Signed-off-by: pkv <pkv@obsproject.com>
This adds a proc handler for statistics for SRT protocol.

Signed-off-by: pkv <pkv@obsproject.com>
This copies QtCharts.dll as well as QtOpenGL.dll & QtOpenGLWidgets.dll
on which it depends.

Signed-off-by: pkv <pkv@obsproject.com>
This adds a new SRT Stats dock with several important statistics for the
SRT protocol (RTT, retransmitted packets, dropped packets) in a chart
which is updated live.

Signed-off-by: pkv <pkv@obsproject.com>
This is to ensure test builds are working.
This commit should be removed on merging of the PR.

Signed-off-by: pkv <pkv@obsproject.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Feature New feature or plugin Seeking Testers Build artifacts on CI UI/UX Anything to do with changes or additions to UI/UX elements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants