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 <http://www.twolame.org/>.
> 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 <http://www.twolame.org/>.
> Added a check for libtwolame to configure.
> 
> 
> Signed-off-by: Fran?ois Revol <address@hidden>
> ---
>  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 0000000..e121a91
> --- /dev/null
> +++ b/audio/twolameaudio.c
> @@ -0,0 +1,393 @@
> +/*
> + * QEMU twolame streaming audio driver
> + *
> + * Copyright (c) 2010 Fran?ois Revol <address@hidden>
> + *
> + * 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 <twolame.h>
> +
> +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, 
> ...)
> +{
> +    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;

Style.

> +
> +    int csock = qemu_accept(twolame->lsock, (struct sockaddr *)&addr, 
> &addrlen);

C99 intermixed declartion and initialization is not allowed.

> +    if (csock != -1) {
> +        twolame->fd = csock;
> +        dolog ("Accepted peer\n");
> +        write (twolame->fd, http_header, sizeof(http_header) - 1);

Write returns value which should be checked.

> +    }
> +}
> +
> +/* playback */
> +static void *qtwolame_thread_out (void *arg)
> +{
> +    LAMEVoiceOut *twolame = arg;
> +    HWVoiceOut *hw = &twolame->hw;
> +    int threshold;
> +
> +    threshold = conf.divisor ? hw->samples / conf.divisor : 0;
> +
> +    if (audio_pt_lock (&twolame->pt, AUDIO_FUNC)) {
> +        return NULL;
> +    }
> +
> +    for (;;) {
> +        int decr, to_mix, rpos;
> +
> +        for (;;) {
> +            if (twolame->done) {
> +                goto exit;
> +            }
> +
> +            if (twolame->live > threshold) {
> +                break;
> +            }
> +
> +            if (audio_pt_wait (&twolame->pt, AUDIO_FUNC)) {
> +                goto exit;
> +            }
> +
> +        }
> +
> +        decr = to_mix = twolame->live;
> +        rpos = hw->rpos;
> +
> +        if (audio_pt_unlock (&twolame->pt, AUDIO_FUNC)) {
> +            return NULL;
> +        }
> +
> +        while (to_mix) {
> +            ssize_t converted, written;
> +            int chunk = audio_MIN (to_mix, hw->samples - rpos);
> +            struct st_sample *src = hw->mix_buf + rpos;
> +
> +            hw->clip (twolame->pcm_buf, src, chunk);
> +
> +            if (twolame->fd > -1) {
> +                converted = twolame_encode_buffer_interleaved 
> (twolame->options, twolame->pcm_buf,
> +                    chunk, twolame->mpg_buf, hw->samples << hw->info.shift);
> +                if (converted < 0) {
> +                    qtwolame_logerr (converted, 
> "twolame_encode_buffer_interleaved failed\n");
> +                    return NULL;
> +                }
> +            }
> +
> +        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.

> +                    }
> +                    if (errno == EINTR || errno == EAGAIN) {
> +                        goto again;
> +                    }
> +                    qtwolame_logerr (errno, "write failed\n");
> +                    return NULL;
> +                }
> +            }
> +
> +            rpos = (rpos + chunk) % hw->samples;
> +            to_mix -= chunk;
> +        }
> +
> +        if (audio_pt_lock (&twolame->pt, AUDIO_FUNC)) {
> +            return NULL;
> +        }
> +
> +        twolame->rpos = rpos;
> +        twolame->live -= decr;
> +        twolame->decr += decr;
> +    }
> +
> + exit:
> +    audio_pt_unlock (&twolame->pt, AUDIO_FUNC);
> +    return NULL;
> +}
> +
> +static int qtwolame_run_out (HWVoiceOut *hw, int live)
> +{
> +    int decr;
> +    LAMEVoiceOut *twolame = (LAMEVoiceOut *) hw;
> +
> +    if (audio_pt_lock (&twolame->pt, AUDIO_FUNC)) {
> +        return 0;
> +    }
> +
> +    decr = audio_MIN (live, twolame->decr);
> +    twolame->decr -= decr;
> +    twolame->live = live - decr;
> +    hw->rpos = twolame->rpos;
> +    if (twolame->live > 0) {
> +        audio_pt_unlock_and_signal (&twolame->pt, AUDIO_FUNC);
> +    }
> +    else {
> +        audio_pt_unlock (&twolame->pt, AUDIO_FUNC);
> +    }
> +    return decr;
> +}
> +
> +static int qtwolame_write (SWVoiceOut *sw, void *buf, int len)
> +{
> +    return audio_pcm_sw_write (sw, buf, len);
> +}
> +
> +static int qtwolame_init_out (HWVoiceOut *hw, struct audsettings *as)
> +{
> +    LAMEVoiceOut *twolame = (LAMEVoiceOut *) hw;
> +    struct audsettings obt_as = *as;
> +
> +    twolame->options = twolame_init();
> +    twolame->fd = -1;
> +
> +    switch (as->fmt) {
> +    case AUD_FMT_S8:
> +    case AUD_FMT_U8:
> +        dolog ("Will use 16 instead of 8 bit samples\n");
> +        goto deffmt;
> +
> +    case AUD_FMT_S32:
> +    case AUD_FMT_U32:
> +        dolog ("Will use 16 instead of 32 bit samples\n");
> +
> +    case AUD_FMT_S16:
> +    case AUD_FMT_U16:
> +    deffmt:
> +        obt_as.fmt = AUD_FMT_S16;
> +        break;
> +
> +    default:
> +        dolog ("Internal logic error: Bad audio format %d\n", as->fmt);
> +        goto deffmt;
> +
> +    }
> +    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.

> +
> +    hw->samples = conf.samples;
> +    twolame->pcm_buf = audio_calloc (AUDIO_FUNC, hw->samples, 1 << 
> hw->info.shift);
> +    if (!twolame->pcm_buf) {
> +        dolog ("Could not allocate buffer (%d bytes)\n",
> +               hw->samples << hw->info.shift);
> +        return -1;
> +    }
> +
> +    twolame->mpg_buf = audio_calloc (AUDIO_FUNC, hw->samples, 1 << 
> hw->info.shift);
> +    if (!twolame->mpg_buf) {

pcm_buf is not freed.

> +        dolog ("Could not allocate mpeg buffer (%d bytes)\n",
> +               hw->samples << hw->info.shift);
> +        return -1;
> +    }
> +
> +    char l[256];

Intermixed...

> +    sprintf(l, ":%d", conf.port);
> +    twolame->lsock = inet_listen (l, l, 256, SOCK_STREAM, 0);
> +
> +    qemu_set_fd_handler2(twolame->lsock, NULL, qtwolame_listen_read, NULL, 
> twolame);
> +
> +    if (audio_pt_init (&twolame->pt, qtwolame_thread_out, twolame, 
> AUDIO_CAP, AUDIO_FUNC)) {
> +        goto fail2;
> +    }
> +
> +    return 0;
> +
> + fail2:
> +    if (close (twolame->fd)) {
> +        qtwolame_logerr (errno, "%s: close on socket(%d) failed\n",
> +                     AUDIO_FUNC, twolame->fd);
> +    }
> +    twolame->fd = -1;
> +
> +// fail1:

Do not use C99 style comments.

> +
> +    qemu_free (twolame->mpg_buf);
> +    twolame->mpg_buf = NULL;
> +
> +    qemu_free (twolame->pcm_buf);
> +    twolame->pcm_buf = NULL;
> +    return -1;
> +}
> +
> +static void qtwolame_fini_out (HWVoiceOut *hw)
> +{
> +    void *ret;
> +    LAMEVoiceOut *twolame = (LAMEVoiceOut *) hw;
> +
> +    audio_pt_lock (&twolame->pt, AUDIO_FUNC);
> +    twolame->done = 1;
> +    audio_pt_unlock_and_signal (&twolame->pt, AUDIO_FUNC);
> +    audio_pt_join (&twolame->pt, &ret, AUDIO_FUNC);
> +
> +    if (twolame->fd >= 0) {
> +        if (close (twolame->fd)) {

close result is not checked consistently trhoughout this code

> +            qtwolame_logerr (errno, "failed to close socket\n");
> +        }
> +        twolame->fd = -1;
> +    }
> +
> +    if (twolame->options)
> +        twolame_close(&twolame->options);

Style.

> +    twolame->options = NULL;
> +
> +    audio_pt_fini (&twolame->pt, AUDIO_FUNC);
> +
> +    qemu_free (twolame->pcm_buf);
> +    twolame->pcm_buf = NULL;
> +    qemu_free (twolame->mpg_buf);
> +    twolame->mpg_buf = NULL;
> +}
> +
> +static int qtwolame_ctl_out (HWVoiceOut *hw, int cmd, ...)
> +{
> +    (void) hw;
> +    (void) cmd;
> +    return 0;
> +}
> +
> +/* common */
> +static void *qtwolame_audio_init (void)
> +{
> +    return &conf;
> +}
> +
> +static void qtwolame_audio_fini (void *opaque)
> +{
> +    (void) opaque;
> +    ldebug ("twolame_fini");
> +}
> +
> +struct audio_option qtwolame_options[] = {
> +    {
> +        .name  = "SAMPLES",
> +        .tag   = AUD_OPT_INT,
> +        .valp  = &conf.samples,
> +        .descr = "buffer size in samples"
> +    },
> +    {
> +        .name  = "DIVISOR",
> +        .tag   = AUD_OPT_INT,
> +        .valp  = &conf.divisor,
> +        .descr = "threshold divisor"
> +    },
> +    {
> +        .name  = "PORT",
> +        .tag   = AUD_OPT_INT,
> +        .valp  = &conf.port,
> +        .descr = "streamer port"
> +    },
> +    {
> +        .name  = "RATE",
> +        .tag   = AUD_OPT_INT,
> +        .valp  = &conf.rate,
> +        .descr = "bitrate"
> +    },
> +    { /* End of list */ }
> +};
> +
> +static struct audio_pcm_ops qtwolame_pcm_ops = {
> +    .init_out = qtwolame_init_out,
> +    .fini_out = qtwolame_fini_out,
> +    .run_out  = qtwolame_run_out,
> +    .write    = qtwolame_write,
> +    .ctl_out  = qtwolame_ctl_out,
> +};
> +
> +struct audio_driver twolame_audio_driver = {
> +    .name           = "twolame",
> +    .descr          = "mpeg1 layer2 streamer http://www.twolame.org/";,
> +    .options        = qtwolame_options,
> +    .init           = qtwolame_audio_init,
> +    .fini           = qtwolame_audio_fini,
> +    .pcm_ops        = &qtwolame_pcm_ops,
> +    .can_be_default = 0,
> +    .max_voices_out = 1,
> +    .max_voices_in  = 0,
> +    .voice_size_out = sizeof (LAMEVoiceOut),
> +    .voice_size_in  = 0
> +};
> diff --git a/configure b/configure
> index 7025d2b..ca8e980 100755
> --- a/configure
> +++ b/configure
> @@ -285,6 +285,7 @@ vnc_jpeg=""
>  vnc_png=""
>  vnc_thread="no"
>  xen=""
> +twolame=""
>  linux_aio=""
>  attr=""
>  vhost_net=""
> @@ -1155,6 +1156,21 @@ EOF
>  fi
>  
>  ##########################################
> +#
> +
> +cat > $TMPC <<EOF
> +#include <twolame.h>
> +int main(void) { twolame_options *encodeOptions; encodeOptions = 
> twolame_init(); return 0; }
> +EOF
> +if compile_prog "" "-ltwolame" ; then
> +  twolame="yes"
> +  audio_pt_int="yes"
> +  libs_softmmu="-ltwolame $libs_softmmu"
> +else
> +  twolame="no"
> +fi
> +
> +##########################################
>  # pkgconfig probe
>  
>  pkgconfig="${cross_prefix}pkg-config"
> @@ -2314,6 +2330,7 @@ if test -n "$sparc_cpu"; then
>      echo "Target Sparc Arch $sparc_cpu"
>  fi
>  echo "xen support       $xen"
> +echo "twolame streaming $twolame"
>  echo "brlapi support    $brlapi"
>  echo "bluez  support    $bluez"
>  echo "Documentation     $docs"
> @@ -2551,6 +2568,9 @@ fi
>  if test "$xen" = "yes" ; then
>    echo "CONFIG_XEN=y" >> $config_host_mak
>  fi
> +if test "$twolame" = "yes" ; then
> +  echo "CONFIG_TWOLAME=y" >> $config_host_mak
> +fi
>  if test "$io_thread" = "yes" ; then
>    echo "CONFIG_IOTHREAD=y" >> $config_host_mak
>    echo "CONFIG_THREAD=y" >> $config_host_mak
> 

-- 
mailto:address@hidden