<html> <head> <base href="https://bugs.freedesktop.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW --- - Stuttering when using "Simultaneous output"" href="https://bugs.freedesktop.org/show_bug.cgi?id=47899#c32">Comment # 32</a> on <a class="bz_bug_link bz_status_NEW " title="NEW --- - Stuttering when using "Simultaneous output"" href="https://bugs.freedesktop.org/show_bug.cgi?id=47899">bug 47899</a> from <a class="email" href="mailto:tanuk@iki.fi" title="Tanu Kaskinen <tanuk@iki.fi>"> Tanu Kaskinen</a> <pre>Thanks for the patch again! It's starting to be pretty complicated to review, but I got through it anyway. Comments: Apparently the logic for setting the output requested latency is to forward the combine sink requested latency, except when that is -1, in which case the maximum of the requested latencies of the downstream sinks is used. I think the code is a bit broken if it does try to implement this (there's at least the issue that if any of the downstream sinks has requested latency of -1, that will propagate to all outputs, and sink_update_requested_latency() doesn't handle transitions to -1 correctly), but actually I think this whole logic is unnecessarily complicated. I think it's sufficient to just directly forward the requested latency of the combine sink to the outputs. If you think -1 needs to be handled specially, please explain why. If the requested latency logic is simplified as I suggest, then I think the userdata->requested_latency and output->requested_latency struct fields can be removed. update_latency_range() doesn't handle correctly the case where there are no outputs, leading to crash in pa_sink_set_latency_range_within_thread(). I think it would be best to set both the max and min latencies to u->block_usec in this case. You should call pa_sink_set_latency_range() before calling pa_sink_put() to ensure that the initial latency range is sane. The latency values should be the same that you use in update_latency_range() when there are no outputs. SINK_MESSAGE_UPDATE_MAX_REQUEST and SINK_MESSAGE_UPDATE_LATENCY_RANGE should be sent also from sink_input_detach_cb(). Then there's this issue that was there already before: SINK_MESSAGE_ADD_OUTPUT is sent to the combine sink thread before the output has been attached. After ADD_OUTPUT, the combine sink thread will start accessing the output parameters, but the output parameters are not valid before attaching. Fixing this should probably be done in a separate patch, and maybe this is something that is out of scope for this bug, so you could ignore this. I have added a todo item for myself to fix this. It would be nice to get the next version also on the mailing list for easier commenting.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the QA Contact for the bug.</li> <li>You are the assignee for the bug.</li> </ul> </body> </html>