[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [RFC][PATCH 4/4] Add support for Marvell 88w8618 / Musi
From: |
Jan Kiszka |
Subject: |
[Qemu-devel] Re: [RFC][PATCH 4/4] Add support for Marvell 88w8618 / MusicPal |
Date: |
Mon, 14 Apr 2008 19:47:20 +0200 |
User-agent: |
Thunderbird 2.0.0.12 (X11/20080226) |
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
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [RFC][PATCH 4/4] Add support for Marvell 88w8618 / MusicPal, Jan Kiszka, 2008/04/13
- Re: [Qemu-devel] [RFC][PATCH 4/4] Add support for Marvell 88w8618 / MusicPal, malc, 2008/04/13
- [Qemu-devel] Re: [RFC][PATCH 4/4] Add support for Marvell 88w8618 / MusicPal, Jan Kiszka, 2008/04/14
- Re: [Qemu-devel] Re: [RFC][PATCH 4/4] Add support for Marvell 88w8618 / MusicPal, malc, 2008/04/14
- [Qemu-devel] Re: [RFC][PATCH 4/4] Add support for Marvell 88w8618 / MusicPal,
Jan Kiszka <=
- Re: [Qemu-devel] Re: [RFC][PATCH 4/4] Add support for Marvell 88w8618 / MusicPal, malc, 2008/04/15
- [Qemu-devel] Re: [RFC][PATCH 4/4] Add support for Marvell 88w8618 / MusicPal, Jan Kiszka, 2008/04/15
- Re: [Qemu-devel] Re: [RFC][PATCH 4/4] Add support for Marvell 88w8618 / MusicPal, malc, 2008/04/16
- [Qemu-devel] Re: [RFC][PATCH 4/4] Add support for Marvell 88w8618 / MusicPal, Jan Kiszka, 2008/04/17
[Qemu-devel] Re: [RFC][PATCH 4/4] Add support for Marvell 88w8618 / MusicPal, Jan Kiszka, 2008/04/14
Re: [Qemu-devel] [RFC][PATCH 4/4] Add support for Marvell 88w8618 / MusicPal, andrzej zaborowski, 2008/04/16