From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JlSmB-0004YV-6n for qemu-devel@nongnu.org; Mon, 14 Apr 2008 13:47:31 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JlSm9-0004Xm-8S for qemu-devel@nongnu.org; Mon, 14 Apr 2008 13:47:30 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JlSm9-0004Xh-0A for qemu-devel@nongnu.org; Mon, 14 Apr 2008 13:47:29 -0400 Received: from fmmailgate01.web.de ([217.72.192.221]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1JlSm7-0004Fq-TX for qemu-devel@nongnu.org; Mon, 14 Apr 2008 13:47:29 -0400 Received: from smtp08.web.de (fmsmtp08.dlan.cinetic.de [172.20.5.216]) by fmmailgate01.web.de (Postfix) with ESMTP id 21323DB4831B for ; Mon, 14 Apr 2008 19:47:25 +0200 (CEST) Received: from [88.64.24.193] (helo=[192.168.1.198]) by smtp08.web.de with asmtp (TLSv1:AES256-SHA:256) (WEB.DE 4.109 #226) id 1JlSm4-00086F-00 for qemu-devel@nongnu.org; Mon, 14 Apr 2008 19:47:24 +0200 Message-ID: <480398A8.5000404@web.de> Date: Mon, 14 Apr 2008 19:47:20 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <4801DC59.1010403@web.de> <480300D2.2060400@web.de> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig5E58B18FF4A6E213A4A2DCAB" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [RFC][PATCH 4/4] Add support for Marvell 88w8618 / MusicPal Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig5E58B18FF4A6E213A4A2DCAB Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable malc wrote: > On Mon, 14 Apr 2008, Jan Kiszka wrote: >=20 >> malc wrote: >>> On Sun, 13 Apr 2008, Jan Kiszka wrote: >=20 > [..snip..] >=20 >>>> +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?? >=20 > 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, ... { =2E.. if (info->sign) { memset (buf, 0x00, len << info->shift); } >>> >>>> + return; >>>> + } >>>> + >>>> + if (s->playback_mode & 1) >>>> + for (pos =3D 0; pos < length; pos +=3D 2) { >>>> + val =3D *(int16_t *)(s->target_buffer + s->play_pos + p= os); >>>> + val =3D val * pow(10.0, s->volume/20.0); >>>> + *(int16_t *)(s->mixer_buffer + pos) =3D val; >>>> + } >>> >>> On the surface it's sounds like endianess problems are ahead. >> >> Oh, yes. >> >>> >>>> + else >>>> + for (pos =3D 0; pos < length; pos +=3D 1) { >>>> + val =3D *(int8_t *)(s->target_buffer + s->play_pos + po= s); >>>> + val =3D val * pow(10.0, s->volume/20.0); >>>> + *(int8_t *)(s->mixer_buffer + pos) =3D 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. >=20 > 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. >=20 > 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= =2E > Fixing it here is just a matter of switching to fixed point. >=20 > 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 =3D opaque; >>>> + uint32_t old_status =3D s->status; >>>> + int64_t now =3D qemu_get_clock(rt_clock); >>>> + unsigned int n; >>>> + >>>> + if (now - s->last_callback < (1000 * s->threshold/2) / (44100*2= )) >>>> + return; >>>> + s->last_callback =3D 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. >=20 > Why is it needed at all? `free' already supplies you with an audio cloc= k > 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 --------------enig5E58B18FF4A6E213A4A2DCAB Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4-svn0 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iD8DBQFIA5isniDOoMHTA+kRAoe7AJ9HgK9eI2+4bp3FotPzCXoJ97pBGACdFCNl yp5NmxGihw+dIm2l3LLBpv4= =M2v/ -----END PGP SIGNATURE----- --------------enig5E58B18FF4A6E213A4A2DCAB--