Am 08.04.25 um 14:55 schrieb Christian Schoenebeck:
> On Friday, April 4, 2025 1:34:27 PM CEST BALATON Zoltan wrote:
>> On Fri, 4 Apr 2025, Christian Schoenebeck wrote:
>>> On Monday, March 31, 2025 3:05:24 PM CEST BALATON Zoltan wrote:
>>>> On Sun, 23 Mar 2025, Christian Schoenebeck wrote:
>>>>> On Sunday, March 16, 2025 1:20:46 AM CET BALATON Zoltan wrote:
>>>>>> Quoting Volker Rümelin: "try-poll=on tells the ALSA backend to try to
>>>>>> use an event loop instead of the audio timer. This works most of the
>>>>>> time. But the poll event handler in the ALSA backend has a bug. For
>>>>>> example, if the guest can't provide enough audio frames in time, the
>>>>>> ALSA buffer is only partly full and the event handler will be called
>>>>>> again and again on every iteration of the main loop. This increases
>>>>>> the processor load and the guest has less processor time to provide
>>>>>> new audio frames in time. I have two examples where a guest can't
>>>>>> recover from this situation and the guest seems to hang."
>>>>>>
>>>>>> One reproducer I've found is booting MorphOS demo iso on
>>>>>> qemu-system-ppc -machine pegasos2 -audio alsa which should play a
>>>>>> startup sound but instead it freezes. Even when it does not hang it
>>>>>> plays choppy sound. Volker suggested using command line to set
>>>>>> try-poll=off saying: "The try-poll=off arguments are typically
>>>>>> necessary, because the alsa backend has a design issue with
>>>>>> try-poll=on. If the guest can't provide enough audio frames, it's
>>>>>> really unhelpful to ask for new audio frames on every main loop
>>>>>> iteration until the guest can provide enough audio frames. Timer based
>>>>>> playback doesn't have that problem."
>>>>>>
>>>>>> But users cannot easily find this option and having a non-working
>>>>>> default is really unhelpful so to make life easier just set it to
>>>>>> false by default which works until the issue with the alsa backend can
>>>>>> be fixed.
>>>>>>
>>>>>> Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
>>>>>> ---
>>>>>> This fixes my issue but if somebody has a better fix I'm open to that
>>>>>> too.
>>>>>>
>>>>>> audio/alsaaudio.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
>>>>>> index cacae1ea59..9b6c01c0ef 100644
>>>>>> --- a/audio/alsaaudio.c
>>>>>> +++ b/audio/alsaaudio.c
>>>>>> @@ -899,7 +899,7 @@ static void alsa_enable_in(HWVoiceIn *hw, bool
>>>>>> enable)
>>>>>> static void alsa_init_per_direction(AudiodevAlsaPerDirectionOptions
>>>>>> *apdo)
>>>>>> {
>>>>>> if (!apdo->has_try_poll) {
>>>>>> - apdo->try_poll = true;
>>>>>> + apdo->try_poll = false;
>>>>>> apdo->has_try_poll = true;
>>>>>> }
>>>>>> }
>>>>>>
>>>>> Correct me if I am wrong, but AFAICS if polling is not used then no state
>>>>> changes would be handled, no? At least I don't see any snd_pcm_state()
>>>>> call
>>>>> outside of alsa_poll_handler().
>>>> I have no idea but this fixes the problem (and does the same that can be
>>>> also done from command line but nobody can find that command line option)
>>>> so unless somebody has a better idea could this be merged as a fix for
>>>> now?
>>> Well, I understand that if fixes the misbehaviour you encountered. But how
>>> helpful would it be if it then breaks behaviour for other people instead?
>> What behaviour would it break and how?
> There are only a bunch of ALSA states handled right now in the QEMU Alsa
> driver (see alsa_poll_handler()):
>
> state = snd_pcm_state (hlp->handle);
> switch (state) {
> case SND_PCM_STATE_SETUP:
> alsa_recover (hlp->handle);
> break;
>
> case SND_PCM_STATE_XRUN:
> alsa_recover (hlp->handle);
> break;
>
> case SND_PCM_STATE_SUSPENDED:
> alsa_resume (hlp->handle);
> break;
>
> case SND_PCM_STATE_PREPARED:
> audio_run(hlp->s, "alsa run (prepared)");
> break;
>
> case SND_PCM_STATE_RUNNING:
> audio_run(hlp->s, "alsa run (running)");
> break;
>
> For instance in poll mode it recovers in case of an xrun, which happens on
> audio output if the audio output data was not delivered by the application in
> time.
>
> The other case is when the system was suspended (standby). It should also
> recover the audio session here.
Hi Christian,
I think the timer based mode works fine. snd_pcm_readi() and
snd_pcm_writei() return -EPIPE in case of a xrun and -ESTRPIPE if a
suspend event occurred. Both cases are handled in alsa_write(). The
-ESTRPIPE case is missing in alsa_read(), which may be a mistake. I
don't think it's possible alsa_read() and alsa_write() get called if the
ALSA system is in state SND_PCM_STATE_SETUP.
The write_loop() example code at
https://www.alsa-project.org/alsa-doc/alsa-lib/examples.html in file
test/pcm.c also doesn't use the snd_pcm_state() function. Please have a
look at write_loop() in test/pcm.c and compare it with
write_and_poll_loop() in the same file.
With best regards
Volker
> Now I haven't tested whether these would work in callback mode right now, but
> looking at the code suggests that they might not.
>
>>> I think it would be better to add a 2nd patch that would handle state
>>> changes
>>> in callback mode. That would satisfy both groups of people. AFAICS
>>> snd_pcm_state() can be called both in polling mode and callback mode.
>> I can't do that because I don't quite know neither alsa nor audio in QEMU
>> so I have no idea what to do. Can you give more clues?
> Well, as a starting point you might try whether these cases described above
> would still work in callback mode. Maybe it is even working, who knows.
>
> /Christian
>
>>> Just my 2 cents. Of course it's up to Gerd to decide.
>> If you know it would break something I agree it should be fixed and I can
>> try but I could not reproduce any breakage and most people likely use
>> other audio backends so unlikely to encounter problems with alsa.
>>
>> Regards,
>> BALATON Zoltan
>
