On 25.02.2015 03:25, Arun Raghavan wrote: > On 23 February 2015 at 22:10, Alexander E. Patrakov <patrakov at gmail.com> wrote: >> 23.02.2015 21:12, Peter Meerwald wrote: >>> Hi Alexander, >>> >>>> Same bug as in module-loopback, pointed out by Georg Chini in a private >>>> email. >>> >>> can you please elaborate? I fail to see the obviousness >> >> The recv_counter == send_counter case looks like a neutral case here >> (although I might be wrong), with no inherent discontinuity at this point. >> So, any branch of the "if" should give the same result in this corner case, >> and the whole point of the "if"/"PA_CLIP_SUB" construction, as I see it, is >> to avoid producing negative numbers by mistake. >> >> The true branch simplifies to: buffer_latency += 0 >> >> The original false branch simplifies to: buffer_latency += >> PA_CLIP_SUB(buffer_latency, 0), which, for buffer_latency > 0, means >> buffer_latency += buffer_latency - 0. I.e. doubling the latency (as opposed >> to not changing it), which looks strange even by itself. > The falce branch would only be called when recv_counter > > send_counter, so you'd never actually have the zero case. AIUI, this > all just accounting for any "in-flight" rewinds being propagated from > the sink-input to the source-output. > > Also, overwriting the value as you do loses the latency values that > were added in previous lines. > > -- Arun > _______________________________________________ > I cannot understand what is to discuss about that patch. Calculating buffer = buffer + (send_counter - recv_counter) in one branch and buffer = 2 * buffer - (recv_counter - send_counter) looks very obviously wrong. What should be the reason for doubling the buffer in the second case?
