[Qemu-devel] Re: [PATCH] alsaaudio: increase default buffer sizes

[Jan Kiszka]

Jan Kiszka wrote:
> malc wrote:
>> On Sun, 4 May 2008, Jan Kiszka wrote:
>>
>>> malc wrote:
>>>> On Fri, 2 May 2008, Jan Kiszka wrote:
>>>>
>>>>> malc wrote:
>>>>>> On Fri, 2 May 2008, Jan Kiszka wrote:
>>>>>>
>>>>>>> Sound though the ALSA driver is skipping here unless I increase the
>>>>>>> buffer size. OSS seems to use 16K as well, and 1K was obviously to
>>>>>>> small
>>>>>>> for recording anyway.
>>>>>>>
>>>>>>> [ PS: Can someone explain to me why I also have to override the
>>>>>>> DAC/ADC_FIXED_FREQ to 48000 to make ALSA work? Suboptimal... ]
>>>>>> How exactly it doesn't work if you don't override it? Do you get any
>>>>>> messages prefixed with "alsa:"? As for the defaults they were set to
>>>>>> 1024/256 because that's what was needed to make it sound on par
>>>>>> with OSS
>>>>>> on the two machines i had at the time.
>> [..snip..]
>>
>>>>
>>>> Could you give it a whirl please?
>>> No difference - except that it now complains about the 16384 byte buffer
>>> size.
>> Okay more of the same...
>>
>> diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
>> index 43cfa25..59c091d 100644
>> --- a/audio/alsaaudio.c
>> +++ b/audio/alsaaudio.c
>> @@ -428,15 +428,15 @@ static int alsa_open (int in, struct
>> alsa_params_req *req,
>>                      }
>>                  }
>>
>> -                err = snd_pcm_hw_params_set_period_size (
>> +                err = snd_pcm_hw_params_set_period_size_near (
>>                      handle,
>>                      hw_params,
>> -                    period_size,
>> +                    &period_size,
>>                      0
>>                      );
>>                  if (err < 0) {
>>                      alsa_logerr2 (err, typ, "Failed to set period size
>> %d\n",
>> -                                  req->period_size);
>> +                                  period_size);
>>                      goto err;
>>                  }
>>              }
>> @@ -466,14 +466,14 @@ static int alsa_open (int in, struct
>> alsa_params_req *req,
>>                  }
>>              }
>>
>> -            err = snd_pcm_hw_params_set_buffer_size (
>> +            err = snd_pcm_hw_params_set_buffer_size_near (
>>                  handle,
>>                  hw_params,
>> -                buffer_size
>> +                &buffer_size
>>                  );
>>              if (err < 0) {
>>                  alsa_logerr2 (err, typ, "Failed to set buffer size %d\n",
>> -                              req->buffer_size);
>> +                              buffer_size);
>>                  goto err;
>>              }
>>          }
> 
> Yeah, great, that was the key! Find below the version that works for me
> (on 64 bit 8) ). It even obsoletes my buffer size patch. Please merge!

Declaring the buffer size tweak obsolete was too quick. I still need
more under certain guest load, but now I'm already fine with
DEFAULT_BUFFER_SIZE=4096. Or all in one:

Index: qemu/audio/alsaaudio.c
===================================================================
--- qemu/audio/alsaaudio.c      (Revision 4332)
+++ qemu/audio/alsaaudio.c      (Arbeitskopie)
@@ -58,7 +58,7 @@ static struct {
     int period_size_out_overridden;
     int verbose;
 } conf = {
-#define DEFAULT_BUFFER_SIZE 1024
+#define DEFAULT_BUFFER_SIZE 4096
 #define DEFAULT_PERIOD_SIZE 256
 #ifdef HIGH_LATENCY
     .size_in_usec_in = 1,
@@ -72,8 +72,8 @@ static struct {
     .buffer_size_out = 400000,
     .period_size_out = 400000 / 4,
 #else
-    .buffer_size_in = DEFAULT_BUFFER_SIZE * 4,
-    .period_size_in = DEFAULT_PERIOD_SIZE * 4,
+    .buffer_size_in = DEFAULT_BUFFER_SIZE,
+    .period_size_in = DEFAULT_PERIOD_SIZE,
     .buffer_size_out = DEFAULT_BUFFER_SIZE,
     .period_size_out = DEFAULT_PERIOD_SIZE,
     .buffer_size_in_overridden = 0,
@@ -396,7 +396,7 @@ static int alsa_open (int in, struct als
         }
         else {
             int dir;
-            snd_pcm_uframes_t minval;
+            snd_pcm_uframes_t minval, val;
 
             if (period_size) {
                 minval = period_size;
@@ -428,15 +428,16 @@ static int alsa_open (int in, struct als
                     }
                 }
 
-                err = snd_pcm_hw_params_set_period_size (
+                val = period_size;
+                err = snd_pcm_hw_params_set_period_size_near (
                     handle,
                     hw_params,
-                    period_size,
+                    &val,
                     0
                     );
                 if (err < 0) {
                     alsa_logerr2 (err, typ, "Failed to set period size %d\n",
-                                  req->period_size);
+                                  period_size);
                     goto err;
                 }
             }
@@ -466,14 +467,15 @@ static int alsa_open (int in, struct als
                 }
             }
 
-            err = snd_pcm_hw_params_set_buffer_size (
+            val = buffer_size;
+            err = snd_pcm_hw_params_set_buffer_size_near (
                 handle,
                 hw_params,
-                buffer_size
+                &val
                 );
             if (err < 0) {
                 alsa_logerr2 (err, typ, "Failed to set buffer size %d\n",
-                              req->buffer_size);
+                              buffer_size);
                 goto err;
             }
         }

signature.asc
Description: OpenPGP digital signature