On 2014-04-22 10:18, Alexander E. Patrakov wrote: > 22.04.2014 13:01, David Henningsson wrote: >> >> >> On 2014-04-21 09:04, Alexander E. Patrakov wrote: >>> 21.04.2014 07:49, David Henningsson wrote: >>>> On 2014-04-20 21:26, Alexander E. Patrakov wrote: >>>>> Thus, it is not possible to tell the hardware device (that can use >>>>> rewinds) from a properly wrapped software encoder (that can't rewind >>>>> and >>>>> doesn't pretend to be able to rewind), because for both cases >>>>> snd_pcm_rewindable() would return 0 at the moment PulseAudio needs to >>>>> make a decision. >>>> >>>> The moment PulseAudio needs to make a decision is when a rewind is >>>> requested. >>> >>> No. The decision definitely needs to be at the device-open time. >>> Otherwise this will happen: >>> >>> """ >>> Now I need to rewind in order to accommodate a new low-latency client. >>> Oops, I can't, and I have so much wrong data in my hardware buffer! I >>> should not have created such a big buffer, but now it too late to change >>> anything. >>> """ >> >> So you want to always go low-latency (and high CPU/power consumption), >> in case rewind is not possible? > > Yes, exactly. And high CPU/power consumption will happen anyway, because > non-rewindable things like the DTS encoder are generally very CPU-hungry > by themselves. So the extra 1% lost due to always enabling the lowest > possible latency does not matter. There might be other scenarios where rewinds are disabled, where you cannot make the assumption. > And BTW I was going to submit more patches in this "can't truly rewind > => always select low latency" direction, namely for the ladspa sink, > possibly hidden behind a module parameter. > >> This sounds like a trade-off. >> >> The other possibility would be to just wait until the hw buffer is empty >> and then continue with low latency. If "accomodate a new low latency >> client" happens rarely, and the maximum buffer size is < 2 seconds, then >> maybe this is not much of an issue, compared to the drawback of forcing >> low latency when it's not needed. > > This is not only "accomodate a new low-latency client". This also > applies to volume changes in the case when the only volume control is a > software one (and for the DTS encoder this is true) - you really don't > want them to apply after two seconds. This is also "new client started", > because you want it to be heard as soon as possible, and not after two > seconds. > > So it is IMHO a reasonable trade-off. Fair point. But maybe a medium latency (150 - 200 ms) would be a more reasonable trade-off in this case, than low latency (20 - 50 ms). >>> And indeed, the current code already has logic to choose different >>> buffer sizes for tstamp and irq-driven modes: >>> >>> http://cgit.freedesktop.org/pulseaudio/pulseaudio/tree/src/modules/alsa/alsa-util.c#n298 >>> >>> >>> >>> >>> On my hardware, the buffer sizes for these two modes differ by a factor >>> of 1000. >> >> What hardware is that? If we don't do that already, we should cap the >> tsched buffer size to ~ 2 seconds, or we'll just use more memory than we >> need. Divide that with 1000 and you claim to have a irq-driven buffer >> size of 2 ms. Which is way too low. > > Haswell HDMI, the IRQ-driven buffer size is 6 ms (which indeed doesn't > make any sense for HDMI but doesn't lead to dropouts), the tsched-driven > buffer size is ~10s. Or maybe it is just PulseAudio claiming 10s and > then clamping to 2s and not announcing it to the log. I will check again > later today. The upstream standard for HDA Intel (which includes Haswell HDMI) is 64K, which translates to 371 ms (in 44100Hz, stereo). And PulseAudio's standard for IRQ-driven scheduling is 4 periods * 25 ms = 100 ms. Some distros tweak these numbers, but your values look a bit too weird to believe, unless something is buggy. >>> So what I want is really not related to tsched. "Don't choose a big >>> buffer size and high latency, and don't try to rewind, if we know in >>> advance that ALSA cannot rewind or only pretends to be able to rewind" >>> would be a better description of my patch. >>> >>>> Whether or not to enable tsched should not matter in this case, unless >>>> I'm missing something. (And this is probably what Raymond is trying to >>>> say too.) >>>> Or, put in another way, why would it be better for the ALSA device >>>> to be >>>> in interrupt driven mode just because it can't rewind? >>> >>> I have two slightly-conflicting answers to this. >>> >>> First answer: >>> >>> Rewinds and timestamp-driven scheduling are only the means to get >>> dynamically reconfigurable latency, which is useful for less dropouts >>> when there are no low-latency clients, lower power usage, and possibly >>> other good things. Due to the inability to do rewinds, the "dynamic >>> client-driven latency" goal becomes unachievable, so there is simply no >>> good point to use timestamp-based scheduling in this case. >>> >>> Of course timestamp-based scheduling will work without rewinds, but, as >>> PulseAudio would then need (due to inability to do rewinds) to lock into >>> the constant minimum latency, the wakeup points will be evenly spaced in >>> time. And that's almost equivalent to the IRQ-based scheduling (with a >>> small exception listed in the second answer). >>> >>> Or to put it another way. Currently, PulseAudio supports two models: >>> "big buffer + timestamp-based scheduling + rewinds" and "small buffer + >>> IRQ-driven scheduling + no rewinds". Intermediate models such as "small >>> buffer + timestamp-based scheduling + no rewinds" are possible, but they >>> would IMHO only unnecessarily inflate the test matrix. >> >> Eh? Rewinds are not disabled under IRQ-driven scheduling. > > They are disabled if pa_alsa_pcm_is_hw() returns false, which is exactly > what I am trying to achieve: > > http://cgit.freedesktop.org/pulseaudio/pulseaudio/tree/src/modules/alsa/alsa-sink.c#n1010 > > > http://cgit.freedesktop.org/pulseaudio/pulseaudio/tree/src/modules/alsa/alsa-sink.c#n2346 > > > And this function returning false is also one of the reasons why tsched > can be automatically disabled: > > http://cgit.freedesktop.org/pulseaudio/pulseaudio/tree/src/modules/alsa/alsa-util.c#n245 > > > So indeed, it is possible to have a "tsched disabled but rewinds > enabled" situation (e.g. due to the BATCH flag), but the code that I > touch disables both. Sorry again for the bad patch subject. Ah, okay. I wonder if the BATCH stuff is correct though. But that's subject for another discussion. > In fact, if you look through the git history behind the uses of > pa_alsa_pcm_is_hw() function, you'll notice that one of them was added > because of the AC3 encoder which has the same limitation: no rewinding. > > In particular, "git show cb55b00ccd25d965b1222e74375aee05427a449b" shows > the commit that I am attempting to fix up for the DTS case, because the > existing check in pa_alsa_pcm_is_hw() does not catch it. If you are > still objecting to my commit, and think that the same objections don't > apply to cb55b00ccd25d965b1222e74375aee05427a449b, then I would like to > see why. > > See also d5f43bd4c6a7eecff7bc0c4ff1be9152b33cb1e0 and > e3f15104cf0386a0e0a782037e8c0323629be749. Well, I think that some existing code should be replaced with code that uses snd_pcm_rewindable, once we have confirmed that snd_pcm_rewindable is actually working in the ALSA layer. The code that needs updating probably includes the code added by commit cb55b00ccd25d ("alsa: disable rewinds when using ALSA plugins"). >>> Second answer: >>> >>> Well, it is not better. In timestamp-based scheduling mode, we can >>> dynamically adjust latency. The limitation is that, without rewinds, our >>> decisions to reduce latency (e.g. due to a new client) would apply too >>> late. But even with this limitation, it means that we can try to keep as >>> low latency as it actually works on the given hardware (similar to the >>> current watermark logic), disregarding any client-specified latency. >>> >>> The problem is that, if one wants to use timestamp-based scheduling >>> without rewinds, one needs to decouple the current watermark logic, the >>> buffer size choice logic, and the "don't use latency lower than >>> requested by any client" logic, because the later only makes sense when >>> rewinds are possible. >> >> I think we can still dynamically switch latency even without rewinds. >> It'll just take slightly longer to start new streams. >> >> Anyway, here's another idea: >> >> During PulseAudio's first five seconds, all streams are running as some >> kind of "startup test". How about we use that to probe the rewind too? >> And if snd_pcm_rewindable says we can't rewind even with a full buffer, >> then we choose a medium latency as our highest latency, e g 150 - 200 >> ms, instead of the 2 seconds? > > I will think a bit more about this. In fact, we could use a special > probe stream at startup for this, write the full buffer and immediately > attempt to rewind it, without wasting much time. > > Alternatively, we can use snd_pcm_forwardable() with an empty buffer > (and then rewinding to undo the effect), wasting no time at all, because > forwards and rewinds are related. Do you also consider this a valid > solution? I'm not sure what just moving the pointer forward without writing anything actually means, but it seems like the ALSA devices allowing forwarding and those allowing rewinding could be different. -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
