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

Ensure that PulseAudio doesn't send samples upstream too quickly. #789

Merged
merged 5 commits into from
Jan 1, 2025

Conversation

tmiw
Copy link
Collaborator

@tmiw tmiw commented Dec 23, 2024

@Tyrbiter reported in #778 that he's occasionally getting "RX FIFO full" logging messages. Additional investigation indicated that this was because libpulse was queueing up huge boluses of samples in between long periods of being idle. This PR adds some additional logic to space out the incoming samples to prevent samples from being inappropriately dropped.

@tmiw tmiw mentioned this pull request Dec 23, 2024
@barjac
Copy link

barjac commented Dec 30, 2024

Using this branch has not affected the reporter issues discussed on IRC.
I see this at end of TX:
11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/TxRxThread.cpp:649: Triggering sending of EOO
11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:178: Queueing 1152 EOO samples to output FIFO
11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 992)
11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 832)
11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 672)
11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 512)
11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 352)
11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 192)
11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 32)
11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 32 EOO samples (remaining in FIFO: 0)
11:46:50 INFO /home/baz/freedv-rade/freedv-gui/src/ongui.cpp:910: All TX finished, going out of PTT

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 30, 2024

Using this branch has not affected the reporter issues discussed on IRC.

Does the audio quality sound okay, at least? Haven't heard back from @Tyrbiter yet.

I see this at end of TX: 11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/TxRxThread.cpp:649: Triggering sending of EOO 11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:178: Queueing 1152 EOO samples to output FIFO 11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 992) 11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 832) 11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 672) 11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 512) 11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 352) 11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 192) 11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 160 EOO samples (remaining in FIFO: 32) 11:46:49 INFO /home/baz/freedv-rade/freedv-gui/src/pipeline/RADETransmitStep.cpp:97: Returning 32 EOO samples (remaining in FIFO: 0) 11:46:50 INFO /home/baz/freedv-rade/freedv-gui/src/ongui.cpp:910: All TX finished, going out of PTT

Yep, normal.

@Tyrbiter
Copy link

Tyrbiter commented Dec 30, 2024

I have not found any real problems with this latest commit, the problems I saw before have not returned.

However I probably need to check some more and listen to some more transmissions.

For info here are some debug output lines from my current console window:

18:57:16 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:210: mean amplitude: 1.524844
18:57:16 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:220: Estimated BER: 0.410714
19:07:23 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:210: mean amplitude: 0.610489
19:07:23 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:220: Estimated BER: 0.464286
19:08:09 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:210: mean amplitude: 0.841547
19:08:09 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:220: Estimated BER: 0.000000
19:08:09 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:334: rxCRC: 110, calcCRC: 231, decodedStr: *.8K-KZ.
19:38:06 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:210: mean amplitude: 0.752846
19:38:06 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:220: Estimated BER: 0.589286
19:40:34 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:210: mean amplitude: 0.543537
19:40:34 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:220: Estimated BER: 0.642857

As you can see there are few people sending EOO text yet, but occasionally things get as far as an rxCRC which usually fails when compared with calcCRC

Here's another one:

17:18:42 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:210: mean amplitude: 0.240837
17:18:42 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:220: Estimated BER: 0.017857
17:18:42 INFO /home/bdm/git/freedv-gui-v2.0-ms-pulseaudio-divide-samples/src/pipeline/rade_text.c:334: rxCRC: 179, calcCRC: 207, decodedStr: 9'.'&DR

It's certainly looking hopeful though, I will report if anything obviously bad happens.

@tmiw
Copy link
Collaborator Author

tmiw commented Jan 1, 2025

BTW the previous merge shouldn't have any impact on this PR; those changes were basically Windows-only and to hopefully make things easier when it's time to merge v2.0-dev into master.

@Tyrbiter
Copy link

Tyrbiter commented Jan 1, 2025

I have seen no further issues with FIFO handling so as far as I can see this is good to merge.

Not sure if any other audio sub-systems behave like pipewire, but maybe these code changes are generic.

@tmiw
Copy link
Collaborator Author

tmiw commented Jan 1, 2025

I have seen no further issues with FIFO handling so as far as I can see this is good to merge.

Sounds good.

Not sure if any other audio sub-systems behave like pipewire, but maybe these code changes are generic.

These changes are actually pipewire-specific. I sent out Windows/macOS builds from v2.0-dev last night for testing (which use PortAudio) and hopefully no further changes are needed there.

@tmiw tmiw merged commit 2b45f3d into v2.0-dev Jan 1, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants