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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
