Re: [PATCH] audio: Add sndio backend

[Volker Rümelin]

Hi Brad,

just a few white space and coding style issues.

> +/*
> + * stop polling descriptors
> + */
> +static void sndio_poll_clear(SndioVoice *self)
> +{
> +    struct pollfd *pfd;
> +    int i;
> +
> +    for (i = 0; i < self->nfds; i++) {
> +        pfd = &self->pfds[i];
> +        qemu_set_fd_handler (pfd->fd, NULL, NULL, NULL);

No space between function name and opening brace.

> +/*
> + * Set handlers for all descriptors libsndio needs to
> + * poll
> + */
> +static void sndio_poll_wait(SndioVoice *self)
> +{
> +    struct pollfd *pfd;
> +    int events, i;
> +
> +    events = 0;
> +    if (self->mode == SIO_PLAY) {
> +        if (self->sndio_pos < self->qemu_pos) {
> +            events |= POLLOUT;
> +        }
> +    } else {
> +        if (self->sndio_pos < self->buf_size) {
> +            events |= POLLIN;
> +        }
> +    }
> +
> +    /*
> +     * fill the given array of descriptors with the events sndio
> +     * wants, they are different from our 'event' variable because
> +     * sndio may use descriptors internally.
> +     */
> +    self->nfds = sio_pollfd(self->hdl, self->pfds, events);
> +
> +    for (i = 0; i < self->nfds; i++) {
> +        pfd = &self->pfds[i];
> +        if (pfd->fd < 0) {
> +                continue;

Indent with 4 spaces.

> +        }
> +        qemu_set_fd_handler(pfd->fd,
> +                (pfd->events & POLLIN) ? sndio_poll_in : NULL,
> +                (pfd->events & POLLOUT) ? sndio_poll_out : NULL,
> +                &self->pindexes[i]);

Same here.

> +static int sndio_init(SndioVoice *self,
> +                      struct audsettings *as, int mode, Audiodev *dev)
> +{
> +    AudiodevSndioOptions *opts = &dev->u.sndio;
> +    unsigned long long latency;
> +    const char *dev_name;
> +    struct sio_par req;
> +    unsigned int nch;
> +    int i, nfds;
> +
> +    dev_name = opts->has_dev ? opts->dev : SIO_DEVANY;
> +    latency = opts->has_latency ? opts->latency : SNDIO_LATENCY_US;
> +
> +    /* open the device in non-blocking mode */
> +    self->hdl = sio_open(dev_name, mode, 1);
> +    if (self->hdl == NULL) {
> +        dolog("failed to open device\n");
> +        return -1;
> +    }
> +
> +    self->mode = mode;
> +
> +    sio_initpar(&req);
> +
> +    switch (as->fmt) {
> +    case AUDIO_FORMAT_S8:
> +        req.bits = 8;
> +        req.sig = 1;
> +        break;
> +    case AUDIO_FORMAT_U8:
> +        req.bits = 8;
> +        req.sig = 0;
> +        break;
> +    case AUDIO_FORMAT_S16:
> +        req.bits = 16;
> +        req.sig = 1;
> +        break;
> +    case AUDIO_FORMAT_U16:
> +        req.bits = 16;
> +        req.sig = 0;
> +        break;
> +    case AUDIO_FORMAT_S32:
> +        req.bits = 32;
> +        req.sig = 1;
> +        break;
> +    case AUDIO_FORMAT_U32:
> +        req.bits = 32;
> +        req.sig = 0;
> +    default:
> +        dolog("unknown audio sample format\n");
> +        return -1;
> +    }
> +
> +    if (req.bits > 8) {
> +        req.le = as->endianness ? 0 : 1;
> +    }
> +
> +    req.rate = as->freq;
> +    if (mode == SIO_PLAY) {
> +            req.pchan = as->nchannels;

Indent with 4 spaces.

> +    } else {
> +            req.rchan = as->nchannels;

Here too.

> +static void sndio_enable_out(HWVoiceOut *hw, bool enable)
> +{
> +    SndioVoice *self = (SndioVoice *) hw;
> +
> +    return sndio_enable(self, enable);
> +}

Unnecessary return statement.

> +
> +static void sndio_enable_in(HWVoiceIn *hw, bool enable)
> +{
> +    SndioVoice *self = (SndioVoice *) hw;
> +
> +    return sndio_enable(self, enable);
> +}

The same here

> +static void sndio_fini_out(HWVoiceOut *hw)
> +{
> +    SndioVoice *self = (SndioVoice *) hw;
> +
> +    return sndio_fini(self);
> +}

and here

> +
> +static void sndio_fini_in(HWVoiceIn *hw)
> +{
> +    SndioVoice *self = (SndioVoice *) hw;
> +
> +    return sndio_fini(self);
> +}

and here.

With best regards,
Volker