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
