malc wrote: > On Mon, 14 Apr 2008, Jan Kiszka wrote: > >> malc wrote: >>> On Sun, 13 Apr 2008, Jan Kiszka wrote: > > [..snip..] > >>>> +static void sound_fill_mixer_buffer(mv88w8618_sound_state *s, >>>> unsigned int length) >>>> +{ >>>> + unsigned int pos; >>>> + double val; >>>> + >>>> + if (s->mute) { >>>> + memset(s->mixer_buffer, 0, length); >>> >>> Zero is not correct "mute" value for signed formats. >> >> Hmm, maybe it's still too early for me - but what else?? > > Well.. 0x80 for S8 and 0x8000 for S16 (audio_pcm_info_clear_buf in > audio.c) void audio_pcm_info_clear_buf (struct audio_pcm_info *info, ... { ... if (info->sign) { memset (buf, 0x00, len << info->shift); } >>> >>>> + return; >>>> + } >>>> + >>>> + if (s->playback_mode & 1) >>>> + for (pos = 0; pos < length; pos += 2) { >>>> + val = *(int16_t *)(s->target_buffer + s->play_pos + pos); >>>> + val = val * pow(10.0, s->volume/20.0); >>>> + *(int16_t *)(s->mixer_buffer + pos) = val; >>>> + } >>> >>> On the surface it's sounds like endianess problems are ahead. >> >> Oh, yes. >> >>> >>>> + else >>>> + for (pos = 0; pos < length; pos += 1) { >>>> + val = *(int8_t *)(s->target_buffer + s->play_pos + pos); >>>> + val = val * pow(10.0, s->volume/20.0); >>>> + *(int8_t *)(s->mixer_buffer + pos) = val; >>>> + } >>>> +} >>> >>> There's actually a generic framework for volume manipulation in QEMUs >>> audio, unfortunatelly it's unconditionally disabled (define NOVOL in >>> audio/mixeng.c) for the reasons i can't recall now. >> >> Yeah, looked around a bit before hacking those loops but indeed found >> nothing ready to use. > > It's just a question of adding(and using) AUD_set_volume[_in/out], > which will set vol field of respective soft voice and removing > uncoditional #define NOVOL from mixeng.c. Well, I was (and still am) deterred by the plain fact that there are some dead AUD_set_volume spots in ac97.c. If it is that simple, why do we still lack an implementation? My feeling is that you are far deeper into this than I am at this point. So maybe you would like me to test some AUD_set_volume-providing patch of yours? :-> >>> >>> Also adlib emulation is only conditionally available due to >>> usage of floating point arithmetics, maybe rules have changed now >>> though, but "fixing" this particular issue in this particular case >>> seems easy enough. >> >> Sorry, didn't get what you mean here. > > Adlib is not built by default, requires explicit --enable-adlib switch > to configure, because fmopl.c uses FPU and Fabrice didn't like the idea. > Fixing it here is just a matter of switching to fixed point. > > Then again if volume manipulation in main audio proper are brought up > to date this all can be scrapped and it will also "magically" solve the > endianness issue. I do agree that such stuff should be solved generically, but for now adding a simple le16_to_cpu does not compare to fixing QEMU's soft-volume management for me (given limited time). >>> >>>> +static void sound_callback(void *opaque, int free) >>>> +{ >>>> + mv88w8618_sound_state *s = opaque; >>>> + uint32_t old_status = s->status; >>>> + int64_t now = qemu_get_clock(rt_clock); >>>> + unsigned int n; >>>> + >>>> + if (now - s->last_callback < (1000 * s->threshold/2) / (44100*2)) >>>> + return; >>>> + s->last_callback = now; >>> >>> Here two clocks are being used for things - the audio one which >>> influences >>> `free' paramter and rt_clock, even if there are reasons why free >>> could not >>> be used choice of rt_clock over vm_clock seems wrong. >> >> Yes, using the monotonic vm_clock is better. Will change. > > Why is it needed at all? `free' already supplies you with an audio clock > source. The callback mechanism fires far more often than the emulated hardware expects (and can handle), I need throttling. The sad truth is that my original version was still off my an order of magnitude, causing overload for the guest while decoding obviously more complex 128kbit-WMA streams. Now it runs smoothly. :) Maybe there is a way to customize the callback rate, I haven't looked that close, but I'm open for suggestions. Thanks again, Jan