Re: [RESEND][PATCH v4 1/3] ALSA: core: let low-level driver or userspace disable rewinds

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 03/29/2018 03:10 PM, Takashi Iwai wrote:
On Thu, 29 Mar 2018 21:16:58 +0200,
Pierre-Louis Bossart wrote:


On 03/29/2018 10:42 AM, Takashi Iwai wrote:
On Wed, 28 Mar 2018 23:51:46 +0200,
Pierre-Louis Bossart wrote:
Chrome can be benefit immediately since rewinds are never used.
Jack can benefit immediately since rewinds are never used
Android HALs can benefit immediately since rewinds are never used.
... if you patch all these.  That's not immediate.
It's a single flag to add to the hw_params. It'll take time to ripple
to distros but it's not really complex.
Yes, but you have to modify *each* of them.  That's flaky.
Sure, it's safer for not enabling it as default, but is it your goal?

OTOH, if you do introduce a single switch, it's immediately
effective, even with the old user-space without compilation.
Humm, now you lost me.
In your replies, you stated that the applications need to tell the
driver - but disagreed that a hw_params flag was the right way. I am
not religious here, as long as it remains simple enough we can look at
other options.

I don't get however what 'single switch' are you referring to here?
Just a simple module option or a kctl, for example.
That is, if we can forget about the ability for adjustment per stream,
the operation mode can be flipped by a switch on the fly.  This makes
things a lot easier.

In all cases, I don't see how we can enable this without rebuilding
the apps.
Except for ChromeOS, I don't know how a distro would enable a switch
that would impact all apps using the ALSA API.
So here came the question how it would impact on PA.
If it doesn't work for PA, it means that essentially all traditional
Linux distros will turn it off as default.  It's only for Android and
Chrome OS, and they have own setup.  They can turn it on as default.

If anyone wants to turn it on for JACK, they can do it.  But I don't
think people would mix up JACK and PA on the same system at the very
same time.  (One can hook up as a client, but then it doesn't matter
about this hardware feature.)
I do recall Lennart implementing a mechanism to hand-over the ALSA
resources between PulseAudio and Jack so there is a precedent here.
And, now back to the question: in which situation you'll have to mix
both no-rewind and rewind operations on a single machine?  In theory,
yes, we may keep rewind working, but from the practical POV, it is
only PA who uses the rewind feature.  And, even if we disable rewind,
PA will still work as is.  (I know it because I broke it previously :)
define 'work'. If PulseAudio cannot use the rewinds, then the system
sounds will be heard with a delay and transitions between use cases
will only happen when the queued audio is finished playing.
I just tried music players with PA, and disabled the rewind forcibly
(by putting return 0 in rewind_appl_ptr() in pcm_native.c).  I
couldn't hear any audible difference in its response for changing the
stream position, etc, interactively.  There was no audible latency
increase by that change.

It was the test weeks ago, so I refreshed testing now.  And the
result is same, I saw no response difference.
Any specific workload to make the clear difference in your mind?
It depends on how the app is written and what the configuration of the
server is.
Isn't it about what PA does, not apps?
The PulseAudio server has parameters, but the app can ask for a different behavior depending on its needs by passing a pa_buffer_attr structure, with a -1 value meaning "highest possible latency". This is only possible btw with the regular API, apps using the simple API will not control this behavior and can't ask for a specific latency.

https://www.freedesktop.org/wiki/Software/PulseAudio/Documentation/Developer/Clients/LatencyControl/


Back in the Meego days the plan was to use very long
buffers and not rewinding did impact user experience. It's the same
issue on Windows and Android with deep buffers, source transitions,
volume changes can be painful when the committed samples cannot be
invalidated.
Yes, but we're talking specifically about the Intel HD-audio, and it
apparently works as is without rewind.  So an API change looks
overcommitting (even though it's one bit flag addition).

If the feature is really generic in wider hardware ranges, it's a
convincing factor for changing the API.  Let's see.

If an app wants to play a sound immediately due to user interaction
requirements, rewinds are an attractive solution. I don't know how we
could determine ahead of time what userspace will do, certainly the
use of rewinds is on a per-card basis and likely a per-stream
decision.

So, what's wrong with introducing a global switch instead of changing
the existing API and requiring the code change in each application and
rebuild?
I totally agree that pretty much all apps don't do any rewinds, and
rewinds on capture is quite useless. But I stuck to the 'we don't
break userspace' mantra with the idea of an opt-in flag requiring an
explicit code change to take effect. we will break userspace if we add
a kernel-level switch, and the net result is that no one will ever use
this option.
Sure, I agree with the golden "no regression" rule, too.  So I thought
of some switch that is default off for the systems that may use
rewind, while turning it on as default on the known systems like
Android.  As PA is the sole user of rewind, here we won't see any
regression, in theory.
The regression will depend on the depth of the buffer and whether apps
are ok to indicate the max latency when connecting with
PulseAudio. we've done crazy things with PulseAudio in the past and
things will go wrong.
Yes, obviously we need testing.  But, in this case, the target
hardware is *very* limited.  So covering this shouldn't be a big
matter, I hope.  (And we'll keep a "big red button" to turn it off in
emergency for some time.)


And, such a system-level switch is preferable from the system
management POV.  It's easier than a hard-coded API extension flag, and
you can toggle it at any time for debugging if a problem happens.

*IFF* the no-rewind feature is required by other multiple systems,
adding some API would make sense.  But, currently, no-rewind is needed
only for enabling some feature that is indirectly involved with the
rewind on Intel chips.  To my eyes, it's too far away from the purpose
of the PCM hw_params API.

(Again, no-irq was in a different situation; it's a flag for a mode
   that didn't exist (zero period) in the hw_params space.  But
   no-rewind is irrelevant with the hw_params configuration itself.)
I hope we didn't give you the wrong impression that we are abusing the
API for nefarious or Intel-specific views.
In the initial patchset some 2+ years ago, there was this change that
disabled rewinds but also an additional functionality that let drivers
expose the granularity of the DMA bursts to userspace (I think
Jaroslav wanted it to be called in-flight-bytes), and possibly set
them depending on the application need. The latter part is of interest
to others, and I don't think removing the rewinds is useful only to
Intel, as soon as people use a second-level buffer rewinds are
problematic for everyone.
I can think of some interface, too, but using hw_params flag is still
questionable.  It's no worst choice, sure, but it doesn't look like
the best fit, either, IMO -- especially because the rewind is no
target of hw_params config space.

We need to be extremely careful if it comes to API.  After all, API is
the only fixed definition we can never ever change.  Once set, it must
be rock-solid for the next decades...

So maybe a way to progress is to leave this flag set as a module
parameter as you suggested for now, but with the information known to
the core, which lets Intel enable functionality on Google OSes
(Android/Chrome), and in a second step (when we are done with SOF
upstream) suggest a more generic API enhancement related to DMA
handling which is not Intel specific.

Would that be agreeable?
Yes, that's a compromise.  I'd really love to have the support for
this kind of feature, while I hesitate to add such a spontaneous bit
flag in the global API level.  So let's begin with the driver-specific
implementation, and extend to API level once when we see what are the
real demands in wide range of hardware.

ok, makes sense.
Thanks for your feedback and enjoy your 4 day weekend.
I'll bug you next week with UCM stuff.
-Pierre
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux