search

fzwoch / obs-gstreamer

GStreamer OBS Studio plugin
GNU General Public License v2.0
349 stars 34 forks source link

Add latency and NTP options #66

Closed normen closed 1 year ago

normen commented 3 years ago

This PR adds latency and NTP server options for the plugin, allowing to sync network inputs with a fixed latency.

fzwoch commented 3 years ago

Thanks for the contribution. Sounds like a neat addition. I'll take a closer look when time allows, hopefully not too far in the future.

fzwoch commented 2 years ago

Really have to take look at that again. Does it change the default behavior in any way? Also, can you copy the .clang-format file from OBS and run clang-format -i *.c? No need to maintain the .clang-format file here, but I like to run it once in a while for new code so that the code format is kind of coherent.

normen commented 2 years ago

Is this an unstable API? It seems to be missing from the docs and not explicitly exported in their code.

I don't think its "unstable", its used in their own Decklink plugin as well as the externaly NDI plugin.

I am still working on the growing delay issue though. I already thought I fixed it completely but on my RTSP cameras it still grows (I let the plugin report latency changes in the pipieline, its not the pipeline). The good news is that with all these settings and when using the gst-plugin-ndi to stream NDI cameras to OBS it seems to be completely fixed.

I have a lengthy post to the issue tracker "in queue" which I composed when I thought these settings fixed it completely as this issue seems to pop up across many OBS plugins in one form or the other.

normen commented 2 years ago

Really have to take look at that again. Does it change the default behavior in any way? Also, can you copy the .clang-format file from OBS and run clang-format -i *.c? No need to maintain the .clang-format file here, but I like to run it once in a while for new code so that the code format is kind of coherent.

It doesn't change the default behavior but I had to change the buffer options caption to still make sense in combination with the "drop" option.

fzwoch commented 2 years ago

There usually is no easy way for clock sync between two different clocks. I know that in the past I had some code that compared network timestamps to current host clock. Over a time and filtering out jitter one could detect a clock drift between them and act accordingly. But I don't think I have it anymore and I'm too laze to tackle the issue again.

Also feel free to mark this PR as draft so you can un-draft it when you feel happy with it.

normen commented 2 years ago

There usually is no easy way for clock sync between two different clocks. I know that in the past I had some code that compared network timestamps to current host clock. Over a time and filtering out jitter one could detect a clock drift between them and act accordingly. But I don't think I have it anymore and I'm too laze to tackle the issue again.

Also feel free to mark this PR as draft so you can un-draft it when you feel happy with it.

Nah, syncing the cameras with NTP works great, I am also doing it in a standalone gstreamer app with the same settings. The issue is a "ghost latency" which seems to only occur in OBS, but not only with this plugin. It even happens when the appsink isn't synced, in that case the cams should simply deliver with network latency which they don't after a while.

Read: I still would like to have all these options in obs-gstreamer either way 😅

TychoWerner commented 1 year ago

It this someone @normen you are able to complete? I would love this functionallity

normen commented 1 year ago

IMO it is/was complete - by now theres new conflicts to resolve due to other changes though

TychoWerner commented 1 year ago

IMO it is/was complete - by now theres new conflicts to resolve due to other changes though

Ah that's nice to hear. Is it now for @fzwoch to merge the final steps? Wish to have SRT with SEI NTP syncing so this would be nice.

lucaspontoexe commented 1 year ago

The only breaking change I can see is in 9831483a290ef956ae88e4b0a63f9c601060ed5b, which split the start_pipe() function into pipeline_restart() and pipeline_destroy()

fzwoch commented 1 year ago

Closed via 1e0d58eba0a223aeea79b18a1a83b86231408726

I had problems applying the PR directly, so I hope I did it right.

I think I original misunderstood this:

I am still working on the growing delay issue though

as the PR not being completed yet.