[pulseaudio-discuss] Loopback patches

[Tanu Kaskinen]

On Tue, 2017-01-03 at 15:44 +0100, Georg Chini wrote:
> Hi Tanu,
> 
> finally I am finding the time and motivation to continue on the loopback 
> patches.

Great!

> Sorry for the long delay, I hope you still remember some of it. I have a few
> questions regarding the thread separation. You write (20.7.2016):
> 
> "I'd be happy if you could somehow clearly divide the variables in
> userdata based on from which context they're supposed to be accessed.
> For example, pa_sink has field "thread_info", which is an embedded
> struct that holds the sink's IO thread variables, and there are other
> similar uses of a "thread_info" struct elsewhere. You could add
> "sink_thread_info" and "source_thread_info" to the userdata struct."
> 
> On the other hand, looking through your comments you only complain
> about variables belonging to the main thread which are (wrongly) accessed
> from an IO thread:
> 
> 1) source_sink_changed -> only set in main thread and detach() callback, 
> so OK,
> no change needed.
> 
> 2) iteration_counter -> only set in main thread, in detach() callback 
> and in conjunction
> with the two calls of update_minimum_latency(). Will be corrected with 
> the changes
> necessary for update_minimum_latency()
> 
> 3) latency_error -> like source_sink_changed
> 
> 4) extra_latency -> like iteration_counter
> 
> 5) underrun_counter -> like iteration_counter plus one additional 
> asynchronous message
> in sink_input_process_msg_cb(). An asynchronous update is OK, so no need 
> to have a
> underrun_counter_in_thread variable.
> 
> 6) underrun_occurred -> This flag is used as a message between the 
> threads, the
> output thread sets it when an underrun occurs and it is reset by the 
> main thread.
> So I think the usage is OK in this case.

If you send a message when underrun_counter changes, doesn't that also
imply that an underrun occurred, so a separate flag isn't needed?

> 6) minimum_latency -> will be corrected with the changes for 
> update_minimum_latency()
> 
> There are also two functions, which are currently accessed from multiple 
> threads:
> 
> 1) memblockq_adjust() which should only be used from the output thread. 
> No problem,
> can be corrected by using a synchronous message.
> 
> 2) update_minimum_latency() which should only be used from the main 
> thread. Will be
> corrected by sending asynchronous messages to the main thread. The use of a
> minimum_latency_in_thread variable is not necessary I think, because it 
> does not hurt
> if the IO threads gets the information only after the message has been 
> processed by
> the main thread.
> 
> Is my assessment correct and does the division you are asking for really 
> make sense
> under these circumstances?

It seems correct, and if dividing the variables doesn't seem useful to
you, feel free to not do that.

-- 
Tanu

https://www.patreon.com/tanuk