Re: [Qemu-devel] [PATCH] Initial implementation of a mpeg1 layer2 streaming audio driver.

[malc]

On Sun, 7 Nov 2010, Fran?ois Revol wrote:

> 
> Le 7 nov. 2010 ? 19:09, malc a ?crit :
> 
> > On Sun, 7 Nov 2010, Fran?ois Revol wrote:
> > 
> > Please CC audio related stuff to audio maintainer.
> 
> And that'd be you according to MAINTAINERS ?
> 
> >> +static const char http_header[] = "HTTP/1.1 200 OK\r\nServer: 
> >> QEMU\r\nContent-Type: audio/mpeg\r\n\r\n";
> > 
> > Line is too long.
> 
> Ok I'll break at 80col.
> 
> >> +if (twolame->fd > -1)
> >> +return;
> > 
> > Style.
> 
> That is ?

Braces around statements.

> >> +
> >> +int csock = qemu_accept(twolame->lsock, (struct sockaddr *)&addr, 
> >> &addrlen);
> > 
> > C99 intermixed declartion and initialization is not allowed.
> 
> This line I copied form ui/vnc.c which does violate C89 too btw...

I do not maintain ui/vnc.c.

> 
> >> +
> >> +again:
> >> +if (twolame->fd > -1) {
> >> +written = write (twolame->fd, twolame->mpg_buf, 
> >> converted);
> >> +if (written == -1) {
> >> +if (errno == EPIPE) {
> >> +dolog ("Lost peer\n");
> >> +close (twolame->fd);
> >> +twolame->fd = -1;
> >> +goto again;
> > 
> > This goto is obfuscated.
> 
> Not much more than in esdaudio.c
> 

Yes much more, esdaudio doesn't close the descriptor before jumping.

> 
> >> +}
> >> +obt_as.endianness = AUDIO_HOST_ENDIANNESS;
> >> +
> >> +audio_pcm_init_info (&hw->info, &obt_as);
> >> +
> >> +twolame_set_mode(twolame->options, (as->nchannels == 2) ? 
> >> TWOLAME_STEREO : TWOLAME_MONO);
> >> +twolame_set_num_channels(twolame->options, as->nchannels);
> >> +twolame_set_in_samplerate(twolame->options, as->freq);
> >> +twolame_set_out_samplerate(twolame->options, as->freq);
> >> +twolame_set_bitrate(twolame->options, 160); //XXX:conf.
> >> +
> >> +if (twolame_init_params(twolame->options)) {
> >> +dolog ("Could not set twolame options\n");
> >> +return -1;
> >> +}
> > 
> > Inconsistent space before opening paren.
> 
> Sorry, used to the Haiku style without space before but it seemed to be 
> different around.
> 
> 
> >> +twolame->mpg_buf = audio_calloc (AUDIO_FUNC, hw->samples, 1 << 
> >> hw->info.shift);
> >> +if (!twolame->mpg_buf) {
> > 
> > pcm_buf is not freed.
> > 
> >> +
> >> +// fail1:
> > 
> > Do not use C99 style comments.
> 
> Oh that's leftover from copied error handling, which isn't correct anyway.
> 
> Fran?ois.
> 

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Initial implementation of a mpeg1 layer2 streaming audio driver.

[François Revol]

Le 7 nov. 2010 à 19:09, malc a écrit :

> On Sun, 7 Nov 2010, Fran?ois Revol wrote:
> 
> Please CC audio related stuff to audio maintainer.

And that'd be you according to MAINTAINERS ?

>> +static const char http_header[] = "HTTP/1.1 200 OK\r\nServer: 
>> QEMU\r\nContent-Type: audio/mpeg\r\n\r\n";
> 
> Line is too long.

Ok I'll break at 80col.

>> +if (twolame->fd > -1)
>> +return;
> 
> Style.

That is ?

>> +
>> +int csock = qemu_accept(twolame->lsock, (struct sockaddr *)&addr, 
>> &addrlen);
> 
> C99 intermixed declartion and initialization is not allowed.

This line I copied form ui/vnc.c which does violate C89 too btw...

>> +
>> +again:
>> +if (twolame->fd > -1) {
>> +written = write (twolame->fd, twolame->mpg_buf, converted);
>> +if (written == -1) {
>> +if (errno == EPIPE) {
>> +dolog ("Lost peer\n");
>> +close (twolame->fd);
>> +twolame->fd = -1;
>> +goto again;
> 
> This goto is obfuscated.

Not much more than in esdaudio.c



>> +}
>> +obt_as.endianness = AUDIO_HOST_ENDIANNESS;
>> +
>> +audio_pcm_init_info (&hw->info, &obt_as);
>> +
>> +twolame_set_mode(twolame->options, (as->nchannels == 2) ? 
>> TWOLAME_STEREO : TWOLAME_MONO);
>> +twolame_set_num_channels(twolame->options, as->nchannels);
>> +twolame_set_in_samplerate(twolame->options, as->freq);
>> +twolame_set_out_samplerate(twolame->options, as->freq);
>> +twolame_set_bitrate(twolame->options, 160); //XXX:conf.
>> +
>> +if (twolame_init_params(twolame->options)) {
>> +dolog ("Could not set twolame options\n");
>> +return -1;
>> +}
> 
> Inconsistent space before opening paren.

Sorry, used to the Haiku style without space before but it seemed to be 
different around.


>> +twolame->mpg_buf = audio_calloc (AUDIO_FUNC, hw->samples, 1 << 
>> hw->info.shift);
>> +if (!twolame->mpg_buf) {
> 
> pcm_buf is not freed.
> 
>> +
>> +// fail1:
> 
> Do not use C99 style comments.

Oh that's leftover from copied error handling, which isn't correct anyway.

François.

Re: [Qemu-devel] [PATCH] Initial implementation of a mpeg1 layer2 streaming audio driver.

[malc]

On Sun, 7 Nov 2010, Fran?ois Revol wrote:

Please CC audio related stuff to audio maintainer.

> Initial implementation of a mpeg1 layer2 streaming audio driver.
> It is based on the twolame library .
> It allows one to listen to the audio produced by a VM from an mp3 http 
> streaming client.
> I just noticed esdaudio.c which I used as template on was under BSD licence, 
> which is fine by me for this one as well.
> For now it almost works with a Haiku guest (with HDA at 22050Hz and the 
> WAKEEN patch I just sent), except with a 1min delay and missing frames, so 
> it's possible buffers get queued up somewhere.
> 
> 
> From 759ce26b14b7c9c5a24fba43b01cfb5d335086be Mon Sep 17 00:00:00 2001
> 
> Initial implementation of a mpeg1 layer2 streaming audio driver.
> It is based on the twolame library .
> Added a check for libtwolame to configure.
> 
> 
> Signed-off-by: Fran?ois Revol 
> ---
>  Makefile.objs|1 +
>  audio/audio.c|3 +
>  audio/audio_int.h|1 +
>  audio/twolameaudio.c |  393 
> ++
>  configure|   20 +++
>  5 files changed, 418 insertions(+), 0 deletions(-)
>  create mode 100644 audio/twolameaudio.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index faf485e..370d59a 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -109,6 +109,7 @@ audio-obj-$(CONFIG_FMOD) += fmodaudio.o
>  audio-obj-$(CONFIG_ESD) += esdaudio.o
>  audio-obj-$(CONFIG_PA) += paaudio.o
>  audio-obj-$(CONFIG_WINWAVE) += winwaveaudio.o
> +audio-obj-$(CONFIG_TWOLAME) += twolameaudio.o
>  audio-obj-$(CONFIG_AUDIO_PT_INT) += audio_pt_int.o
>  audio-obj-$(CONFIG_AUDIO_WIN_INT) += audio_win_int.o
>  audio-obj-y += wavcapture.o
> diff --git a/audio/audio.c b/audio/audio.c
> index ad51077..0c2c304 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -46,6 +46,9 @@
>  static struct audio_driver *drvtab[] = {
>  CONFIG_AUDIO_DRIVERS
>  &no_audio_driver,
> +#ifdef CONFIG_TWOLAME
> +&twolame_audio_driver,
> +#endif
>  &wav_audio_driver
>  };
>  
> diff --git a/audio/audio_int.h b/audio/audio_int.h
> index d8560b6..337188b 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -210,6 +210,7 @@ extern struct audio_driver dsound_audio_driver;
>  extern struct audio_driver esd_audio_driver;
>  extern struct audio_driver pa_audio_driver;
>  extern struct audio_driver winwave_audio_driver;
> +extern struct audio_driver twolame_audio_driver;
>  extern struct mixeng_volume nominal_volume;
>  
>  void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings 
> *as);
> diff --git a/audio/twolameaudio.c b/audio/twolameaudio.c
> new file mode 100644
> index 000..e121a91
> --- /dev/null
> +++ b/audio/twolameaudio.c
> @@ -0,0 +1,393 @@
> +/*
> + * QEMU twolame streaming audio driver
> + *
> + * Copyright (c) 2010 Fran?ois Revol 
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#include "config-host.h"
> +#include "qemu-common.h"
> +#include "qemu-char.h"
> +#include "qemu_socket.h"
> +#include "audio.h"
> +
> +#define AUDIO_CAP "twolame"
> +#include "audio_int.h"
> +#include "audio_pt_int.h"
> +
> +#include 
> +
> +typedef struct {
> +HWVoiceOut hw;
> +int done;
> +int live;
> +int decr;
> +int rpos;
> +void *pcm_buf;
> +void *mpg_buf;
> +int lsock;
> +int fd;
> +struct audio_pt pt;
> +twolame_options *options;
> +} LAMEVoiceOut;
> +
> +static struct {
> +int samples;
> +int divisor;
> +int port;
> +int rate;
> +} conf = {
> +.samples = 1024,
> +.divisor = 2,
> +.port = 8080,
> +.rate = 160
> +};
> +
> +static const char http_header[] = "HTTP/1.1 200 OK\r\nServer: 
> QEMU\r\nContent-Type: audio/mpeg\r\n\r\n";

Line is too long.

> +
> +static void GCC_FMT_ATTR (2, 3) qtwolame_logerr (int err, const char *fmt, 
> ..

[Qemu-devel] [PATCH] Initial implementation of a mpeg1 layer2 streaming audio driver.

[François Revol]

Initial implementation of a mpeg1 layer2 streaming audio driver.
It is based on the twolame library .
It allows one to listen to the audio produced by a VM from an mp3 http 
streaming client.
I just noticed esdaudio.c which I used as template on was under BSD licence, 
which is fine by me for this one as well.
For now it almost works with a Haiku guest (with HDA at 22050Hz and the WAKEEN 
patch I just sent), except with a 1min delay and missing frames, so it's 
possible buffers get queued up somewhere.


From 759ce26b14b7c9c5a24fba43b01cfb5d335086be Mon Sep 17 00:00:00 2001

Initial implementation of a mpeg1 layer2 streaming audio driver.
It is based on the twolame library .
Added a check for libtwolame to configure.


Signed-off-by: François Revol 
---
 Makefile.objs|1 +
 audio/audio.c|3 +
 audio/audio_int.h|1 +
 audio/twolameaudio.c |  393 ++
 configure|   20 +++
 5 files changed, 418 insertions(+), 0 deletions(-)
 create mode 100644 audio/twolameaudio.c

diff --git a/Makefile.objs b/Makefile.objs
index faf485e..370d59a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -109,6 +109,7 @@ audio-obj-$(CONFIG_FMOD) += fmodaudio.o
 audio-obj-$(CONFIG_ESD) += esdaudio.o
 audio-obj-$(CONFIG_PA) += paaudio.o
 audio-obj-$(CONFIG_WINWAVE) += winwaveaudio.o
+audio-obj-$(CONFIG_TWOLAME) += twolameaudio.o
 audio-obj-$(CONFIG_AUDIO_PT_INT) += audio_pt_int.o
 audio-obj-$(CONFIG_AUDIO_WIN_INT) += audio_win_int.o
 audio-obj-y += wavcapture.o
diff --git a/audio/audio.c b/audio/audio.c
index ad51077..0c2c304 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -46,6 +46,9 @@
 static struct audio_driver *drvtab[] = {
 CONFIG_AUDIO_DRIVERS
 &no_audio_driver,
+#ifdef CONFIG_TWOLAME
+&twolame_audio_driver,
+#endif
 &wav_audio_driver
 };
 
diff --git a/audio/audio_int.h b/audio/audio_int.h
index d8560b6..337188b 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -210,6 +210,7 @@ extern struct audio_driver dsound_audio_driver;
 extern struct audio_driver esd_audio_driver;
 extern struct audio_driver pa_audio_driver;
 extern struct audio_driver winwave_audio_driver;
+extern struct audio_driver twolame_audio_driver;
 extern struct mixeng_volume nominal_volume;
 
 void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as);
diff --git a/audio/twolameaudio.c b/audio/twolameaudio.c
new file mode 100644
index 000..e121a91
--- /dev/null
+++ b/audio/twolameaudio.c
@@ -0,0 +1,393 @@
+/*
+ * QEMU twolame streaming audio driver
+ *
+ * Copyright (c) 2010 François Revol 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "config-host.h"
+#include "qemu-common.h"
+#include "qemu-char.h"
+#include "qemu_socket.h"
+#include "audio.h"
+
+#define AUDIO_CAP "twolame"
+#include "audio_int.h"
+#include "audio_pt_int.h"
+
+#include 
+
+typedef struct {
+HWVoiceOut hw;
+int done;
+int live;
+int decr;
+int rpos;
+void *pcm_buf;
+void *mpg_buf;
+int lsock;
+int fd;
+struct audio_pt pt;
+twolame_options *options;
+} LAMEVoiceOut;
+
+static struct {
+int samples;
+int divisor;
+int port;
+int rate;
+} conf = {
+.samples = 1024,
+.divisor = 2,
+.port = 8080,
+.rate = 160
+};
+
+static const char http_header[] = "HTTP/1.1 200 OK\r\nServer: 
QEMU\r\nContent-Type: audio/mpeg\r\n\r\n";
+
+static void GCC_FMT_ATTR (2, 3) qtwolame_logerr (int err, const char *fmt, ...)
+{
+va_list ap;
+
+va_start (ap, fmt);
+AUD_vlog (AUDIO_CAP, fmt, ap);
+va_end (ap);
+
+AUD_log (AUDIO_CAP, "Reason: %s\n", strerror (err));
+}
+
+static void qtwolame_listen_read(void *opaque)
+{
+LAMEVoiceOut *twolame = opaque;
+struct sockaddr_in addr;
+socklen_t addrlen = sizeof(addr);
+
+if (twolame->fd > -1)
+return;
+
+int csock = qemu_accept(twola