On 27.02.2017 11:17, Tanu Kaskinen wrote: > On Sat, 2017-02-25 at 22:23 +0100, Georg Chini wrote: >>>> Sorry, slowly I am thinking that you don't want the patches at all and >>>> are nitpicking to discourage me. If you don't want the patches, just say so. >>> I assure you, I want these patches, especially this one, because this >>> patch should fix two bugs that I know have caused people trouble (slow >>> adjustment to target latency and accumulating a lot of latency when the >>> sink is slow to start). >>> >>> I fully understand that the process is frustrating when we have trouble >>> understanding each other. It's frustrating to me too. I wish I knew how >>> to make it easier for us to reach common understanding. >>> >>> If you think that some criticism is excessive nitpicking, and you don't >>> want to do the suggested change, you can say "I prefer doing it this >>> way. If you want to change it, you can write a patch later yourself, >>> ok?" That has been known to work sometimes. >> Thanks. >> >>>> Thinking twice, there is not even a way you could get to that call >>>> while the sink is suspended, so pa_sink_get_latency_in_thread() will >>>> always return some value. >>> Are you still referring to this code: >>> >>> + /* If pop has not been called yet, make sure the latency does not grow too much. >>> + * Don't push any silence here, because we already have new data in the queue */ >>> + if (!u->output_thread_info.pop_called) >>> + memblockq_adjust(u, pa_sink_get_latency_within_thread(u->sink_input->sink), false); >>> >>> The only condition is that pop hasn't been called, and from that you >>> certainly can't draw the conclusion that the sink is not suspended. I >>> am again very confused. >>> >> You are right. I even needed a check for the sink being suspended when >> detecting an underrun one line further down in the code ... >> The call is more or less there for symmetry reasons similar to the the >> SINK_CHANGED message (which I will remove). I can remove it as well >> if you like. It should not have much effect. > Which call do you mean? Do you mean the memblockq_adjust() call quoted > above, the one that is called when pop hasn't been called? That > adjustment seems useful, because it prevents the memblockq storing huge > amounts of data if it takes a long time for the sink to start. I > thought that was the reason for having the call, not just to have > symmetry. But it's true that the call is not absolutely necessary. I am not talking about the memblockq_adjust() call itself, but of the pa_sink_get_latency_within_thread() call within the memblockq_adjust() call, which is the one you complained about. > >> This also applies for the other call in sink_input_pop_cb(). > Do you mean that you can remove also the memblockq_adjust() call from > pop_cb()? Yes, that one can safely be removed, if you generate silence > in a different way than relying on the memblockq to produce it. Uh, sorry, I mean removing the effective_source_latency variable from the memblockq_adjust() call in sink_input_pop_cb()
