On 25.03.2018 09:44, Georg Chini wrote:
On 25.03.2018 08:34, Tanu Kaskinen wrote:
On Sat, 2018-03-24 at 19:27 +0100, Georg Chini wrote:
On 24.03.2018 18:13, Tanu Kaskinen wrote:
On Mon, 2018-03-05 at 08:49 +0100, 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
---
src/modules/bluetooth/module-bluez5-device.c | 275
++++++++++++++++++---------
1 file changed, 182 insertions(+), 93 deletions(-)
Thanks! Reading the new code caused less trouble than I recall the
bluetooth thread_func() previously having caused, so the changes are
good.
There are some issues pointed out below.
+ }
u->rtpoll_item = pa_rtpoll_item_new(u->rtpoll,
PA_RTPOLL_NEVER, 1);
pollfd = pa_rtpoll_item_get_pollfd(u->rtpoll_item, NULL);
@@ -861,7 +903,7 @@ static void setup_stream(struct userdata *u) {
pollfd->events = pollfd->revents = 0;
u->read_index = u->write_index = 0;
- u->started_at = 0;
+ u->started_at = pa_rtclock_now();
This seems unnecessary. write_block() resets started_at anyway when
the
first write is done.
Hmm... Now I noticed that u->started_at is used before the first
write.
But it seems wrong to set u->started_at twice using (slightly)
different values. u->started_at is used in this code before the first
write:
time_passed = pa_rtclock_now() - u->started_at;
I think before the first write time_passed should be set to exactly 0.
We have to discard audio that accumulated during the startup
time of the device to make sure audio is in sync. There will always
be some bytes dropped at the start. So it is correct to set the
initial time twice.
Dropped? I don't see how that happens. In the first iteration
time_passed is some small value, likely less than one block. audio_sent
is zero. Skipping is done only if (time_passed - audio_sent) is more
than two blocks.
It definitely is more than two blocks.
If the delay is large for some reason, then skipping might happen, but
that's not good. A long startup delay shouldn't cause audio to be
dropped.
Why shouldn't it? If audio accumulated, it needs to be dropped
to achieve initial sync.
AV sync only requires accurate latency reporting, and the GET_LATENCY
handler uses u->started_at in its calculations. The delay between
setup_stream() where started_at is first set and the first
write_block() call doesn't affect the latency at all (and if it did,
GET_LATENCY would not take that latency into account). Consider this
thought experiment: if the startup delay is 10 seconds for some reason,
surely the end latency is still much less 10 seconds (even without
dropping any audio).
Don't understand. If the startup delay is 10 seconds and audio is
accumulating
during that time, the delay will always be 10 seconds. How should it
ever change?
So without dropping (and without resetting start time) A/V Sync does
not work.
Maybe some additional explanation helps:
Consider you are watching a video and switch from your internal sound card
to your BT headset. At some point, the thread function will be set up
and audio
will start to accumulate. On the first iteration of the thread function
nothing
happens (and nothing is dropped), it will simply fall through and then
wait for
POLLOUT. This can take some time and the video is continuing. Now, when
POLLOUT is set, the audio that was sent before needs to be dropped,
otherwise
A/V sync will be lost.
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
