-
-
Notifications
You must be signed in to change notification settings - Fork 10.7k
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
Improve audio player behavior #4572
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rom1v
force-pushed
the
audio_player_atomic
branch
4 times, most recently
from
January 19, 2024 16:13
52fe7b5
to
c8f00f0
Compare
rom1v
force-pushed
the
audio_player_atomic
branch
3 times, most recently
from
January 24, 2024 15:17
0126abd
to
4dd201e
Compare
rom1v
force-pushed
the
audio_player_atomic
branch
3 times, most recently
from
February 2, 2024 13:47
edad610
to
8fe2e29
Compare
rom1v
force-pushed
the
audio_player_atomic
branch
from
February 16, 2024 10:59
0c5b7af
to
4e35761
Compare
The audio output thread only reads samples from the buffer, and most of the time, the audio receiver thread only writes samples to the buffer. In these cases, using atomics avoids lock contention. There are still corner cases where the audio receiver thread needs to "read" samples (and drop them), so lock only in these cases. PR #4572 <#4572>
Use different thresholds for enabling and disabling compensation. Concretely, enable compensation if the difference between the average and the target buffering levels exceeds 4 ms (instead of 1 ms). This avoids unnecessary compensation due to small noise in buffering level estimation. But keep a smaller threshold (1 ms) for disabling compensation, so that the buffering level is restored closer to the target value. This avoids to keep the actual level close to the compensation threshold. PR #4572 <#4572>
The buffering level does not change continuously: it increases abruptly when a packet is received, and decreases abruptly when an audio block is consumed. To estimate the buffering level, a rolling average is used. To make the buffering more stable, increase the smoothness of this rolling average. This decreases the risk of enabling audio compensation due to an estimation error. PR #4572 <#4572>
The assumption that underflow and overbuffering are caused by jitter (and that the delay between the producer and consumer will be caught up) does not always hold. For example, if the consumer does not consume at the expected rate (the SDL callback is not called often enough, which is an audio output issue), many samples will be dropped due to overbuffering, decreasing the average buffering indefinitely. Prevent the average buffering to become negative to limit the consequences of an unexpected behavior. PR #4572 <#4572>
rom1v
force-pushed
the
audio_player_atomic
branch
from
February 17, 2024 15:14
4e35761
to
a7cf4da
Compare
armm29393
added a commit
to armm29393/scrcpy-root
that referenced
this pull request
May 24, 2024
scrcpy v2.4 Changes since v2.3.1: - Add UHID keyboard and mouse support (Genymobile#4473) - Simulate tilt multitouch by pressing Shift (Genymobile#4529) - Add rotation support for non-default display (Genymobile#4698) - Improve audio player (Genymobile#4572) - Adapt to display API changes in Android 15 (Genymobile#4646, Genymobile#4656, Genymobile#4657) - Adapt audio workarounds to Android 14 (Genymobile#4492) - Fix clipboard for IQOO devices on Android 14 (Genymobile#4492, Genymobile#4589, Genymobile#4703) - Fix integer overflow for audio packet duration (Genymobile#4536) - Rework cleanup (Genymobile#4649) - Upgrade FFmpeg to 6.1.1 in Windows releases (Genymobile#4713) - Upgrade libusb to 1.0.27 in Windows releases (Genymobile#4713) - Various technical fixes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR improves internal implementation details of the audio player.
Atomics
The main change consists in removing locking contention between the audio receiver thread and the audio output thread in the "happy path". Synchronization is replaced by atomics. Locking is kept for corner cases where the writer thread needs to "read" (to consume/drop samples).
Compensation thresholds
To adjust the audio samples so that a target latency is preserved between the input and the output, compensation (think "resampling", see blogpost) is applied. The compensation is proportional to the difference between the actual buffering level and the target buffering level.
But to avoid spurious compensation (due to noise errors), it was only enabled if this difference was more than 1 ms. However, the buffering level does not change continuously: it increases abruptly when a packet is received, and decreases abruptly when an audio block is consumed, so a rolling average is used. This estimation may sometimes vary by an amount which may trigger (unwanted) compensation.
To avoid the problem, make two changes:
But keep a smaller threshold (1 ms) for disabling compensation, so that the buffering level is restored closer to the target value. This avoids to keep the actual level close to the compensation threshold.
Here is a log capture before the changes (
scrcpy -Vverbose
) (look at the actual spurious compensation values):The compensation values are expressed in samples / 4 seconds, so for example a value of 96 means 24 samples compensated per second (for 48000 input samples, there will be 48024 samples written to the buffer, which adds 500µs of compensation).
And after the changes:
Spurious compensation is still possible, but less likely (of course, expected compensation still occurs, for example on buffer underflow).