-
Notifications
You must be signed in to change notification settings - Fork 469
[Alsa Host] Set appropriate start threshold depending on the direction #433
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
[Alsa Host] Set appropriate start threshold depending on the direction #433
Conversation
1824c9c to
656b568
Compare
This is caused by the way poll_descriptors_and_prepare_buffer defers the read and write and essentially defers the read indefinitely and that causes the start to be deferred indefinitely. So the pcm will be started explicitly for capture. Buffer underruns aren't a problem for capture streams anyway
|
Thanks for diving into this! And thanks for taking a close look into the ALSA backend in general - I've already learned a few new things reading through your recent comments/PRs. Perhaps we should land #401 first and then base this work on top of that? |
|
I'm fine with that. I am still doing some research into this at the moment. It appears that on my machine the fact that |
That hypothesis can be scrapped. My understanding of the semantics involved was correct. I can now confirm that this will definitely result in under-runs. In fact it happens immediately at the |
| // if the pcm is not started | ||
| if stream_type == alsa::Direction::Capture { | ||
| handle.start()? | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you set the start_threshold for capture streams to 1, I think you would not have to call start() here at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexmoon any chance you might be interested in getting this work over the line in a new PR? My understanding on this is a bit vague, but it sounds like this might help with some of the initial underruns that can occur when kicking off an ALSA stream?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mitchmindtree yes, I can work on that. I'd probably like to combine this change with #431 since they're related and wait until #506 is merged so we don't have too many outstanding PRs touching the same files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexmoon You may be right. I vaguely recall trying 1 and it not working. I then hypothesized that it was due to readi not getting called at all if the pcm is not running and the start_threshold for recording relying on that to switch the pcm to running. But that memory is shaky at best and I am not an expert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that seems to be correct. In my other implementation I was using readi() and a start_threshold of 1 worked correctly.
|
Superseded by #520 |
I have taken the alsa pcm's approach of a period of the buffer size for playback. Hopefully that fills up the buffer since the next period is coming shortly and can probably be written well within the time it takes to play the first period in the buffer. I don't know if this assumption holds in extremely low latency situations, but if it doesn't I suspect underruns would start happening anyway.
Fixes #432