Skip to content

libobs/media-io: Sleep to next audio time accurately#6415

Merged
jp9000 merged 2 commits intoobsproject:masterfrom
norihiro:audio-thread-os_sleepto_ns
May 15, 2022
Merged

libobs/media-io: Sleep to next audio time accurately#6415
jp9000 merged 2 commits intoobsproject:masterfrom
norihiro:audio-thread-os_sleepto_ns

Conversation

@norihiro
Copy link
Contributor

@norihiro norihiro commented May 3, 2022

Description

This PR implements a new function os_sleepto_ns_fast, which sleeps in a low precision but won't return until reaching the specific time.
This change introduces os_sleepto_ns_fast to the audio thread so that the audio packets will be periodically output.

Motivation and Context

Prior to this change, the audio thread roughly waits the time of AUDIO_OUTPUT_FRAMES, which is hard coded 1024, and consecutively processes twice or more if the latency is accumulated. This behavior caused fluctuation of timing to output audio packets and affects audio buffering in decklink output.

How Has This Been Tested?

The amount of audio buffer is monitored with a temporary commit db923d1, which is one more commit ahead of #1865 for logging purpose.
The buffer size in decklink is monitored at just before sending new audio frames. The buffer size is plotted and checked the buffer size is constant. (The buffer is gradually increasing, which is another issue to be fixed in the future.)

deckout-data-comp-os_sleepto_ns_fast-WriteAudio
Audio buffer for the decklink-output device. This figure is taken with Intensity Pro 4K and Fedora 34. The buffer size is monitored at every 6.4 seconds.

Types of changes

  • Tweak (non-breaking change to improve existing 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.

@WizardCM WizardCM added the Enhancement Improvement to existing functionality label May 3, 2022
@norihiro
Copy link
Contributor Author

norihiro commented May 3, 2022

If the CPU usage is a concern, how about an idea to add a new function os_sleepto_ns_fast, which sleeps in ms accuracy in Windows or us accuracy in Unix and return after reaching the specified time.

@notr1ch
Copy link
Member

notr1ch commented May 3, 2022

CPU usage is indeed a concern, os_sleepto_ns runs a spin loop so it will definitely impact CPU use. If millisecond precision is enough then I think we should go with that.

norihiro added 2 commits May 4, 2022 01:08
The function `os_sleepto_ns` has a spin loop so it will consume CPU
resource. The new function has same interface but consumes less CPU
resource with a drawback of less precision.
Prior to this change, the audio thread roughly waits the time of
`AUDIO_OUTPUT_FRAMES`, and consecutively processes twice or more if the
latency is accumulated. This behavior causes fluctuation of timing to
output audio packets. This change introduces `os_sleepto_ns_fast` so
that the audio packets will be periodically output.
@norihiro norihiro force-pushed the audio-thread-os_sleepto_ns branch from 3d28177 to 8aeada4 Compare May 3, 2022 16:08
@norihiro
Copy link
Contributor Author

norihiro commented May 3, 2022

The new function os_sleepto_ns_fast is implemented for the audio thread.
The function is tested with this standalone program https://gist.github.com/norihiro/df8b73e692a9d96106b8e8d71bcf3e1f
With my testing environment, the accuracy is within 0.2ms on Linux and within 2ms (mostly within 1ms) on Windows. A do-while loop is implemented but it will pass only once for most cases.
Not tested on macOS.

@mufunyo
Copy link

mufunyo commented May 3, 2022

I would recommend SetWaitableTimer as an alternative to Sleep combined with a spin loop.

More info: https://groups.google.com/a/chromium.org/g/scheduler-dev/c/0GlSPYreJeY

@norihiro
Copy link
Contributor Author

norihiro commented May 4, 2022

I would recommend SetWaitableTimer as an alternative to Sleep combined with a spin loop.

Are you suggesting for os_gettime_ns or os_gettime_ns_fast?
I'm not so knowledgeable on Windows. Reading the SDK's document, the timer object requires to be created and released. Especially considering multithreading, do you have an idea how to implement it?

@mufunyo
Copy link

mufunyo commented May 4, 2022

I meant for os_sleepto_ns_fast. I defer to more senior developers on implementation details though.

@notr1ch
Copy link
Member

notr1ch commented May 4, 2022

I experimented with the waitable / high resolution timers last year, they seem very platform dependent in their accuracy (they would often overshoot the requested time) so are probably not a good idea to use in such critical code. For example, compare my numbers to the thread above:

delay is 100 us - slept for 176.7 us
delay is 100 us - slept for 381.4 us
delay is 100 us - slept for 450.1 us
delay is 10 us - slept for 460.6 us
delay is 10 us - slept for 460.6 us
delay is 10 us - slept for 465.5 us
delay is 1 us - slept for 471.9 us
delay is 1 us - slept for 10.1 us
delay is 1 us - slept for 247.2 us

@norihiro norihiro marked this pull request as draft May 7, 2022 23:17
@norihiro
Copy link
Contributor Author

norihiro commented May 7, 2022

This PR is related to #6407 and #1865. I'd like to test more to identify tthe benefit and necessity.

@norihiro
Copy link
Contributor Author

norihiro commented May 11, 2022

deckout-data
This figure shows comparison of decklink audio buffer without and with this PR, tested with 48kHz sampling frequency settings on OBS Studio.

Before this change, the buffer size starts around 25ms and gradually increases and suddenly decreases 21ms. It means that the timestamp is ignored by decklink-output and the video and audio latency could differ depending on when the decklink-output starts. So, I expect this will improve the video and audio sync in decklink-output.

After the change, the buffer size stays almost constant around 20ms. (Still one run shows high buffer size. I think this is another issue.)

@norihiro norihiro marked this pull request as ready for review May 11, 2022 14:54
@jp9000 jp9000 merged commit d044231 into obsproject:master May 15, 2022
@norihiro norihiro deleted the audio-thread-os_sleepto_ns branch May 15, 2022 10:27
@RytoEX RytoEX added this to the OBS Studio 28.0 milestone May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Improvement to existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants