[vlc-devel] [PATCH] sndio: Updated sndio backend which actually works.
Rémi Denis-Courmont
remi at remlab.net
Thu Jun 21 05:20:52 CEST 2012
Le jeudi 21 juin 2012 03:05:24 Brad Smith, vous avez écrit :
> sndio: Updated sndio backend which actually works.
>
> Provided by Alexandre Ratchov <alex at caoua.org>
This patch defeats my reviewing skills because it dilutes 5% fixes and 5%
arguable changes in 90% cosmetic changes.
> ---
> NEWS | 2 +-
> configure.ac | 7 +-
> modules/LIST | 2 +-
> modules/audio_output/sndio.c | 237
> +++++++++++++++++++++++++++--------------- 4 files changed, 156
> insertions(+), 92 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index ecde695..6bbf5d0 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -31,7 +31,7 @@ Demuxers:
>
> Audio output:
> * Windows Audio Session API audio output support
> - * OpenBSD sndio audio output support
> + * sndio audio output support
Why? Far more people know what OpenBSD is, than what sndio is.
> + vlc_fourcc_to_char (aout->format.i_format, fourcc);
> + do {
> + if (fourcc[0] == 's')
> + par.sig = 1;
> + else if (fourcc[0] == 'u')
> + par.sig = 0;
> + else
> + break;
> + if (fourcc[1] == '8' && fourcc[2] == ' ' && fourcc[3] == ' ') {
> + par.bits = 8;
> + break;
> + } else if (fourcc[1] == '1' && fourcc[2] == '6') {
> + par.bits = 16;
> + } else if (fourcc[1] == '2' && fourcc[2] == '4') {
> + par.bits = 24;
> + par.bps = 3;
> + } else if (fourcc[1] == '3' && fourcc[2] == '2') {
> + par.bits = 32;
> + }
> + if (fourcc[3] == 'l')
> + par.le = 1;
> + else if (fourcc[3] == 'b')
> + par.le = 0;
> + else
> + break;
> + } while (0);
You're not supposed to "parse" FOURCC like that. Use switch statement.
> /* Channel map */
> - unsigned chans;
> switch (par.pchan)
> {
> case 1:
> chans = AOUT_CHAN_CENTER;
> break;
> case 2:
> - chans = AOUT_CHAN_LEFT | AOUT_CHAN_RIGHT;
> + chans = AOUT_CHANS_STEREO;
> break;
> case 4:
> - chans = AOUT_CHAN_LEFT | AOUT_CHAN_RIGHT
> - | AOUT_CHAN_REARLEFT | AOUT_CHAN_REARRIGHT;
> + chans = AOUT_CHANS_4_0;
> break;
> case 6:
> - chans = AOUT_CHAN_LEFT | AOUT_CHAN_RIGHT
> - | AOUT_CHAN_REARLEFT | AOUT_CHAN_REARRIGHT
> - | AOUT_CHAN_CENTER | AOUT_CHAN_LFE;
> + chans = AOUT_CHANS_5_1;
> + break;
> + case 8:
> + chans = AOUT_CHANS_7_1;
> break;
> default:
> msg_Err (aout, "unknown %u channels map", par.pchan);
> goto error;
> }
That chunk is purely cosmetic and belongs in a separate patch.
> - f.i_original_channels = f.i_physical_channels = chans;
> - aout_FormatPrepare (&f);
> -
> - aout->format = f;
> - aout->sys = (void *)sio;
> + sys = malloc (sizeof(struct aout_sys_t));
> + if (sys == NULL) {
> + msg_Err (aout, "failed to allocate sndio structure");
> + goto error;
> + }
> + sys->hdl = hdl;
> + sys->bpf = par.bps * par.pchan;
> + sys->delay = 0;
> + aout->sys = sys;
> + aout->format.i_format =
> + VLC_FOURCC(fourcc[0], fourcc[1], fourcc[2], fourcc[3]);
> + msg_Warn(aout, "pchan = %u, rate = %u, bufsz = %u, round = %u\n",
> + par.pchan, par.rate, par.bufsz, par.round);
> + msg_Warn(aout, "orig_chans = 0x%x, phys_chans = 0x%x, chans = 0x%x\n",
> + aout->format.i_original_channels,
> + aout->format.i_physical_channels,
> + chans);
Warning can't be right.
> + aout->format.i_original_channels = chans;
> + aout->format.i_physical_channels = chans;
> + aout->format.i_rate = par.rate;
> aout->pf_play = Play;
> aout->pf_pause = Pause;
> - aout->pf_flush = NULL; /* sndio sucks! */
> - aout_VolumeSoftInit (aout); /* TODO: sio_onvol() */
> -
> - sio_start (sio);
> + aout->pf_flush = Flush;
> + aout_VolumeHardInit (aout, VolumeSet);
> + VolumeSet(aout,
> + var_InheritInteger (aout, "volume") / (float)AOUT_VOLUME_DEFAULT,
> + var_InheritBool (aout, "mute"));
> + sio_onmove (hdl, onmove, aout);
> + sio_start (hdl);
> return VLC_SUCCESS;
>
> error:
> - sio_close (sio);
> + sio_close (hdl);
> return VLC_EGENERIC;
> }
>
> static void Close (vlc_object_t *obj)
> {
> audio_output_t *aout = (audio_output_t *)obj;
> - struct sio_hdl *sio = (void *)aout->sys;
> + struct aout_sys_t *sys = (struct aout_sys_t *)aout->sys;
>
> - sio_close (sio);
> + sio_close (sys->hdl);
> + free (sys);
> }
>
> static void Play (audio_output_t *aout, block_t *block)
> {
> - struct sio_hdl *sio = (void *)aout->sys;
> - struct sio_par par;
> + struct aout_sys_t *sys = (struct aout_sys_t *)aout->sys;
>
> - if (sio_getpar (sio, &par) == 0)
> - {
> - mtime_t delay = par.bufsz * CLOCK_FREQ / aout->format.i_rate;
> -
> - delay = block->i_pts - (mdate () - delay);
> - aout_TimeReport (aout, block->i_pts - delay);
> - }
> + aout_TimeReport (aout, block->i_pts -
> + sys->delay * CLOCK_FREQ / aout->format.i_rate);
> + sio_write (sys->hdl, block->p_buffer, block->i_nb_samples * sys->bpf);
> + sys->delay += block->i_nb_samples;
> + block_Release (block);
> +}
>
> - while (block->i_buffer > 0 && !sio_eof (sio))
> - {
> - size_t bytes = sio_write (sio, block->p_buffer, block->i_buffer);
> +static void Pause (audio_output_t *aout, bool pause, mtime_t date)
> +{
> + struct aout_sys_t *sys = (struct aout_sys_t *)aout->sys;
> + static char zeros[100];
> + unsigned int todo, n;
>
> - block->p_buffer += bytes;
> - block->i_buffer -= bytes;
> - /* Note that i_nb_samples and i_pts are corrupted here. */
> + if (pause) {
> + sio_stop (sys->hdl);
> + sio_start (sys->hdl);
> + } else {
> + todo = sys->delay * sys->bpf;
> + while (todo > 0) {
> + n = todo;
> + if (n >= sizeof(zeros))
> + n = sizeof(zeros);
> + sio_write(sys->hdl, zeros, n);
> + todo -= n;
> + }
> }
> - block_Release (block);
> + (void)date;
> }
This cannot possibly work. The buffer might have underrun a million times
before Pause(..., false, ...) is called.
>
> -static void Pause (audio_output_t *aout, bool pause, mtime_t date)
> +static void Flush (audio_output_t *aout, bool wait)
> {
> - struct sio_hdl *sio = (void *)aout->sys;
> + struct aout_sys_t *sys = (struct aout_sys_t *)aout->sys;
> +
> + sys->delay = 0;
> + (void)wait;
> +}
Doing nothing in Flush() cannot be right.
> +
> +static int VolumeSet(audio_output_t *aout, float vol, bool mute)
> +{
> + struct aout_sys_t *sys = (struct aout_sys_t *)aout->sys;
> + int ctl;
>
> - if (pause)
> - sio_stop (sio);
> - else
> - sio_start (sio);
> - (void) date;
> + if (mute)
> + ctl = 0;
> + else {
> + if (vol < 0)
> + vol = 0;
> + if (vol > 1)
> + vol = 1;
> + ctl = vol * SIO_MAXVOL;
> + }
> + sio_setvol (sys->hdl, ctl);
> + return VLC_SUCCESS;
> }
--
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis
More information about the vlc-devel
mailing list