On Fri, 2013-09-13 at 20:21 -0300, Jo?o Paulo Rechi Vita wrote: > On Mon, Aug 19, 2013 at 6:05 AM, Tanu Kaskinen > <tanu.kaskinen at linux.intel.com> wrote: > > On Tue, 2013-08-13 at 01:54 -0300, jprvita at gmail.com wrote: > >> From: Jo?o Paulo Rechi Vita <jprvita at openbossa.org> > >> > >> --- > >> src/modules/bluetooth/module-bluez5-device.c | 81 ++++++++++++++++++++++++++++ > >> 1 file changed, 81 insertions(+) > > > > Apparently you did no changes, despite my feedback: > > http://lists.freedesktop.org/archives/pulseaudio-discuss/2013-July/018049.html > > > > I don't have a deep knowledge of how the sink latency should be > calculated and reported. My understanding of this is that > FIXED_LATENCY_PLAYBACK_A2DP is synthesizing the over-the-air A2DP > latency (which can be calculated with the current specification of > AVDTP), and then we set a fixed latency for the sink in > a2dp_set_bitpool adding up this FIXED_LATENCY_PLAYBACK_A2DP and the > time the whole data buffer takes. If my assumptions are right, then I > think we should keep using thread_info.fixed_latency in > PA_SINK_MESSAGE_GET_LATENCY. What are your thoughts about it? The latency behind the the socket interface fluctuates as the headset consumes data and as we write more data to the socket. There is some lower and upper bound for the latency. The upper bound is reached when all buffers are full. We have no idea about what the lower bound is, but that's not so important. We don't have an interface for querying how much there is room in the buffer behind the socket, so we use the system clock to estimate the fill level. If we write a block exactly at the moment when there's room for one block, according to the system clock, then that is the moment when all buffers should be full. This is the moment when the real latency should equal the fixed_latency field of the sink. So, fixed_latency represents the maximum latency. Whatever we return in PA_SINK_MESSAGE_GET_LATENCY, it shouldn't be larger than fixed_latency. What should we return then? We should return fixed_latency - pa_bytes_to_usec(u->write_block_size) + wi - ri which equals FIXED_LATENCY_PLAYBACK_A2DP + wi - ri I hope I was able to convince you that it's wrong to use fixed_latency + wi - ri as the reported latency like your patch does. I didn't really explain why I think "FIXED_LATENCY_PLAYBACK_A2DP + wi - ri" would be the correct replacement. If you want me to explain that, I can try, but it's not easy to do (otherwise I would have explained it already in this mail). -- Tanu
