[pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency

[Georg Chini]

>>>> in the case of timer-based scheduling (where even module-alsa-sink
>>>> does not trust the result, i.e. discards it if it is greater than the
>>>> non-transformed time interval). And, if I recollect correctly, there
>>>> were complaints about it being fooled by batch cards, and they were
>>>> cited as one of the reasons not to enable timer-based scheduling on
>>>> batch cards. So - maybe, for the purposes of timer based-scheduling we
>>>> should just assume the worst case, i.e. the card that is, say, 0.75%
>>>> faster than nominal, and use the nominal rate together with the latest
>>>> snapshot time in {source,sink}_get_latency()? Basically, the fear is
>>>> that the smoother makes a greater mistake in the estimated rate than
>>>> just assuming the nominal one. Maybe you can try this suggestion?
>>>>
>>>
>>> For timer based scheduling the regulator works perfect, you would not
>>> even need a stop criterion,
>>> so why bother?
>
> I think there is some misunderstanding. Let me repeat in a different way.
>
> The smoother works perfectly (both for timer-based scheduling and for 
> the needs of your module) on non-batch cards.
>
> But, even for batch cards, where timer-based scheduling is disabled, 
> the smoother is active and is actually used for reporting the latency 
> to your module. An attempt to use the smoother for timer-based 
> scheduling on batch cards has failed. That's why I suspect that it, on 
> batch cards, also tells lies to your module.

OK, understood. I don't have anything to test it though.

>
>>>
>>>> For Tanu's patch status page: please leave the status of this patch as
>>>> unreviewed. The general idea of the patch does not look brilliant, but
>>>> it's the best known-working idea that we currently have on the topic,
>>>> and I have not reviewed all the fine details.
>>>>
>>>
>>> Well from a practical point of view it does a pretty good job although
>>> the idea may not be brilliant.
>>> I'm willing to implement your better idea when you come up with it.
>>>   Did you ever test it? And compare it to what the current
>>> module-loopback does?
>>
>> I did not test it, will do it now and add some logging in order to
>> verify what you said above. And hopefully will try to implement an
>> alternative latency-snapshotting implementation, just to compare.
>>
>
> I can confirm (based on a reimplementation attempt) that the code 
> after patching deals with the capture and playback timestamp 
> difference 100% correctly - so it cannot be the problem. Just a minor 
> nitpick: I moved saving of the timestamp to the message handlers. For 
> me, this makes no difference, though. The patch (to be applied on top 
> of yours) is attached. Could you please confirm or disprove that it 
> makes no difference in your setup, either?
>

I tried the same and measured the time difference between the two 
methods. It is around 1 - 2 us.
So it does not really matter if you obtain the time stamp within the 
snapshot or outside.
I thought it was more simple the way I implemented it, but I have no 
objection to your change.

> So, the current status of the patch, from my viewpoint, is:
>
> 1. The patch adds a perfectly correct (assuming no xruns) way to 
> account for latency snapshots being made not simultaneously for 
> playback and capture. I think that this is the main improvement, and 
> it needs to be merged even if we disagree on the rest.
>
> 2. The result has an optimal coefficient that relates the observed 
> latency difference and the resulting rate correction, assuming the 
> currently-implemented way to snapshot the latency and assuming no 
> interference from the smoother - which still has to be verified 
> independently, possibly after merging.
>
> 3. The patch adds buffer_latency_msec, which seems to be an unrelated 
> improvement, and I think it should be split out. I have no opinion on 
> whether this change should be merged.

I'm fine with separating this out. I am even fine with dropping it 
completely, but I thought
it might be useful for those who know what they are doing and for those 
who have to care
about used CPU cycles.

>
> 4. The patch has a criterion when to stop adjusting rates, and it is a 
> source of disagreements. But I could not suggest anything 
> constructive. So I think that a good approach would be to split it out 
> and let others comment. Also, it would be a good idea to add a 
> debugging message so that we can see when it happens.
>

It nearly always happens when you are approaching the requested latency. 
The case where
you hit the base rate spot on is very rare, so a message doesn't make 
sense for me.
Maybe it would make sense to print out the latency_error somewhere 
because it is an
indicator how good your latency can be adjusted. Without stop criterion 
you will see instabilities
with USB devices in certain situations, so I would not merge the patch 
without it.
Even if the regulator were completely stable in all tested situations I 
would still add something
like it, just to make sure. The current module-loopback also contains a 
stop criterion which
in my opinion is insufficient and should be replaced.

> If you want, I can do the splitting for you.
>

Thanks, please do so. It makes more sense when you do it because you 
better know
what you want to separate.

>
>
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/pulseaudio-discuss/attachments/20150208/f9095de1/attachment-0001.html>