>>>> 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>
