Re: [AD] [BUG?] X11 Windowed driver delay...

[Eric Botcazou <ebotcazou@xxxxxxxxxx>]

> Heh, I can imagine that :P Since the alsa5 and alsa9 code was splitted,
> I had no more excuse to not look at it - and I finally got it to work :)

Well done.  Great!  WUNDERBAR! :-)

> I'm still wondering though if a simple threaded version wouldn't be a
> better idea. Is anyone still needing the unix-signals version of
> Allegro? Not even AllegGL works with that version, so I guess ALSA
> doesn't have to either.

We need to keep the signals version until Allegro w/threads is thread-safe.

> Patch is attached.

I can't really comment on the code, only on its form:


@@ -10,7 +10,8 @@
  *
  *      ALSA 0.9 sound driver.
  *
- *      By Thomas Fjellstrom.
+ *      Initial version by Thomas Fjellstrom.
+ *      Fixed by Elias Pschernig.
  *
  *      See readme.txt for copyright information.
  */

I'd prefer that you leave the "By..." untouched, this would be more 
consistent with other files.  You can add "Extensively modified by..." or 
"General overhaul by..." or something equivalent.


@@ -164,9 +170,10 @@ static int xrun_recovery(snd_pcm_t *hand
 	 fprintf(stderr, "Can't recovery from underrun, prepare failed: %s\n", 
snd_strerror(err));
       return 0;
    }
-   else if (err == -ESTRPIPE) {
+   /* TODO: Can't wait here like that - we are inside an 'interrupt' after 
all. */
+   /*else if (err == -ESTRPIPE) {
       while ((err = snd_pcm_resume(pcm_handle)) == -EAGAIN)
-	 sleep(1);  /* wait until the suspend flag is released */
+	 sleep(1);  / * wait until the suspend flag is released * /
 
       if (err < 0) {
 	 err = snd_pcm_prepare(pcm_handle);
@@ -174,37 +181,76 @@ static int xrun_recovery(snd_pcm_t *hand
 	    fprintf(stderr, "Can't recovery from suspend, prepare failed: %s\n", 
snd_strerror(err));
       }
       return 0;
-   }
+   }*/
 
    return err;
 }

Glurp! :-)  Use #if 0/#endif to comment out a block of code.


 static void alsa_update(int threaded)
-{
-   int i, ret = 0;
+{ 
+   unsigned short revents;

There is a superfluous space after the brace in your version :-)

OK for mainline with these changes.  I'll not upload the release until Monday 
morning so don't hesitate to post corrective patches until then.  And we 
could delay the release a bit if necessary.


> Regarding Allegro 4.1.13 - if this gets included despite the freeze, we
> should still leave it at low priority I think - since OSS works just as
> good.

Yes, but the main purpose of WIP releases is to test new stuff.  So if ALSA 
and OSS work equally well, I'd say that ALSA should be the default.  Do you 
have any strong objection?


Many thanks again for your work on this ALSA stuff.

-- 
Eric Botcazou