Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On Wed, 2015-02-25 at 18:48 +0100, Georg Chini wrote: > On 25.02.2015 17:51, Tanu Kaskinen wrote: > > If there was the kind of function that you propose, would it return > > false in this scenario? If so, that would just make module-alsa-card.c > > kill the sink input, because it's not going to cancel the profile change > > just because some sink input refuses to be moved. > > > No, it would not return false in this situation, but I when you read > the description of the may_move_to() function it says: > > 1) this function is called before the sink input is moved away The documentation says "moved to a sink", not "moved away from a sink". So technically it's correct, but a note should indeed be added that the sink input may already have been detached from the old sink. > 2) if it returns false the move will not be allowed > > Both is wrong. During profile switching the sink is already invalid > when the function is called and the profile will be switched anyway, > regardless what the function returns. If the function returns false, moving won't be done, so I don't think the documentation has issues here. In case of a profile switch, the sink input will be killed. I wouldn't call that moving. > That's why I thought maybe there should be a function for which both > is true. But never mind - it was just a thought and I have worked around > the issue meanwhile. But maybe the description of the may_move_to() > function should be modified. Good that a solution was found already! -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On 25.02.2015 17:51, Tanu Kaskinen wrote: On Mon, 2015-02-23 at 18:56 +0100, Georg Chini wrote: On 23.02.2015 08:02, Georg Chini wrote: On 22.02.2015 23:25, Alexander E. Patrakov wrote: Anyway, the original submission (i.e. the patch that I am replying to) has a bug: it crashes PulseAudio in the Bluetooth A2DP -> HDA test if one changes the sink profile from Stereo Output to 2.1 Surround Output while the loopback is active. Before the patch, there was no such crash. I do not think that is a bug in the module. It looks like the sink has already changed when pa_sink_input_may_move_to_cb() is called and that should definitely not be the case. The original version does not show that problem because it never resets the rate when source or sink changes. Not too sure, but maybe this code in card_set_profile in module_alsa_card.c, line 223 already frees the sink before it should? sink_inputs = pa_sink_move_all_start(am->sink, sink_inputs); pa_alsa_sink_free(am->sink); am->sink = NULL; I have looked into this somewhat more, and it seems like the may_move_to() function of the sink input cannot hold what it promises. In sink-input.h the description is: /* If non-NULL this function is called before this sink input is * move to a sink and if it returns false the move will not * be allowed */ bool (*may_move_to) (pa_sink_input *i, pa_sink *s); /* may be NULL */ When the profile changes, the old sink does no longer exist when may_move_to() is called, because it must be destroyed to create a new sink with the different profile. So it is obviously too late to cancel the move here (and module-loopback cannot reset the rate). Unfortunately in this situation there is also no way to call may_move_to() before the old sink is destroyed because the new sink is not known before it is created. So my question: Should there be a function may_move() that is called before anything is done? /* If non-NULL this function is called before this sink input is * moved anywhere and if it returns false the move will not * be allowed */ bool (*may_move) (pa_sink_input *i); /* may be NULL */ The call to this function could be added to pa_sink_input_may_move(). The same probably applies for the source output. Why is sink_input_may_move_to_cb() in module-loopback.c calling pa_sink_input_set_rate()? That doesn't sound like a proper place to do such a thing. When module-loopback is adjusting, it changes the rate of the sink input. So if you switch source or sink it may be possible that the rate of he sink input is far away from the expected rate. This should be corrected before the sink or source is moved, otherwise you will risk underruns. If there was the kind of function that you propose, would it return false in this scenario? If so, that would just make module-alsa-card.c kill the sink input, because it's not going to cancel the profile change just because some sink input refuses to be moved. No, it would not return false in this situation, but I when you read the description of the may_move_to() function it says: 1) this function is called before the sink input is moved away 2) if it returns false the move will not be allowed Both is wrong. During profile switching the sink is already invalid when the function is called and the profile will be switched anyway, regardless what the function returns. That's why I thought maybe there should be a function for which both is true. But never mind - it was just a thought and I have worked around the issue meanwhile. But maybe the description of the may_move_to() function should be modified. Regards Georg ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On Mon, 2015-02-23 at 18:56 +0100, Georg Chini wrote: > On 23.02.2015 08:02, Georg Chini wrote: > > On 22.02.2015 23:25, Alexander E. Patrakov wrote: > > > >> Anyway, the original submission (i.e. the patch that I am replying > >> to) has a bug: it crashes PulseAudio in the Bluetooth A2DP -> HDA > >> test if one changes the sink profile from Stereo Output to 2.1 > >> Surround Output while the loopback is active. Before the patch, there > >> was no such crash. > >> > > > > I do not think that is a bug in the module. It looks like the sink has > > already changed when > > pa_sink_input_may_move_to_cb() is called and that should definitely > > not be the case. > > The original version does not show that problem because it never > > resets the rate when source > > or sink changes. > > > > Not too sure, but maybe this code in card_set_profile in > > module_alsa_card.c, line 223 already frees > > the sink before it should? > > > > sink_inputs = pa_sink_move_all_start(am->sink, sink_inputs); > > pa_alsa_sink_free(am->sink); > > am->sink = NULL; > > > > > I have looked into this somewhat more, and it seems like the > may_move_to() function of the > sink input cannot hold what it promises. In sink-input.h the description is: > > /* If non-NULL this function is called before this sink input is > * move to a sink and if it returns false the move will not > * be allowed */ > bool (*may_move_to) (pa_sink_input *i, pa_sink *s); /* may be NULL */ > > When the profile changes, the old sink does no longer exist when > may_move_to() is called, > because it must be destroyed to create a new sink with the different > profile. So it is obviously > too late to cancel the move here (and module-loopback cannot reset the > rate). > Unfortunately in this situation there is also no way to call > may_move_to() before the old sink is > destroyed because the new sink is not known before it is created. > > So my question: Should there be a function may_move() that is called > before anything is done? > > /* If non-NULL this function is called before this sink input is > * moved anywhere and if it returns false the move will not > * be allowed */ > bool (*may_move) (pa_sink_input *i); /* may be NULL */ > > The call to this function could be added to pa_sink_input_may_move(). > > The same probably applies for the source output. Why is sink_input_may_move_to_cb() in module-loopback.c calling pa_sink_input_set_rate()? That doesn't sound like a proper place to do such a thing. If there was the kind of function that you propose, would it return false in this scenario? If so, that would just make module-alsa-card.c kill the sink input, because it's not going to cancel the profile change just because some sink input refuses to be moved. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On 22.02.2015 23:25, Alexander E. Patrakov wrote: This is mostly for Tanu's patch-status page. I have split the patch into 11 components and sent the result to Georg privately. I have not done a careful self-review of the resulting components, and can't say that I 100% agree with the result. But there are definitely some things worth cherry-picking. I expect one or two more rounds of exchanging patches via private email. Thanks again for your work. Anyway, the original submission (i.e. the patch that I am replying to) has a bug: it crashes PulseAudio in the Bluetooth A2DP -> HDA test if one changes the sink profile from Stereo Output to 2.1 Surround Output while the loopback is active. Before the patch, there was no such crash. I do not think that is a bug in the module. It looks like the sink has already changed when pa_sink_input_may_move_to_cb() is called and that should definitely not be the case. The original version does not show that problem because it never resets the rate when source or sink changes. Not too sure, but maybe this code in card_set_profile in module_alsa_card.c, line 223 already frees the sink before it should? sink_inputs = pa_sink_move_all_start(am->sink, sink_inputs); pa_alsa_sink_free(am->sink); am->sink = NULL; Valgrind says: ==3812== Invalid read of size 8 ==3812==at 0x4E7588C: pa_sink_input_set_rate (sink-input.c:1463) ==3812==by 0x22A6F515: sink_input_may_move_to_cb (module-loopback.c:899) ==3812==by 0x4E791B7: pa_sink_input_finish_move (sink-input.c:1756) ==3812==by 0x4E7BB55: pa_sink_move_all_finish (sink.c:916) ==3812==by 0x19890F63: card_set_profile (module-alsa-card.c:259) ==3812==by 0x4E5BA64: pa_card_set_profile (card.c:279) ==3812==by 0x18C55246: command_set_card_profile (protocol-native.c:4782) ==3812==by 0x5C3BF0C: pa_pdispatch_run (pdispatch.c:341) ==3812==by 0x18C5DC7F: pstream_packet_callback (protocol-native.c:4896) ==3812==by 0x5C3E7FA: do_read (pstream.c:880) ==3812==by 0x5C40DDC: do_pstream_read_write (pstream.c:193) ==3812==by 0x50EF1B3: dispatch_pollfds (mainloop.c:655) ==3812==by 0x50EF1B3: pa_mainloop_dispatch (mainloop.c:898) ==3812== Address 0x348 is not stack'd, malloc'd or (recently) free'd ==3812== ==3812== ==3812== Process terminating with default action of signal 11 (SIGSEGV) ==3812== Access not within mapped region at address 0x348 ==3812==at 0x4E7588C: pa_sink_input_set_rate (sink-input.c:1463) ==3812==by 0x22A6F515: sink_input_may_move_to_cb (module-loopback.c:899) ==3812==by 0x4E791B7: pa_sink_input_finish_move (sink-input.c:1756) ==3812==by 0x4E7BB55: pa_sink_move_all_finish (sink.c:916) ==3812==by 0x19890F63: card_set_profile (module-alsa-card.c:259) ==3812==by 0x4E5BA64: pa_card_set_profile (card.c:279) ==3812==by 0x18C55246: command_set_card_profile (protocol-native.c:4782) ==3812==by 0x5C3BF0C: pa_pdispatch_run (pdispatch.c:341) ==3812==by 0x18C5DC7F: pstream_packet_callback (protocol-native.c:4896) ==3812==by 0x5C3E7FA: do_read (pstream.c:880) ==3812==by 0x5C40DDC: do_pstream_read_write (pstream.c:193) ==3812==by 0x50EF1B3: dispatch_pollfds (mainloop.c:655) ==3812==by 0x50EF1B3: pa_mainloop_dispatch (mainloop.c:898) ==3812== If you believe this happened as a result of a stack ==3812== overflow in your program's main thread (unlikely but ==3812== possible), you can try to increase the size of the ==3812== main thread stack using the --main-stacksize= flag. ==3812== The main thread stack size used in this run was 8388608. 01.02.2015 03:43, Georg Chini wrote: This is the final version of my patch for module-loopback. It is on top of the patch I sent about an hour ago and contains a lot more changes than the previous versions: - Honor specified latency if possible, if not adjust to the lowest possible value - Smooth switching from fixed latency to dynamic latency source or sink and vice versa - good rate and latency stability, no rate oscillation - adjusts latency as good as your setup allows - fast regulation of latency offsets, adjusts 100 ms offset within 22 seconds (adjust time=1) to 60 seconds (adjust_time=10) - usable latency range 4 - 3 ms - Avoid rewinds and "cannot peek into queue" messages during startup and switching - works with rates between 200 and 19 Hz - maximum latency offset after source/sink switch or at startup around is 200 ms I also introduced a new parameter, buffer_latency_msec which can be used together with latency_msec. If buffer_latency_msec is specified, the resulting latency will be latency_msec + buffer_latency_msec. Latency_msec then refers only to the source/sink latency while buffer_latency_msec specifies the buffer part. This can be used to save a lot of CPU at low latencies, running 10 ms latency with latency_msec=6 buffer_latency_msec=4 gives 8% CPU on my system compared to 12% when I o
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
This is mostly for Tanu's patch-status page. I have split the patch into 11 components and sent the result to Georg privately. I have not done a careful self-review of the resulting components, and can't say that I 100% agree with the result. But there are definitely some things worth cherry-picking. I expect one or two more rounds of exchanging patches via private email. Anyway, the original submission (i.e. the patch that I am replying to) has a bug: it crashes PulseAudio in the Bluetooth A2DP -> HDA test if one changes the sink profile from Stereo Output to 2.1 Surround Output while the loopback is active. Before the patch, there was no such crash. Valgrind says: ==3812== Invalid read of size 8 ==3812==at 0x4E7588C: pa_sink_input_set_rate (sink-input.c:1463) ==3812==by 0x22A6F515: sink_input_may_move_to_cb (module-loopback.c:899) ==3812==by 0x4E791B7: pa_sink_input_finish_move (sink-input.c:1756) ==3812==by 0x4E7BB55: pa_sink_move_all_finish (sink.c:916) ==3812==by 0x19890F63: card_set_profile (module-alsa-card.c:259) ==3812==by 0x4E5BA64: pa_card_set_profile (card.c:279) ==3812==by 0x18C55246: command_set_card_profile (protocol-native.c:4782) ==3812==by 0x5C3BF0C: pa_pdispatch_run (pdispatch.c:341) ==3812==by 0x18C5DC7F: pstream_packet_callback (protocol-native.c:4896) ==3812==by 0x5C3E7FA: do_read (pstream.c:880) ==3812==by 0x5C40DDC: do_pstream_read_write (pstream.c:193) ==3812==by 0x50EF1B3: dispatch_pollfds (mainloop.c:655) ==3812==by 0x50EF1B3: pa_mainloop_dispatch (mainloop.c:898) ==3812== Address 0x348 is not stack'd, malloc'd or (recently) free'd ==3812== ==3812== ==3812== Process terminating with default action of signal 11 (SIGSEGV) ==3812== Access not within mapped region at address 0x348 ==3812==at 0x4E7588C: pa_sink_input_set_rate (sink-input.c:1463) ==3812==by 0x22A6F515: sink_input_may_move_to_cb (module-loopback.c:899) ==3812==by 0x4E791B7: pa_sink_input_finish_move (sink-input.c:1756) ==3812==by 0x4E7BB55: pa_sink_move_all_finish (sink.c:916) ==3812==by 0x19890F63: card_set_profile (module-alsa-card.c:259) ==3812==by 0x4E5BA64: pa_card_set_profile (card.c:279) ==3812==by 0x18C55246: command_set_card_profile (protocol-native.c:4782) ==3812==by 0x5C3BF0C: pa_pdispatch_run (pdispatch.c:341) ==3812==by 0x18C5DC7F: pstream_packet_callback (protocol-native.c:4896) ==3812==by 0x5C3E7FA: do_read (pstream.c:880) ==3812==by 0x5C40DDC: do_pstream_read_write (pstream.c:193) ==3812==by 0x50EF1B3: dispatch_pollfds (mainloop.c:655) ==3812==by 0x50EF1B3: pa_mainloop_dispatch (mainloop.c:898) ==3812== If you believe this happened as a result of a stack ==3812== overflow in your program's main thread (unlikely but ==3812== possible), you can try to increase the size of the ==3812== main thread stack using the --main-stacksize= flag. ==3812== The main thread stack size used in this run was 8388608. 01.02.2015 03:43, Georg Chini wrote: This is the final version of my patch for module-loopback. It is on top of the patch I sent about an hour ago and contains a lot more changes than the previous versions: - Honor specified latency if possible, if not adjust to the lowest possible value - Smooth switching from fixed latency to dynamic latency source or sink and vice versa - good rate and latency stability, no rate oscillation - adjusts latency as good as your setup allows - fast regulation of latency offsets, adjusts 100 ms offset within 22 seconds (adjust time=1) to 60 seconds (adjust_time=10) - usable latency range 4 - 3 ms - Avoid rewinds and "cannot peek into queue" messages during startup and switching - works with rates between 200 and 19 Hz - maximum latency offset after source/sink switch or at startup around is 200 ms I also introduced a new parameter, buffer_latency_msec which can be used together with latency_msec. If buffer_latency_msec is specified, the resulting latency will be latency_msec + buffer_latency_msec. Latency_msec then refers only to the source/sink latency while buffer_latency_msec specifies the buffer part. This can be used to save a lot of CPU at low latencies, running 10 ms latency with latency_msec=6 buffer_latency_msec=4 gives 8% CPU on my system compared to 12% when I only specify latency_msec=10. Additionally you can go beyond the safe-guard limits that are built in, you can access the range 1 - 3 ms or lower the buffer latency for fixed latency devices. Some of my USB devices run fine at a buffer latency of fragment size + 4 ms instead of the dfault fragment size + 20 ms. I tested it all with Intel HDA, USB and bluetooth sound devices. I would like to see some test results from other people. -- Alexander E. Patrakov ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On 08.02.2015 19:34, Alexander E. Patrakov wrote: 01.02.2015 03:43, Georg Chini wrote: This is the final version of my patch for module-loopback. It is on top of the patch I sent about an hour ago and contains a lot more changes than the previous versions: - Honor specified latency if possible, if not adjust to the lowest possible value - Smooth switching from fixed latency to dynamic latency source or sink and vice versa - good rate and latency stability, no rate oscillation - adjusts latency as good as your setup allows - fast regulation of latency offsets, adjusts 100 ms offset within 22 seconds (adjust time=1) to 60 seconds (adjust_time=10) - usable latency range 4 - 3 ms - Avoid rewinds and "cannot peek into queue" messages during startup and switching - works with rates between 200 and 19 Hz - maximum latency offset after source/sink switch or at startup around is 200 ms I also introduced a new parameter, buffer_latency_msec which can be used together with latency_msec. If buffer_latency_msec is specified, the resulting latency will be latency_msec + buffer_latency_msec. Latency_msec then refers only to the source/sink latency while buffer_latency_msec specifies the buffer part. This can be used to save a lot of CPU at low latencies, running 10 ms latency with latency_msec=6 buffer_latency_msec=4 gives 8% CPU on my system compared to 12% when I only specify latency_msec=10. Additionally you can go beyond the safe-guard limits that are built in, you can access the range 1 - 3 ms or lower the buffer latency for fixed latency devices. Some of my USB devices run fine at a buffer latency of fragment size + 4 ms instead of the dfault fragment size + 20 ms. I tested it all with Intel HDA, USB and bluetooth sound devices. I would like to see some test results from other people. After attempting to split up this patch and to add comments, I got some remarks and questions. +pa_log_debug("Loopback overall latency is %0.2f ms + %0.2f ms + %0.2f ms = %0.2f ms, latency difference: %0.2f ms, rate difference: %i Hz", (double) u->latency_snapshot.sink_latency / PA_USEC_PER_MSEC, -(double) buffer_latency / PA_USEC_PER_MSEC, +(double) current_buffer_latency / PA_USEC_PER_MSEC, (double) u->latency_snapshot.source_latency / PA_USEC_PER_MSEC, -((double) u->latency_snapshot.sink_latency + buffer_latency + u->latency_snapshot.source_latency) / PA_USEC_PER_MSEC); I am not sure whether this split of latency accounting makes sense anymore, because it is not possible to attribute these latencies to any particular point in time. Especially current_buffer_latency, which (for me) is just a meaningless-by-itself intermediate quantity. I don't care, I left it in because it was already there. If you would like to delete it, no problem. But maybe it makes sense to print out the corrected latency at this point. Also, here is my line of thought (an alternative derivation of current_buffer_latency, which does not, however, yield exactly the same), in some pseudocode. At the moment source_timestamp, the source had already given us receive_counter bytes of data, and had source_output_buffer bytes of data buffered at the source output level and source_latency microseconds of data still sitting in the soundcard buffer. So, at that moment, we have been recording for this amount of time, according to the source clock: recording_duration_at_source_timestamp = source_latency + bytes_to_usec(receive_counter + source_output_buffer, base_rate) If we knew that base_rate is accurate (i.e. that the source clock and wall clock are exactly the same), we could add the timestamp difference to see for how long we have been recording at sink_timestamp: recording_duration_at_sink_timestamp = recording_duration_at_source_timestamp + sink_timestamp - source_timestamp We don't know that, because base_rate is in fact not accurate according to the wall clock. But we don't have an estimate of the actual source sample rate (according to the wall clock), and thus cannot translate the timestamp difference from the wallclock domain to the source clock domain any better. So we have to live with the above formula, and accept that it gives us the absolute error of this order: recording_duration_error = (sink_timestamp - source_timestamp) * abs(1 - real_base_rate / base_rate) i.e. less than 0.75% of error if we accept that the real sample rate never deviates from the nominal one by more than 0.75%. Using the similar arguments, let's calculate how long the sink input has been playing at sink_timestamp. The sink input, according to the source clock, has received send_counter bytes of data, but has sink_input_buffer bytes buffered in the sink input, and sink_latency microseconds of data (according to the sink clock) buffered in the sink. So: playback_duration = bytes_to_usec(send_
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
09.02.2015 00:35, Georg Chini пишет: On 08.02.2015 20:30, Georg Chini wrote: On 08.02.2015 19:54, Alexander E. Patrakov wrote: 01.02.2015 03:43, Georg Chini wrote: +/* Minimum number of adjust times + 1 needed to adjust at 0.75% deviation from base rate */ +min_cycles = (double)abs(latency_difference) / u->adjust_time / 0.0075 + 1; + +/* Rate calculation, maximum deviation from base rate will be less than 0.75% due to min_cycles */ +new_rate = base_rate * (1.0 + latency_difference / min_cycles / u->adjust_time) + 0.5; What's the aim here with min_cycles? Why not just clamp new_rate post-factum to 0.75% vicinity of base_rate, as this is done in the 2‰ case? Without min_cycles you will far more often hit the 2 ‰ limit and when you are approaching the base_rate. This seriously disturbs the regulation. The goal was to get out to 0.75% as quick as possible while approaching the base rate cautiously (with a weak regulator when latency is far off). Also without min_cycles you see the rates hopping up and down (due to the 2‰ limitation), you do not see a (more or less) continuous rate function. Doing what you suggest would give you 0.75% as long as the latency is more than one cycle off - but from 0.75% rate deviation you need at least 4 steps to go back to the base_rate with the 2 ‰ restriction, so you would seriously over-regulate. Thanks for your answers. Now I consider the adjust_rate() function fully reviewed. So, now (or, rather, tomorrow) I have to properly split this patch, taking your explanations into account. -- Alexander E. Patrakov ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On 08.02.2015 20:30, Georg Chini wrote: On 08.02.2015 19:54, Alexander E. Patrakov wrote: 01.02.2015 03:43, Georg Chini wrote: +/* Minimum number of adjust times + 1 needed to adjust at 0.75% deviation from base rate */ +min_cycles = (double)abs(latency_difference) / u->adjust_time / 0.0075 + 1; + +/* Rate calculation, maximum deviation from base rate will be less than 0.75% due to min_cycles */ +new_rate = base_rate * (1.0 + latency_difference / min_cycles / u->adjust_time) + 0.5; What's the aim here with min_cycles? Why not just clamp new_rate post-factum to 0.75% vicinity of base_rate, as this is done in the 2‰ case? Without min_cycles you will far more often hit the 2 ‰ limit and when you are approaching the base_rate. This seriously disturbs the regulation. The goal was to get out to 0.75% as quick as possible while approaching the base rate cautiously (with a weak regulator when latency is far off). Also without min_cycles you see the rates hopping up and down (due to the 2‰ limitation), you do not see a (more or less) continuous rate function. Doing what you suggest would give you 0.75% as long as the latency is more than one cycle off - but from 0.75% rate deviation you need at least 4 steps to go back to the base_rate with the 2 ‰ restriction, so you would seriously over-regulate. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On 08.02.2015 19:54, Alexander E. Patrakov wrote: 01.02.2015 03:43, Georg Chini wrote: +/* Minimum number of adjust times + 1 needed to adjust at 0.75% deviation from base rate */ +min_cycles = (double)abs(latency_difference) / u->adjust_time / 0.0075 + 1; + +/* Rate calculation, maximum deviation from base rate will be less than 0.75% due to min_cycles */ +new_rate = base_rate * (1.0 + latency_difference / min_cycles / u->adjust_time) + 0.5; What's the aim here with min_cycles? Why not just clamp new_rate post-factum to 0.75% vicinity of base_rate, as this is done in the 2‰ case? Without min_cycles you will far more often hit the 2 ‰ limit and when you are approaching the base_rate. This seriously disturbs the regulation. The goal was to get out to 0.75% as quick as possible while approaching the base rate cautiously (with a weak regulator when latency is far off). Also without min_cycles you see the rates hopping up and down (due to the 2‰ limitation), you do not see a (more or less) continuous rate function. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
01.02.2015 03:43, Georg Chini wrote: +/* Minimum number of adjust times + 1 needed to adjust at 0.75% deviation from base rate */ +min_cycles = (double)abs(latency_difference) / u->adjust_time / 0.0075 + 1; + +/* Rate calculation, maximum deviation from base rate will be less than 0.75% due to min_cycles */ +new_rate = base_rate * (1.0 + latency_difference / min_cycles / u->adjust_time) + 0.5; What's the aim here with min_cycles? Why not just clamp new_rate post-factum to 0.75% vicinity of base_rate, as this is done in the 2‰ case? -- Alexander E. Patrakov ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On 08.02.2015 19:33, Alexander E. Patrakov wrote: 08.02.2015 22:43, Georg Chini wrote: On 08.02.2015 16:52, Alexander E. Patrakov wrote: OK, then I think there was some misunderstanding on my side. Could you please post some log lines with two USB devices to completely clear this up? I want logs without the stop criterion (which is properly called a "deadband"), and with both 0.75% and the 2‰ restraints commented out, and adjust_time of 2 seconds. Just for debugging my assumptions :) Here is the log from my webcam to my USB sound card, command line was: pacmd load-module module-loopback rate=44100 latency_msec=150 adjust_time=2 pure p-regulator without restrictions and new_rate = base_rate * (1 + latency_difference / u->adjust_time) Feb 8 18:34:21 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 106,47 ms + 76,51 ms + 1,84 ms = 184,61 ms, latency difference: 31,31 ms, rate difference: 0 Hz Feb 8 18:34:21 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44790 Hz. Feb 8 18:34:23 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 106,44 ms + 47,33 ms + 0,21 ms = 153,67 ms, latency difference: 1,19 ms, rate difference: 690 Hz Feb 8 18:34:23 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44179 Hz. Feb 8 18:34:25 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 107,93 ms + 40,54 ms + 0,88 ms = 148,98 ms, latency difference: -3,94 ms, rate difference: 79 Hz Feb 8 18:34:25 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44019 Hz. Feb 8 18:34:27 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 104,25 ms + 47,30 ms + 0,77 ms = 152,18 ms, latency difference: -0,01 ms, rate difference: -81 Hz Feb 8 18:34:27 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44094 Hz. Feb 8 18:34:29 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 106,71 ms + 46,40 ms + 0,06 ms = 152,93 ms, latency difference: 0,81 ms, rate difference: -6 Hz Feb 8 18:34:29 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44117 Hz. Feb 8 18:34:31 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 108,62 ms + 43,59 ms + 0,38 ms = 152,27 ms, latency difference: -0,15 ms, rate difference: 17 Hz Feb 8 18:34:31 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44098 Hz. Feb 8 18:34:33 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 105,39 ms + 45,60 ms + 0,87 ms = 151,79 ms, latency difference: -0,46 ms, rate difference: -2 Hz Feb 8 18:34:33 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44090 Hz. Feb 8 18:34:35 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 107,00 ms + 45,02 ms + 0,35 ms = 152,19 ms, latency difference: -0,09 ms, rate difference: -10 Hz Feb 8 18:34:35 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44097 Hz. Feb 8 18:34:37 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 103,47 ms + 48,14 ms + 0,71 ms = 152,03 ms, latency difference: 0,11 ms, rate difference: -3 Hz Feb 8 18:34:37 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44102 Hz. Feb 8 18:34:39 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 104,88 ms + 46,98 ms + 0,00 ms = 151,48 ms, latency difference: -0,24 ms, rate difference: 2 Hz Feb 8 18:34:39 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44095 Hz. Feb 8 18:34:41 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 108,49 ms + 44,15 ms + 0,05 ms = 152,53 ms, latency difference: 0,65 ms, rate difference: -5 Hz Feb 8 18:34:41 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44114 Hz. Feb 8 18:34:43 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 105,14 ms + 46,45 ms + 0,23 ms = 151,59 ms, latency difference: -0,17 ms, rate difference: 14 Hz Fe
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
01.02.2015 03:43, Georg Chini wrote: This is the final version of my patch for module-loopback. It is on top of the patch I sent about an hour ago and contains a lot more changes than the previous versions: - Honor specified latency if possible, if not adjust to the lowest possible value - Smooth switching from fixed latency to dynamic latency source or sink and vice versa - good rate and latency stability, no rate oscillation - adjusts latency as good as your setup allows - fast regulation of latency offsets, adjusts 100 ms offset within 22 seconds (adjust time=1) to 60 seconds (adjust_time=10) - usable latency range 4 - 3 ms - Avoid rewinds and "cannot peek into queue" messages during startup and switching - works with rates between 200 and 19 Hz - maximum latency offset after source/sink switch or at startup around is 200 ms I also introduced a new parameter, buffer_latency_msec which can be used together with latency_msec. If buffer_latency_msec is specified, the resulting latency will be latency_msec + buffer_latency_msec. Latency_msec then refers only to the source/sink latency while buffer_latency_msec specifies the buffer part. This can be used to save a lot of CPU at low latencies, running 10 ms latency with latency_msec=6 buffer_latency_msec=4 gives 8% CPU on my system compared to 12% when I only specify latency_msec=10. Additionally you can go beyond the safe-guard limits that are built in, you can access the range 1 - 3 ms or lower the buffer latency for fixed latency devices. Some of my USB devices run fine at a buffer latency of fragment size + 4 ms instead of the dfault fragment size + 20 ms. I tested it all with Intel HDA, USB and bluetooth sound devices. I would like to see some test results from other people. After attempting to split up this patch and to add comments, I got some remarks and questions. +pa_log_debug("Loopback overall latency is %0.2f ms + %0.2f ms + %0.2f ms = %0.2f ms, latency difference: %0.2f ms, rate difference: %i Hz", (double) u->latency_snapshot.sink_latency / PA_USEC_PER_MSEC, -(double) buffer_latency / PA_USEC_PER_MSEC, +(double) current_buffer_latency / PA_USEC_PER_MSEC, (double) u->latency_snapshot.source_latency / PA_USEC_PER_MSEC, -((double) u->latency_snapshot.sink_latency + buffer_latency + u->latency_snapshot.source_latency) / PA_USEC_PER_MSEC); I am not sure whether this split of latency accounting makes sense anymore, because it is not possible to attribute these latencies to any particular point in time. Especially current_buffer_latency, which (for me) is just a meaningless-by-itself intermediate quantity. Also, here is my line of thought (an alternative derivation of current_buffer_latency, which does not, however, yield exactly the same), in some pseudocode. At the moment source_timestamp, the source had already given us receive_counter bytes of data, and had source_output_buffer bytes of data buffered at the source output level and source_latency microseconds of data still sitting in the soundcard buffer. So, at that moment, we have been recording for this amount of time, according to the source clock: recording_duration_at_source_timestamp = source_latency + bytes_to_usec(receive_counter + source_output_buffer, base_rate) If we knew that base_rate is accurate (i.e. that the source clock and wall clock are exactly the same), we could add the timestamp difference to see for how long we have been recording at sink_timestamp: recording_duration_at_sink_timestamp = recording_duration_at_source_timestamp + sink_timestamp - source_timestamp We don't know that, because base_rate is in fact not accurate according to the wall clock. But we don't have an estimate of the actual source sample rate (according to the wall clock), and thus cannot translate the timestamp difference from the wallclock domain to the source clock domain any better. So we have to live with the above formula, and accept that it gives us the absolute error of this order: recording_duration_error = (sink_timestamp - source_timestamp) * abs(1 - real_base_rate / base_rate) i.e. less than 0.75% of error if we accept that the real sample rate never deviates from the nominal one by more than 0.75%. Using the similar arguments, let's calculate how long the sink input has been playing at sink_timestamp. The sink input, according to the source clock, has received send_counter bytes of data, but has sink_input_buffer bytes buffered in the sink input, and sink_latency microseconds of data (according to the sink clock) buffered in the sink. So: playback_duration = bytes_to_usec(send_counter, ???) - bytes_to_usec(sink_input_buffer, !!!) - sink_latency ...with an obvious source of error that we didn't convert the sink latency to the source clock domain. But this error is of the same order as the recording duration error (because both
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
08.02.2015 22:43, Georg Chini wrote: On 08.02.2015 16:52, Alexander E. Patrakov wrote: OK, then I think there was some misunderstanding on my side. Could you please post some log lines with two USB devices to completely clear this up? I want logs without the stop criterion (which is properly called a "deadband"), and with both 0.75% and the 2‰ restraints commented out, and adjust_time of 2 seconds. Just for debugging my assumptions :) Here is the log from my webcam to my USB sound card, command line was: pacmd load-module module-loopback rate=44100 latency_msec=150 adjust_time=2 pure p-regulator without restrictions and new_rate = base_rate * (1 + latency_difference / u->adjust_time) Feb 8 18:34:21 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 106,47 ms + 76,51 ms + 1,84 ms = 184,61 ms, latency difference: 31,31 ms, rate difference: 0 Hz Feb 8 18:34:21 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44790 Hz. Feb 8 18:34:23 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 106,44 ms + 47,33 ms + 0,21 ms = 153,67 ms, latency difference: 1,19 ms, rate difference: 690 Hz Feb 8 18:34:23 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44179 Hz. Feb 8 18:34:25 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 107,93 ms + 40,54 ms + 0,88 ms = 148,98 ms, latency difference: -3,94 ms, rate difference: 79 Hz Feb 8 18:34:25 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44019 Hz. Feb 8 18:34:27 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 104,25 ms + 47,30 ms + 0,77 ms = 152,18 ms, latency difference: -0,01 ms, rate difference: -81 Hz Feb 8 18:34:27 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44094 Hz. Feb 8 18:34:29 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 106,71 ms + 46,40 ms + 0,06 ms = 152,93 ms, latency difference: 0,81 ms, rate difference: -6 Hz Feb 8 18:34:29 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44117 Hz. Feb 8 18:34:31 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 108,62 ms + 43,59 ms + 0,38 ms = 152,27 ms, latency difference: -0,15 ms, rate difference: 17 Hz Feb 8 18:34:31 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44098 Hz. Feb 8 18:34:33 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 105,39 ms + 45,60 ms + 0,87 ms = 151,79 ms, latency difference: -0,46 ms, rate difference: -2 Hz Feb 8 18:34:33 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44090 Hz. Feb 8 18:34:35 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 107,00 ms + 45,02 ms + 0,35 ms = 152,19 ms, latency difference: -0,09 ms, rate difference: -10 Hz Feb 8 18:34:35 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44097 Hz. Feb 8 18:34:37 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 103,47 ms + 48,14 ms + 0,71 ms = 152,03 ms, latency difference: 0,11 ms, rate difference: -3 Hz Feb 8 18:34:37 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44102 Hz. Feb 8 18:34:39 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 104,88 ms + 46,98 ms + 0,00 ms = 151,48 ms, latency difference: -0,24 ms, rate difference: 2 Hz Feb 8 18:34:39 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44095 Hz. Feb 8 18:34:41 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 108,49 ms + 44,15 ms + 0,05 ms = 152,53 ms, latency difference: 0,65 ms, rate difference: -5 Hz Feb 8 18:34:41 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44114 Hz. Feb 8 18:34:43 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 105,14 ms + 46,45 ms + 0,23 ms = 151,59 ms, latency difference: -0,17 ms, rate difference: 14 Hz Feb 8 18:34:43 mini pulseaudio[25589]: [pulseaudio] mo
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On 08.02.2015 16:52, Alexander E. Patrakov wrote: OK, then I think there was some misunderstanding on my side. Could you please post some log lines with two USB devices to completely clear this up? I want logs without the stop criterion (which is properly called a "deadband"), and with both 0.75% and the 2‰ restraints commented out, and adjust_time of 2 seconds. Just for debugging my assumptions :) Here is the log from my webcam to my USB sound card, command line was: pacmd load-module module-loopback rate=44100 latency_msec=150 adjust_time=2 pure p-regulator without restrictions and new_rate = base_rate * (1 + latency_difference / u->adjust_time) Feb 8 18:34:21 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 106,47 ms + 76,51 ms + 1,84 ms = 184,61 ms, latency difference: 31,31 ms, rate difference: 0 Hz Feb 8 18:34:21 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44790 Hz. Feb 8 18:34:23 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 106,44 ms + 47,33 ms + 0,21 ms = 153,67 ms, latency difference: 1,19 ms, rate difference: 690 Hz Feb 8 18:34:23 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44179 Hz. Feb 8 18:34:25 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 107,93 ms + 40,54 ms + 0,88 ms = 148,98 ms, latency difference: -3,94 ms, rate difference: 79 Hz Feb 8 18:34:25 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44019 Hz. Feb 8 18:34:27 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 104,25 ms + 47,30 ms + 0,77 ms = 152,18 ms, latency difference: -0,01 ms, rate difference: -81 Hz Feb 8 18:34:27 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44094 Hz. Feb 8 18:34:29 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 106,71 ms + 46,40 ms + 0,06 ms = 152,93 ms, latency difference: 0,81 ms, rate difference: -6 Hz Feb 8 18:34:29 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44117 Hz. Feb 8 18:34:31 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 108,62 ms + 43,59 ms + 0,38 ms = 152,27 ms, latency difference: -0,15 ms, rate difference: 17 Hz Feb 8 18:34:31 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44098 Hz. Feb 8 18:34:33 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 105,39 ms + 45,60 ms + 0,87 ms = 151,79 ms, latency difference: -0,46 ms, rate difference: -2 Hz Feb 8 18:34:33 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44090 Hz. Feb 8 18:34:35 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 107,00 ms + 45,02 ms + 0,35 ms = 152,19 ms, latency difference: -0,09 ms, rate difference: -10 Hz Feb 8 18:34:35 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44097 Hz. Feb 8 18:34:37 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 103,47 ms + 48,14 ms + 0,71 ms = 152,03 ms, latency difference: 0,11 ms, rate difference: -3 Hz Feb 8 18:34:37 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44102 Hz. Feb 8 18:34:39 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 104,88 ms + 46,98 ms + 0,00 ms = 151,48 ms, latency difference: -0,24 ms, rate difference: 2 Hz Feb 8 18:34:39 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44095 Hz. Feb 8 18:34:41 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 108,49 ms + 44,15 ms + 0,05 ms = 152,53 ms, latency difference: 0,65 ms, rate difference: -5 Hz Feb 8 18:34:41 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: [alsa_output.usb-0d8c_USB_Sound_Device-00-Device.analog-stereo] Updated sampling rate to 44114 Hz. Feb 8 18:34:43 mini pulseaudio[25589]: [pulseaudio] module-loopback.c: Loopback overall latency is 105,14 ms + 46,45 ms + 0,23 ms = 151,59 ms, latency difference: -0,17 ms, rate difference: 14 Hz Feb 8 18:34:43 mini pulseaudio[25589]: [
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
08.02.2015 18:50, Georg Chini wrote: On 08.02.2015 14:03, Alexander E. Patrakov wrote: 08.02.2015 17:35, Georg Chini wrote: I think there is some misunderstanding. Let me repeat in a different way. The smoother works perfectly (both for timer-based scheduling and for the needs of your module) on non-batch cards. But, even for batch cards, where timer-based scheduling is disabled, the smoother is active and is actually used for reporting the latency to your module. An attempt to use the smoother for timer-based scheduling on batch cards has failed. That's why I suspect that it, on batch cards, also tells lies to your module. OK, understood. I don't have anything to test it though Mh, are my USB devices batch cards? I just noticed it says "Disabling timer scheduling because BATCH flag is set" in the log and I am not sure, what a batch card is. Yes, your USB devices are batch cards. This means that they don't report their playback position accurately enough. For USB devices, the granularity of position reports is 6 ms (for large period sizes), but for others, it may be up to one period size. When I understand you right, this means that the error of the latency report will be in the range of the granularity, so latency may jump around within the bounds of this error. Then you definitely need a stop criterion and my approach is correct because it measures the error of the latency report and uses that to stop regulation when you are within those bounds. To give you an impression how it works on batch cards: With an adjust_time of 2 seconds, 44100 Hz sample rate and the default fragment size of 25 ms you usually end up with about +/-1 ms error, which is better than I would expect from your statement above. But I am also seeing some drift, that means that the regulator will do some small correction every few adjust_times. I do not know if this drift is real or caused by the smoother. I will look into it and see if I can work around the smoother. OK, then I think there was some misunderstanding on my side. Could you please post some log lines with two USB devices to completely clear this up? I want logs without the stop criterion (which is properly called a "deadband"), and with both 0.75% and the 2‰ restraints commented out, and adjust_time of 2 seconds. Just for debugging my assumptions :) -- Alexander E. Patrakov ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On 08.02.2015 14:03, Alexander E. Patrakov wrote: 08.02.2015 17:35, Georg Chini wrote: I think there is some misunderstanding. Let me repeat in a different way. The smoother works perfectly (both for timer-based scheduling and for the needs of your module) on non-batch cards. But, even for batch cards, where timer-based scheduling is disabled, the smoother is active and is actually used for reporting the latency to your module. An attempt to use the smoother for timer-based scheduling on batch cards has failed. That's why I suspect that it, on batch cards, also tells lies to your module. OK, understood. I don't have anything to test it though Mh, are my USB devices batch cards? I just noticed it says "Disabling timer scheduling because BATCH flag is set" in the log and I am not sure, what a batch card is. Yes, your USB devices are batch cards. This means that they don't report their playback position accurately enough. For USB devices, the granularity of position reports is 6 ms (for large period sizes), but for others, it may be up to one period size. When I understand you right, this means that the error of the latency report will be in the range of the granularity, so latency may jump around within the bounds of this error. Then you definitely need a stop criterion and my approach is correct because it measures the error of the latency report and uses that to stop regulation when you are within those bounds. To give you an impression how it works on batch cards: With an adjust_time of 2 seconds, 44100 Hz sample rate and the default fragment size of 25 ms you usually end up with about +/-1 ms error, which is better than I would expect from your statement above. But I am also seeing some drift, that means that the regulator will do some small correction every few adjust_times. I do not know if this drift is real or caused by the smoother. I will look into it and see if I can work around the smoother. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
08.02.2015 17:35, Georg Chini wrote: I think there is some misunderstanding. Let me repeat in a different way. The smoother works perfectly (both for timer-based scheduling and for the needs of your module) on non-batch cards. But, even for batch cards, where timer-based scheduling is disabled, the smoother is active and is actually used for reporting the latency to your module. An attempt to use the smoother for timer-based scheduling on batch cards has failed. That's why I suspect that it, on batch cards, also tells lies to your module. OK, understood. I don't have anything to test it though Mh, are my USB devices batch cards? I just noticed it says "Disabling timer scheduling because BATCH flag is set" in the log and I am not sure, what a batch card is. Yes, your USB devices are batch cards. This means that they don't report their playback position accurately enough. For USB devices, the granularity of position reports is 6 ms (for large period sizes), but for others, it may be up to one period size. -- Alexander E. Patrakov ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
I think there is some misunderstanding. Let me repeat in a different way. The smoother works perfectly (both for timer-based scheduling and for the needs of your module) on non-batch cards. But, even for batch cards, where timer-based scheduling is disabled, the smoother is active and is actually used for reporting the latency to your module. An attempt to use the smoother for timer-based scheduling on batch cards has failed. That's why I suspect that it, on batch cards, also tells lies to your module. OK, understood. I don't have anything to test it though Mh, are my USB devices batch cards? I just noticed it says "Disabling timer scheduling because BATCH flag is set" in the log and I am not sure, what a batch card is. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
in the case of timer-based scheduling (where even module-alsa-sink does not trust the result, i.e. discards it if it is greater than the non-transformed time interval). And, if I recollect correctly, there were complaints about it being fooled by batch cards, and they were cited as one of the reasons not to enable timer-based scheduling on batch cards. So - maybe, for the purposes of timer based-scheduling we should just assume the worst case, i.e. the card that is, say, 0.75% faster than nominal, and use the nominal rate together with the latest snapshot time in {source,sink}_get_latency()? Basically, the fear is that the smoother makes a greater mistake in the estimated rate than just assuming the nominal one. Maybe you can try this suggestion? For timer based scheduling the regulator works perfect, you would not even need a stop criterion, so why bother? I think there is some misunderstanding. Let me repeat in a different way. The smoother works perfectly (both for timer-based scheduling and for the needs of your module) on non-batch cards. But, even for batch cards, where timer-based scheduling is disabled, the smoother is active and is actually used for reporting the latency to your module. An attempt to use the smoother for timer-based scheduling on batch cards has failed. That's why I suspect that it, on batch cards, also tells lies to your module. OK, understood. I don't have anything to test it though. For Tanu's patch status page: please leave the status of this patch as unreviewed. The general idea of the patch does not look brilliant, but it's the best known-working idea that we currently have on the topic, and I have not reviewed all the fine details. Well from a practical point of view it does a pretty good job although the idea may not be brilliant. I'm willing to implement your better idea when you come up with it. Did you ever test it? And compare it to what the current module-loopback does? I did not test it, will do it now and add some logging in order to verify what you said above. And hopefully will try to implement an alternative latency-snapshotting implementation, just to compare. I can confirm (based on a reimplementation attempt) that the code after patching deals with the capture and playback timestamp difference 100% correctly - so it cannot be the problem. Just a minor nitpick: I moved saving of the timestamp to the message handlers. For me, this makes no difference, though. The patch (to be applied on top of yours) is attached. Could you please confirm or disprove that it makes no difference in your setup, either? I tried the same and measured the time difference between the two methods. It is around 1 - 2 us. So it does not really matter if you obtain the time stamp within the snapshot or outside. I thought it was more simple the way I implemented it, but I have no objection to your change. So, the current status of the patch, from my viewpoint, is: 1. The patch adds a perfectly correct (assuming no xruns) way to account for latency snapshots being made not simultaneously for playback and capture. I think that this is the main improvement, and it needs to be merged even if we disagree on the rest. 2. The result has an optimal coefficient that relates the observed latency difference and the resulting rate correction, assuming the currently-implemented way to snapshot the latency and assuming no interference from the smoother - which still has to be verified independently, possibly after merging. 3. The patch adds buffer_latency_msec, which seems to be an unrelated improvement, and I think it should be split out. I have no opinion on whether this change should be merged. I'm fine with separating this out. I am even fine with dropping it completely, but I thought it might be useful for those who know what they are doing and for those who have to care about used CPU cycles. 4. The patch has a criterion when to stop adjusting rates, and it is a source of disagreements. But I could not suggest anything constructive. So I think that a good approach would be to split it out and let others comment. Also, it would be a good idea to add a debugging message so that we can see when it happens. It nearly always happens when you are approaching the requested latency. The case where you hit the base rate spot on is very rare, so a message doesn't make sense for me. Maybe it would make sense to print out the latency_error somewhere because it is an indicator how good your latency can be adjusted. Without stop criterion you will see instabilities with USB devices in certain situations, so I would not merge the patch without it. Even if the regulator were completely stable in all tested situations I would still add something like it, just to make sure. The current module-loopback also contains a stop criterion which in my opinion is insufficient and should be replaced. If you want, I can do the splitting f
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
08.02.2015 13:21, Alexander E. Patrakov wrote: 08.02.2015 02:14, Georg Chini wrote: Sorry, but I do not think the smoother is the problem here. I do get quite reliable latency results. The problem is really (if there is a problem at all) the execution time of the code. These are not asynchronously called functions, they wait until they are finished. And that exactly is the problem, sometimes the queue is quite full and so it takes a lot of time until the pa_asyncmsgq_send() returns. The smoother was one of the first things I suspected to be responsible for false reports, but I could not verify it. Since I measure the time for the second call to pa_asyncmsgq_send() the numbers reported look ok. I think there really is some "latency jitter" that cannot be avoided - interrupts that cannot be handled immediately, USB bus in use when the sound card wants to deliver data and you can probably come up with a lot more situations where it is possible that data cannot be delivered on time. You'll never get perfect results when you measure something and so you cannot apply a perfect regulator even if it would be nice in theory. OK, but I cannot reproduce anything of this kind at home, so cannot really argue. Anyway, we could implement some kind of median filter in order to suppress this jitter - but this would be an incremental improvement, let's first review what we have :) Unfortunately, this is easy to recommend, but I can't really see how this can be done. The smoother is updated _after_ a successful write to the alsa device (via traditional UNIX write or via mmap), while the pop callbacks are executed just before that. So, they are called at the moment when the influence from a bad rate estimation via the smoother is the greatest. Now the suggestions. I think that, ideally, for such use cases, it is important to have timestamped latency snapshots for both sinks and sources in PulseAudio core. This would mean introducing a new message that gets the latest reliable latency snapshot (i.e., timestamp according to the wall clock, send/receive counter, input/output buffer size, and the latency itself), without any interpolation. If the sink does not implement this, just fallback (in the generic sink code) to getting the current latency. Also, because such snapshots for the sink and the source will not happen at the same moment, you have to deal with it. You can actually try and get both snapshots at the same time. I did this and was quite astonished to find that the results were less reliable this way. I could not figure out why. (You can call get_latency_in_thread() for source and sink from both snapshot functions without crashing pulse, at least when you make sure they are ONLY called from the timer function. Something else seems to call one of the snapshots for whatever reason). I have read this, but have no ideas. Anyway, latency reports obtained this way also rely on the smoother. Also, I have a very heretic thought. Namely, that the smoother in the alsa sink and source may actually be a bad idea and is better removed. I have not tested this. But it is used only in two places: for reporting latency (where it confuses your module) and for calculating the amount of time to sleep As I said, I think the latency deviations I see are real and not artifacts, so there is no confusion. in the case of timer-based scheduling (where even module-alsa-sink does not trust the result, i.e. discards it if it is greater than the non-transformed time interval). And, if I recollect correctly, there were complaints about it being fooled by batch cards, and they were cited as one of the reasons not to enable timer-based scheduling on batch cards. So - maybe, for the purposes of timer based-scheduling we should just assume the worst case, i.e. the card that is, say, 0.75% faster than nominal, and use the nominal rate together with the latest snapshot time in {source,sink}_get_latency()? Basically, the fear is that the smoother makes a greater mistake in the estimated rate than just assuming the nominal one. Maybe you can try this suggestion? For timer based scheduling the regulator works perfect, you would not even need a stop criterion, so why bother? I think there is some misunderstanding. Let me repeat in a different way. The smoother works perfectly (both for timer-based scheduling and for the needs of your module) on non-batch cards. But, even for batch cards, where timer-based scheduling is disabled, the smoother is active and is actually used for reporting the latency to your module. An attempt to use the smoother for timer-based scheduling on batch cards has failed. That's why I suspect that it, on batch cards, also tells lies to your module. For Tanu's patch status page: please leave the status of this patch as unreviewed. The general idea of the patch does not look brilliant, but it's the best known-working idea that we currently have on the topic, and I have not reviewed all th
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
08.02.2015 02:14, Georg Chini wrote: Sorry, but I do not think the smoother is the problem here. I do get quite reliable latency results. The problem is really (if there is a problem at all) the execution time of the code. These are not asynchronously called functions, they wait until they are finished. And that exactly is the problem, sometimes the queue is quite full and so it takes a lot of time until the pa_asyncmsgq_send() returns. The smoother was one of the first things I suspected to be responsible for false reports, but I could not verify it. Since I measure the time for the second call to pa_asyncmsgq_send() the numbers reported look ok. I think there really is some "latency jitter" that cannot be avoided - interrupts that cannot be handled immediately, USB bus in use when the sound card wants to deliver data and you can probably come up with a lot more situations where it is possible that data cannot be delivered on time. You'll never get perfect results when you measure something and so you cannot apply a perfect regulator even if it would be nice in theory. Unfortunately, this is easy to recommend, but I can't really see how this can be done. The smoother is updated _after_ a successful write to the alsa device (via traditional UNIX write or via mmap), while the pop callbacks are executed just before that. So, they are called at the moment when the influence from a bad rate estimation via the smoother is the greatest. Now the suggestions. I think that, ideally, for such use cases, it is important to have timestamped latency snapshots for both sinks and sources in PulseAudio core. This would mean introducing a new message that gets the latest reliable latency snapshot (i.e., timestamp according to the wall clock, send/receive counter, input/output buffer size, and the latency itself), without any interpolation. If the sink does not implement this, just fallback (in the generic sink code) to getting the current latency. Also, because such snapshots for the sink and the source will not happen at the same moment, you have to deal with it. You can actually try and get both snapshots at the same time. I did this and was quite astonished to find that the results were less reliable this way. I could not figure out why. (You can call get_latency_in_thread() for source and sink from both snapshot functions without crashing pulse, at least when you make sure they are ONLY called from the timer function. Something else seems to call one of the snapshots for whatever reason). Also, I have a very heretic thought. Namely, that the smoother in the alsa sink and source may actually be a bad idea and is better removed. I have not tested this. But it is used only in two places: for reporting latency (where it confuses your module) and for calculating the amount of time to sleep As I said, I think the latency deviations I see are real and not artifacts, so there is no confusion. in the case of timer-based scheduling (where even module-alsa-sink does not trust the result, i.e. discards it if it is greater than the non-transformed time interval). And, if I recollect correctly, there were complaints about it being fooled by batch cards, and they were cited as one of the reasons not to enable timer-based scheduling on batch cards. So - maybe, for the purposes of timer based-scheduling we should just assume the worst case, i.e. the card that is, say, 0.75% faster than nominal, and use the nominal rate together with the latest snapshot time in {source,sink}_get_latency()? Basically, the fear is that the smoother makes a greater mistake in the estimated rate than just assuming the nominal one. Maybe you can try this suggestion? For timer based scheduling the regulator works perfect, you would not even need a stop criterion, so why bother? For Tanu's patch status page: please leave the status of this patch as unreviewed. The general idea of the patch does not look brilliant, but it's the best known-working idea that we currently have on the topic, and I have not reviewed all the fine details. Well from a practical point of view it does a pretty good job although the idea may not be brilliant. I'm willing to implement your better idea when you come up with it. Did you ever test it? And compare it to what the current module-loopback does? I did not test it, will do it now and add some logging in order to verify what you said above. And hopefully will try to implement an alternative latency-snapshotting implementation, just to compare. -- Alexander E. Patrakov ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On 07.02.2015 20:50, Alexander E. Patrakov wrote: 06.02.2015 14:56, Georg Chini wrote: One more thing: There is a systematic error in the adjust_time I could not work around without introducing too much overhead. The latency snapshot varies widely in the execution time, I measured values between 50 us and more than 60 ms. So if the extreme values follow each other you will have one adjust time that is around 60 ms too long and another one which is 60 ms too short. Maybe this also contributes significantly to the (in)stability of regulation. Well, I have looked into this issue. Basically, you have a callback, time_callback, which is called every u->adjust_time microseconds, according to a timer. All it does is to send asynchronous messages to the sink input and the source output using pa_asyncmsgq_send(), and they fill in various portions of the latency snapshot by querying the relevant memblockq and the sink/source itself, as well as snapshotting the total length of data received or sent. A potential problem is that pa_source_get_latency_within_thread() and pa_sink_get_latency_within_thread() themselves, in the case of an alsa source, go through a smoother (see source_get_latency() in src/modules/alsa/alsa-source.c), which _also_ tries to do sample rate estimation for you! Try to avoid that, even though this means code duplication. Sorry, but I do not think the smoother is the problem here. I do get quite reliable latency results. The problem is really (if there is a problem at all) the execution time of the code. These are not asynchronously called functions, they wait until they are finished. And that exactly is the problem, sometimes the queue is quite full and so it takes a lot of time until the pa_asyncmsgq_send() returns. The smoother was one of the first things I suspected to be responsible for false reports, but I could not verify it. Since I measure the time for the second call to pa_asyncmsgq_send() the numbers reported look ok. I think there really is some "latency jitter" that cannot be avoided - interrupts that cannot be handled immediately, USB bus in use when the sound card wants to deliver data and you can probably come up with a lot more situations where it is possible that data cannot be delivered on time. You'll never get perfect results when you measure something and so you cannot apply a perfect regulator even if it would be nice in theory. Unfortunately, this is easy to recommend, but I can't really see how this can be done. The smoother is updated _after_ a successful write to the alsa device (via traditional UNIX write or via mmap), while the pop callbacks are executed just before that. So, they are called at the moment when the influence from a bad rate estimation via the smoother is the greatest. Now the suggestions. I think that, ideally, for such use cases, it is important to have timestamped latency snapshots for both sinks and sources in PulseAudio core. This would mean introducing a new message that gets the latest reliable latency snapshot (i.e., timestamp according to the wall clock, send/receive counter, input/output buffer size, and the latency itself), without any interpolation. If the sink does not implement this, just fallback (in the generic sink code) to getting the current latency. Also, because such snapshots for the sink and the source will not happen at the same moment, you have to deal with it. You can actually try and get both snapshots at the same time. I did this and was quite astonished to find that the results were less reliable this way. I could not figure out why. (You can call get_latency_in_thread() for source and sink from both snapshot functions without crashing pulse, at least when you make sure they are ONLY called from the timer function. Something else seems to call one of the snapshots for whatever reason). Also, I have a very heretic thought. Namely, that the smoother in the alsa sink and source may actually be a bad idea and is better removed. I have not tested this. But it is used only in two places: for reporting latency (where it confuses your module) and for calculating the amount of time to sleep As I said, I think the latency deviations I see are real and not artifacts, so there is no confusion. in the case of timer-based scheduling (where even module-alsa-sink does not trust the result, i.e. discards it if it is greater than the non-transformed time interval). And, if I recollect correctly, there were complaints about it being fooled by batch cards, and they were cited as one of the reasons not to enable timer-based scheduling on batch cards. So - maybe, for the purposes of timer based-scheduling we should just assume the worst case, i.e. the card that is, say, 0.75% faster than nominal, and use the nominal rate together with the latest snapshot time in {source,sink}_get_latency()? Basically, the fear is that the smoother makes a greater mistake in the esti
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
06.02.2015 14:56, Georg Chini wrote: One more thing: There is a systematic error in the adjust_time I could not work around without introducing too much overhead. The latency snapshot varies widely in the execution time, I measured values between 50 us and more than 60 ms. So if the extreme values follow each other you will have one adjust time that is around 60 ms too long and another one which is 60 ms too short. Maybe this also contributes significantly to the (in)stability of regulation. Well, I have looked into this issue. Basically, you have a callback, time_callback, which is called every u->adjust_time microseconds, according to a timer. All it does is to send asynchronous messages to the sink input and the source output using pa_asyncmsgq_send(), and they fill in various portions of the latency snapshot by querying the relevant memblockq and the sink/source itself, as well as snapshotting the total length of data received or sent. A potential problem is that pa_source_get_latency_within_thread() and pa_sink_get_latency_within_thread() themselves, in the case of an alsa source, go through a smoother (see source_get_latency() in src/modules/alsa/alsa-source.c), which _also_ tries to do sample rate estimation for you! Try to avoid that, even though this means code duplication. Unfortunately, this is easy to recommend, but I can't really see how this can be done. The smoother is updated _after_ a successful write to the alsa device (via traditional UNIX write or via mmap), while the pop callbacks are executed just before that. So, they are called at the moment when the influence from a bad rate estimation via the smoother is the greatest. Now the suggestions. I think that, ideally, for such use cases, it is important to have timestamped latency snapshots for both sinks and sources in PulseAudio core. This would mean introducing a new message that gets the latest reliable latency snapshot (i.e., timestamp according to the wall clock, send/receive counter, input/output buffer size, and the latency itself), without any interpolation. If the sink does not implement this, just fallback (in the generic sink code) to getting the current latency. Also, because such snapshots for the sink and the source will not happen at the same moment, you have to deal with it. Also, I have a very heretic thought. Namely, that the smoother in the alsa sink and source may actually be a bad idea and is better removed. I have not tested this. But it is used only in two places: for reporting latency (where it confuses your module) and for calculating the amount of time to sleep in the case of timer-based scheduling (where even module-alsa-sink does not trust the result, i.e. discards it if it is greater than the non-transformed time interval). And, if I recollect correctly, there were complaints about it being fooled by batch cards, and they were cited as one of the reasons not to enable timer-based scheduling on batch cards. So - maybe, for the purposes of timer based-scheduling we should just assume the worst case, i.e. the card that is, say, 0.75% faster than nominal, and use the nominal rate together with the latest snapshot time in {source,sink}_get_latency()? Basically, the fear is that the smoother makes a greater mistake in the estimated rate than just assuming the nominal one. Maybe you can try this suggestion? For Tanu's patch status page: please leave the status of this patch as unreviewed. The general idea of the patch does not look brilliant, but it's the best known-working idea that we currently have on the topic, and I have not reviewed all the fine details. -- Alexander E. Patrakov ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On 06.02.2015 11:02, Alexander E. Patrakov wrote: 06.02.2015 14:56, Georg Chini wrote: One more thing: There is a systematic error in the adjust_time I could not work around without introducing too much overhead. The latency snapshot varies widely in the execution time, I measured values between 50 us and more than 60 ms. So if the extreme values follow each other you will have one adjust time that is around 60 ms too long and another one which is 60 ms too short. Maybe this also contributes significantly to the (in)stability of regulation. Thanks for this observation. I will look into it, too. I just ran perf to look at the difference between using module-loopback with "latency_msec=10" and "latency_msec=6 buffer_latency_msec=4". I cannot clock down my CPU, it is an Atom board. Top shows about 14% CPU with the first option and 9% with the second option. With perf I cannot see where this difference comes from: latency_msec=10 (everything below 0.5% omitted): 2,67% alsa-sink-ALC88 libspeexdsp.so.1.5.0[.] 0x985d 2,11% alsa-sink-ALC88 libspeexdsp.so.1.5.0[.] 0x9855 1,92% alsa-sink-ALC88 libspeexdsp.so.1.5.0[.] 0x984c 1,88% alsa-sink-ALC88 libspeexdsp.so.1.5.0[.] 0x9844 1,71% alsa-sink-ALC88 libspeexdsp.so.1.5.0[.] 0x986d 1,24% alsa-sink-ALC88 [snd_hda_intel] [k] 0x00a7 0,74% alsa-sink-ALC88 libspeexdsp.so.1.5.0[.] 0x9858 0,66% alsa-sink-ALC88 libspeexdsp.so.1.5.0[.] 0x984f 0,52% alsa-sink-ALC88 libspeexdsp.so.1.5.0[.] 0x9861 latency_msec=6 buffer_latency_msec=4 (everything below 0.5% omitted): 3,51% alsa-sink-ALC88 libspeexdsp.so.1.5.0 [.] 0x985d 2,64% alsa-sink-ALC88 libspeexdsp.so.1.5.0 [.] 0x9855 2,45% alsa-sink-ALC88 libspeexdsp.so.1.5.0 [.] 0x9844 2,44% alsa-sink-ALC88 libspeexdsp.so.1.5.0 [.] 0x984c 2,29% alsa-sink-ALC88 libspeexdsp.so.1.5.0 [.] 0x986d 1,83% alsa-sink-ALC88 libspeexdsp.so.1.5.0 [.] 0x9a02 1,69% alsa-sink-ALC88 libspeexdsp.so.1.5.0 [.] 0x9a11 0,99% alsa-sink-ALC88 libspeexdsp.so.1.5.0 [.] 0x9858 0,97% alsa-sink-ALC88 [snd_hda_intel] [k] 0x00a7 0,87% alsa-sink-ALC88 libspeexdsp.so.1.5.0 [.] 0x984f 0,69% alsa-sink-ALC88 libspeexdsp.so.1.5.0 [.] 0x9a2f 0,61% alsa-sink-ALC88 libspeexdsp.so.1.5.0 [.] 0x9861 0,54% alsa-sink-ALC88 libspeexdsp.so.1.5.0 [.] 0x9a2c As there seem to be less overall busy cycles in the second case the somewhat higher (instead of lower as expected) numbers can be explained by that. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
06.02.2015 14:56, Georg Chini wrote: One more thing: There is a systematic error in the adjust_time I could not work around without introducing too much overhead. The latency snapshot varies widely in the execution time, I measured values between 50 us and more than 60 ms. So if the extreme values follow each other you will have one adjust time that is around 60 ms too long and another one which is 60 ms too short. Maybe this also contributes significantly to the (in)stability of regulation. Thanks for this observation. I will look into it, too. -- Alexander E. Patrakov ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On 06.02.2015 09:42, Georg Chini wrote: On 06.02.2015 08:17, Alexander E. Patrakov wrote: First of all, thanks for a quick and detailed answer. 06.02.2015 02:02, Georg Chini wrote: On 05.02.2015 16:59, Alexander E. Patrakov wrote: 01.02.2015 03:43, Georg Chini wrote: This is the final version of my patch for module-loopback. It is on top of the patch I sent about an hour ago and contains a lot more changes than the previous versions: - Honor specified latency if possible, if not adjust to the lowest possible value - Smooth switching from fixed latency to dynamic latency source or sink and vice versa - good rate and latency stability, no rate oscillation - adjusts latency as good as your setup allows - fast regulation of latency offsets, adjusts 100 ms offset within 22 seconds (adjust time=1) to 60 seconds (adjust_time=10) - usable latency range 4 - 3 ms - Avoid rewinds and "cannot peek into queue" messages during startup and switching - works with rates between 200 and 19 Hz - maximum latency offset after source/sink switch or at startup around is 200 ms I also introduced a new parameter, buffer_latency_msec which can be used together with latency_msec. If buffer_latency_msec is specified, the resulting latency will be latency_msec + buffer_latency_msec. Latency_msec then refers only to the source/sink latency while buffer_latency_msec specifies the buffer part. This can be used to save a lot of CPU at low latencies, running 10 ms latency with latency_msec=6 buffer_latency_msec=4 gives 8% CPU on my system compared to 12% when I only specify latency_msec=10. Is there any more detailed profiling data for it? E.g., have you tried running perf record / perf report? No, I haven't - I just compared CPU for the current module and my patched version. I did not change the way audio is processed so I would not expect large differences to the current version as long as you only use latency_msec. CPU usage mainly depends on the latency set for sink and source and splitting the latency into buffer and source/sink latency aims at that. I can provide figures if you would like to see them, but I do not know how to use the tools you mention above. Is there a HowTo somewhere? In the kernel source, there is a tools/perf directory. That's where you build perf from. It has a Documentation subdirectory, where you can read perf-record.txt and perf-report.txt. You can also read https://www.mail-archive.com/pulseaudio-discuss@lists.freedesktop.org/msg11469.html , that's how I ran perf on PulseAudio, and some replies in the thread contain improvements to my methodology. Thanks, I'll take a look. I am not quoting the patch, but my opinion is that it needs either comments or some text in the commit message on the following topics: Mh, I thought I already put a lot of comments in ... Why does it use hand-crafted heuristics instead of a PID-controller? Or, is this "Low pass filtered difference between expectation value and observed latency" a PI controller in disguise? What do you mean by "handcrafted heuristics"? It's basic math - how many samples do I need for the requested latency. The equation new_rate = base_rate * (1 + latency_difference / adjust_time) is a simple P-regulator without I or D part, rate difference is proportional to latency difference. Yes, it is a P regulator (and you address the "heuristics" below in your email as "additional restrictions", sorry for not noticing the regulator behind them), and your explanation for choosing the coefficient is quoted below. There were two options - either to get the latency right as quickly as possible - that would be base_rate * (1 + latency_difference / (adjust_time + latency)) - or to get the fastest convergence to the latency at the expected rate. I choose the second option. With latency << adjust_time, I do not treat these two possibilities as significantly different options Only correct for small latencies, if you specify latency = 3 ms and adjust time = 1000 ms this obviously not applies, so for large latencies you see a significant difference between the two. . The module makes the decision about the new rate once in adjust_time seconds. The equation that you quoted (either one) means that you aim for the latency difference to be fully corrected just at the next time your module makes a decision. I am not 100% sure that it is the optimal strategy, especially since in the text below you talk about too-sensitive controller and even instability/oscillation. That is not totally correct. Due to min_cycles, the controller does not aim for 100% correction but for a (slightly) lower value, which seems to be enough to avoid overshoot. Also I do not try to correct the whole latency difference in one step, this is only the case when you are near the expected latency so that min_cycles is nearly 1. Else I am only correcting what can be corrected within one step. I experimented with adding I- or
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On 06.02.2015 08:17, Alexander E. Patrakov wrote: First of all, thanks for a quick and detailed answer. 06.02.2015 02:02, Georg Chini wrote: On 05.02.2015 16:59, Alexander E. Patrakov wrote: 01.02.2015 03:43, Georg Chini wrote: This is the final version of my patch for module-loopback. It is on top of the patch I sent about an hour ago and contains a lot more changes than the previous versions: - Honor specified latency if possible, if not adjust to the lowest possible value - Smooth switching from fixed latency to dynamic latency source or sink and vice versa - good rate and latency stability, no rate oscillation - adjusts latency as good as your setup allows - fast regulation of latency offsets, adjusts 100 ms offset within 22 seconds (adjust time=1) to 60 seconds (adjust_time=10) - usable latency range 4 - 3 ms - Avoid rewinds and "cannot peek into queue" messages during startup and switching - works with rates between 200 and 19 Hz - maximum latency offset after source/sink switch or at startup around is 200 ms I also introduced a new parameter, buffer_latency_msec which can be used together with latency_msec. If buffer_latency_msec is specified, the resulting latency will be latency_msec + buffer_latency_msec. Latency_msec then refers only to the source/sink latency while buffer_latency_msec specifies the buffer part. This can be used to save a lot of CPU at low latencies, running 10 ms latency with latency_msec=6 buffer_latency_msec=4 gives 8% CPU on my system compared to 12% when I only specify latency_msec=10. Is there any more detailed profiling data for it? E.g., have you tried running perf record / perf report? No, I haven't - I just compared CPU for the current module and my patched version. I did not change the way audio is processed so I would not expect large differences to the current version as long as you only use latency_msec. CPU usage mainly depends on the latency set for sink and source and splitting the latency into buffer and source/sink latency aims at that. I can provide figures if you would like to see them, but I do not know how to use the tools you mention above. Is there a HowTo somewhere? In the kernel source, there is a tools/perf directory. That's where you build perf from. It has a Documentation subdirectory, where you can read perf-record.txt and perf-report.txt. You can also read https://www.mail-archive.com/pulseaudio-discuss@lists.freedesktop.org/msg11469.html , that's how I ran perf on PulseAudio, and some replies in the thread contain improvements to my methodology. Thanks, I'll take a look. I am not quoting the patch, but my opinion is that it needs either comments or some text in the commit message on the following topics: Mh, I thought I already put a lot of comments in ... Why does it use hand-crafted heuristics instead of a PID-controller? Or, is this "Low pass filtered difference between expectation value and observed latency" a PI controller in disguise? What do you mean by "handcrafted heuristics"? It's basic math - how many samples do I need for the requested latency. The equation new_rate = base_rate * (1 + latency_difference / adjust_time) is a simple P-regulator without I or D part, rate difference is proportional to latency difference. Yes, it is a P regulator (and you address the "heuristics" below in your email as "additional restrictions", sorry for not noticing the regulator behind them), and your explanation for choosing the coefficient is quoted below. There were two options - either to get the latency right as quickly as possible - that would be base_rate * (1 + latency_difference / (adjust_time + latency)) - or to get the fastest convergence to the latency at the expected rate. I choose the second option. With latency << adjust_time, I do not treat these two possibilities as significantly different options Only correct for small latencies, if you specify latency = 3 ms and adjust time = 1000 ms this obviously not applies, so for large latencies you see a significant difference between the two. . The module makes the decision about the new rate once in adjust_time seconds. The equation that you quoted (either one) means that you aim for the latency difference to be fully corrected just at the next time your module makes a decision. I am not 100% sure that it is the optimal strategy, especially since in the text below you talk about too-sensitive controller and even instability/oscillation. That is not totally correct. Due to min_cycles, the controller does not aim for 100% correction but for a (slightly) lower value, which seems to be enough to avoid overshoot. Also I do not try to correct the whole latency difference in one step, this is only the case when you are near the expected latency so that min_cycles is nearly 1. Else I am only correcting what can be corrected within one step. I experimented with adding I- or D-parts but did not see any significant im
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
First of all, thanks for a quick and detailed answer. 06.02.2015 02:02, Georg Chini wrote: On 05.02.2015 16:59, Alexander E. Patrakov wrote: 01.02.2015 03:43, Georg Chini wrote: This is the final version of my patch for module-loopback. It is on top of the patch I sent about an hour ago and contains a lot more changes than the previous versions: - Honor specified latency if possible, if not adjust to the lowest possible value - Smooth switching from fixed latency to dynamic latency source or sink and vice versa - good rate and latency stability, no rate oscillation - adjusts latency as good as your setup allows - fast regulation of latency offsets, adjusts 100 ms offset within 22 seconds (adjust time=1) to 60 seconds (adjust_time=10) - usable latency range 4 - 3 ms - Avoid rewinds and "cannot peek into queue" messages during startup and switching - works with rates between 200 and 19 Hz - maximum latency offset after source/sink switch or at startup around is 200 ms I also introduced a new parameter, buffer_latency_msec which can be used together with latency_msec. If buffer_latency_msec is specified, the resulting latency will be latency_msec + buffer_latency_msec. Latency_msec then refers only to the source/sink latency while buffer_latency_msec specifies the buffer part. This can be used to save a lot of CPU at low latencies, running 10 ms latency with latency_msec=6 buffer_latency_msec=4 gives 8% CPU on my system compared to 12% when I only specify latency_msec=10. Is there any more detailed profiling data for it? E.g., have you tried running perf record / perf report? No, I haven't - I just compared CPU for the current module and my patched version. I did not change the way audio is processed so I would not expect large differences to the current version as long as you only use latency_msec. CPU usage mainly depends on the latency set for sink and source and splitting the latency into buffer and source/sink latency aims at that. I can provide figures if you would like to see them, but I do not know how to use the tools you mention above. Is there a HowTo somewhere? In the kernel source, there is a tools/perf directory. That's where you build perf from. It has a Documentation subdirectory, where you can read perf-record.txt and perf-report.txt. You can also read https://www.mail-archive.com/pulseaudio-discuss@lists.freedesktop.org/msg11469.html , that's how I ran perf on PulseAudio, and some replies in the thread contain improvements to my methodology. I am not quoting the patch, but my opinion is that it needs either comments or some text in the commit message on the following topics: Mh, I thought I already put a lot of comments in ... Why does it use hand-crafted heuristics instead of a PID-controller? Or, is this "Low pass filtered difference between expectation value and observed latency" a PI controller in disguise? What do you mean by "handcrafted heuristics"? It's basic math - how many samples do I need for the requested latency. The equation new_rate = base_rate * (1 + latency_difference / adjust_time) is a simple P-regulator without I or D part, rate difference is proportional to latency difference. Yes, it is a P regulator (and you address the "heuristics" below in your email as "additional restrictions", sorry for not noticing the regulator behind them), and your explanation for choosing the coefficient is quoted below. There were two options - either to get the latency right as quickly as possible - that would be base_rate * (1 + latency_difference / (adjust_time + latency)) - or to get the fastest convergence to the latency at the expected rate. I choose the second option. With latency << adjust_time, I do not treat these two possibilities as significantly different options. The module makes the decision about the new rate once in adjust_time seconds. The equation that you quoted (either one) means that you aim for the latency difference to be fully corrected just at the next time your module makes a decision. I am not 100% sure that it is the optimal strategy, especially since in the text below you talk about too-sensitive controller and even instability/oscillation. I experimented with adding I- or D-parts but did not see any significant improvement (maybe because of incorrect parameter values). I will try to look into this on Sunday, then. But this first means getting the P-part right. Here is what I think here, but not yet 100% sure of. If you use this (obviously wrong) equation: new_rate = base_rate * (1 + 2 * latency_difference / adjust_time) then it will overcorrect the latency difference, so that it becomes exactly the negatine of what it was before. And so on and so on, i.e. there will be oscillations. With values of the factor before latency_difference between 1 and 2, overcorrection will also happen, but its absolute value will be reduced each time. So, 2 seems to be the critical value that leads to
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
On 05.02.2015 16:59, Alexander E. Patrakov wrote: 01.02.2015 03:43, Georg Chini wrote: This is the final version of my patch for module-loopback. It is on top of the patch I sent about an hour ago and contains a lot more changes than the previous versions: - Honor specified latency if possible, if not adjust to the lowest possible value - Smooth switching from fixed latency to dynamic latency source or sink and vice versa - good rate and latency stability, no rate oscillation - adjusts latency as good as your setup allows - fast regulation of latency offsets, adjusts 100 ms offset within 22 seconds (adjust time=1) to 60 seconds (adjust_time=10) - usable latency range 4 - 3 ms - Avoid rewinds and "cannot peek into queue" messages during startup and switching - works with rates between 200 and 19 Hz - maximum latency offset after source/sink switch or at startup around is 200 ms I also introduced a new parameter, buffer_latency_msec which can be used together with latency_msec. If buffer_latency_msec is specified, the resulting latency will be latency_msec + buffer_latency_msec. Latency_msec then refers only to the source/sink latency while buffer_latency_msec specifies the buffer part. This can be used to save a lot of CPU at low latencies, running 10 ms latency with latency_msec=6 buffer_latency_msec=4 gives 8% CPU on my system compared to 12% when I only specify latency_msec=10. Is there any more detailed profiling data for it? E.g., have you tried running perf record / perf report? No, I haven't - I just compared CPU for the current module and my patched version. I did not change the way audio is processed so I would not expect large differences to the current version as long as you only use latency_msec. CPU usage mainly depends on the latency set for sink and source and splitting the latency into buffer and source/sink latency aims at that. I can provide figures if you would like to see them, but I do not know how to use the tools you mention above. Is there a HowTo somewhere? I am not quoting the patch, but my opinion is that it needs either comments or some text in the commit message on the following topics: Mh, I thought I already put a lot of comments in ... Why does it use hand-crafted heuristics instead of a PID-controller? Or, is this "Low pass filtered difference between expectation value and observed latency" a PI controller in disguise? What do you mean by "handcrafted heuristics"? It's basic math - how many samples do I need for the requested latency. The equation new_rate = base_rate * (1 + latency_difference / adjust_time) is a simple P-regulator without I or D part, rate difference is proportional to latency difference. There were two options - either to get the latency right as quickly as possible - that would be base_rate * (1 + latency_difference / (adjust_time + latency)) - or to get the fastest convergence to the latency at the expected rate. I choose the second option. I experimented with adding I- or D-parts but did not see any significant improvement (maybe because of incorrect parameter values). There are however two additional restrictions: 1) Do not deviate too much (I choose 0.75%) from the base rate 2) Do the adjustment in small amounts at a time The first restriction was solved by introducing min_cycles, which makes sure the rate cannot deviate more than 0.75% from the base rate. It also produces some dampening of the P-regulator and smooths out the steps produced by the second restriction (at least when you are approaching the base rate). For 2) I just copied what was already in the current code. And then I was looking for the right criterion when to stop regulation. When is the latency "good enough"? Using some percentage of the latency will not work well when you have a latency range of 4 - 3 ms to cover (at varying rates). Just cutting off when you are a few Hz away from the base rate like the current code does is also not a good idea for the same reason (100 - 19 Hz). The best you can get is an error of adjust_time / (2 * base_rate). But this does not work either because the latency measurement itself has some error and there is some jitter in the latency. This easily leads to instable rates or oscillation because the controller follows every tiny change. The jitter gets significant for example if you are using USB devices that are connected to a busy USB hub. The approach was then to additionally use the error of the regulation as a stop criterion. I calculate the next expected latency and compare it to the real latency I receive. When the difference between configured latency and current latency is larger than twice this error, the difference can be considered significant. Adding the "best error" from above and a bit of safety margin finally leads to the stop criterion in the patch. The low pass filter of the error is necessary to get some running average value. I have
Re: [pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency
01.02.2015 03:43, Georg Chini wrote: This is the final version of my patch for module-loopback. It is on top of the patch I sent about an hour ago and contains a lot more changes than the previous versions: - Honor specified latency if possible, if not adjust to the lowest possible value - Smooth switching from fixed latency to dynamic latency source or sink and vice versa - good rate and latency stability, no rate oscillation - adjusts latency as good as your setup allows - fast regulation of latency offsets, adjusts 100 ms offset within 22 seconds (adjust time=1) to 60 seconds (adjust_time=10) - usable latency range 4 - 3 ms - Avoid rewinds and "cannot peek into queue" messages during startup and switching - works with rates between 200 and 19 Hz - maximum latency offset after source/sink switch or at startup around is 200 ms I also introduced a new parameter, buffer_latency_msec which can be used together with latency_msec. If buffer_latency_msec is specified, the resulting latency will be latency_msec + buffer_latency_msec. Latency_msec then refers only to the source/sink latency while buffer_latency_msec specifies the buffer part. This can be used to save a lot of CPU at low latencies, running 10 ms latency with latency_msec=6 buffer_latency_msec=4 gives 8% CPU on my system compared to 12% when I only specify latency_msec=10. Is there any more detailed profiling data for it? E.g., have you tried running perf record / perf report? I am not quoting the patch, but my opinion is that it needs either comments or some text in the commit message on the following topics: Why does it use hand-crafted heuristics instead of a PID-controller? Or, is this "Low pass filtered difference between expectation value and observed latency" a PI controller in disguise? How was the lowpass filter tuned? -- Alexander E. Patrakov ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss