On 26.04.2018 17:38, Georg Chini wrote:
On 26.04.2018 15:13, Tanu Kaskinen wrote:
On Thu, 2018-03-29 at 15:33 +0200, Georg Chini wrote:
The rewrite of the thread function does not change functionality much,
most of it is only cleanup, minor bug fixing and documentation work.
This patch also changes the send buffer size for a2dp sink to avoid
lags
after temporary connection drops, following the proof-of-concept patch
posted by Dmitry Kalyanov.
Bug-Link: https://bugs.freedesktop.org/show_bug.cgi?id=58746
---
Changes in v2:
- fix issues pointed out by Tanu
- set writable to false for HSP only if a block really needs to be
written
src/modules/bluetooth/module-bluez5-device.c | 289
++++++++++++++++++---------
1 file changed, 191 insertions(+), 98 deletions(-)
+ blocks_to_write -= result;
+ if (blocks_to_write > 0)
+ writable = false;
This "blocks_to_write > 0" check is new. In the previous version
writable was set to false unconditionally, and that's how I believe it
should be done. I guess this is an optimization: we won't write
anything before we get POLLIN, so to you it seemed unnecessary to wake
up on POLLOUT. Getting the POLLOUT notification is indeed unnecessary
in itself, but we still need to set POLLOUT when polling, because
otherwise when we wake up due to POLLIN, pollfd->revents will not have
POLLOUT set even if the socket is writable. As a result we won't write
when we're supposed to.
Or actually... if we wake up on POLLIN, and POLLOUT isn't in revents
even if the socket is writable, we'll set POLLOUT on the subsequent
poll, and that will return immediately, so there's not much delay with
the write. So maybe the "blocks_to_write > 0" is fine after all. Some
kind of a comment would probably be good in any case. For example:
"writable controls whether we set POLLOUT when polling - we set it to
false to enable POLLOUT. If there are more blocks to write, we want to
be woken up immediately when the socket becomes writable. If there
aren't currently any more blocks to write, then we'll have to wait
until we've received more data, so in that case we only want to set
POLLIN. Note that when we are woken up the next time, POLLOUT won't be
set in revents even if the socket has meanwhile become writable, which
may seem bad, but in that case we'll set POLLOUT in the subsequent
poll, and the poll will return immediately, so our writes won't be
delayed."
Yes it is an optimization and it works fine and significantly reduces
CPU load on slow systems (which was the reason to implement it).
I will add your comment to clarify.
Thinking again, you will end up in the situation you describe anyway:
If writable is set to false and POLLOUT is set, the rtpoll run will
immediately return. Then writable will be true, but since we have no
block to write, it will not be set to false. So again you disable POLLOUT,
just one iteration later (unless POLLIN was also set).
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
