[pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency

[Georg Chini]

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 - 30000 ms
>> - Avoid rewinds and "cannot peek into queue" messages during startup 
>> and switching
>> - works with rates between 200 and 190000 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.
>>
>