On Thu, Jul 19, 2018 at 11:24:16AM +0300, Leonid Bobrov wrote:
>       From: Alexandre Ratchov <a...@caoua.org>
> 
>       On Sun, Jul 15, 2018 at 06:40:54AM +0300, Leonid Bobrov wrote:
>       > 
>       > The diff I use is initial one:
>       > https://marc.info/?l=openbsd-ports&m=152807586927014&w=2
>       > 
>       > ratchov@'s sndio backend is not ready yet, plus I'm afraid to keep
>       > testing it, because that made kernel panic, after that panic I 
> rebooted
>       > with corrupted FFS (I should read about sending bug reports).
>       > 
> 
>       Please let me know what doesn't work, including steps to reproduce the
>       problem. If the problem is specific to your machines, at least provide
>       a stack trace.
> 
>       The kernel crashes you mentioned (unrelated to openal) and are very
>       important to fix, see sthen@ suggestion on how to handle them.
> 
>       Note that the diff I sent doesn't affect the playback direction, so
>       verifying that it doesn't cause regressions would help as well.
> 
> The steps to reproduce are: install net/toxic, install your latest diff
> to audio/openal:
> https://marc.info/?l=openbsd-ports&m=152817852312699&w=2
> 
> Then start Toxic
> $ toxic
> and make audiocall to anybody. If you don't have contacts to test
> audiocall, add echobot to your contact list:
> /add echo...@toxme.io
> Wait few seconds, switch to buffer with echobot and use:
> /call
> Then when you're done testing audio input, end audio call:
> /hangup
> And that'll lead to segmentation fault.
> 

Thank you. It seems toxic closes the device without stopping it. I've
fixed the playback direction as well, maybe there are other programs
doing this for playback. New diff below.

I've reviewed the sndio backend and see no more capture-related
issues. If this works for you and there are no regressions, we could
either commit it or start merging 1.18.2 changes.

What do you think?

Leonid, could you confirm capture works?

Index: Makefile
===================================================================
RCS file: /cvs/ports/audio/openal/Makefile,v
retrieving revision 1.50
diff -u -p -u -p -r1.50 Makefile
--- Makefile    31 Dec 2017 18:46:26 -0000      1.50
+++ Makefile    25 Jul 2018 07:24:48 -0000
@@ -10,7 +10,7 @@ DISTNAME =    openal-soft-$V
 PKGNAME =      openal-$V
 CATEGORIES =   audio
 SHARED_LIBS =  openal  3.0
-REVISION =     0
+REVISION =     1
 
 HOMEPAGE =     http://kcat.strangesoft.net/openal.html
 
Index: patches/patch-Alc_backends_sndio_c
===================================================================
RCS file: patches/patch-Alc_backends_sndio_c
diff -N patches/patch-Alc_backends_sndio_c
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ patches/patch-Alc_backends_sndio_c  25 Jul 2018 07:24:48 -0000
@@ -0,0 +1,404 @@
+$OpenBSD$
+
+Index: Alc/backends/sndio.c
+--- Alc/backends/sndio.c.orig
++++ Alc/backends/sndio.c
+@@ -42,16 +42,18 @@ static ALCboolean sndio_load(void)
+ 
+ typedef struct {
+     struct sio_hdl *sndHandle;
++    int mode, started;
+ 
+     ALvoid *mix_data;
+     ALsizei data_size;
+ 
++    ll_ringbuffer_t *ring;
++
+     volatile int killNow;
+     althrd_t thread;
+ } sndio_data;
+ 
+-
+-static int sndio_proc(void *ptr)
++static int sndio_proc_playback(void *ptr)
+ {
+     ALCdevice *device = ptr;
+     sndio_data *data = device->ExtraData;
+@@ -89,53 +91,22 @@ static int sndio_proc(void *ptr)
+     return 0;
+ }
+ 
+-
+-
+-static ALCenum sndio_open_playback(ALCdevice *device, const ALCchar 
*deviceName)
++static ALCboolean sndio_setparams(ALCdevice *device)
+ {
+-    sndio_data *data;
+-
+-    if(!deviceName)
+-        deviceName = sndio_device;
+-    else if(strcmp(deviceName, sndio_device) != 0)
+-        return ALC_INVALID_VALUE;
+-
+-    data = calloc(1, sizeof(*data));
+-    data->killNow = 0;
+-
+-    data->sndHandle = sio_open(NULL, SIO_PLAY, 0);
+-    if(data->sndHandle == NULL)
+-    {
+-        free(data);
+-        ERR("Could not open device\n");
+-        return ALC_INVALID_VALUE;
+-    }
+-
+-    al_string_copy_cstr(&device->DeviceName, deviceName);
+-    device->ExtraData = data;
+-
+-    return ALC_NO_ERROR;
+-}
+-
+-static void sndio_close_playback(ALCdevice *device)
+-{
+     sndio_data *data = device->ExtraData;
+-
+-    sio_close(data->sndHandle);
+-    free(data);
+-    device->ExtraData = NULL;
+-}
+-
+-static ALCboolean sndio_reset_playback(ALCdevice *device)
+-{
+-    sndio_data *data = device->ExtraData;
+     struct sio_par par;
++    unsigned int nch;
+ 
+     sio_initpar(&par);
+ 
+     par.rate = device->Frequency;
+-    par.pchan = ((device->FmtChans != DevFmtMono) ? 2 : 1);
++    nch = ((device->FmtChans != DevFmtMono) ? 2 : 1);
+ 
++    if (data->mode & SIO_PLAY)
++      par.pchan = nch;
++    else if (data->mode & SIO_REC)
++      par.rchan = nch;
++
+     switch(device->FmtType)
+     {
+         case DevFmtByte:
+@@ -182,8 +153,10 @@ static ALCboolean sndio_reset_playback(ALCdevice *devi
+         return ALC_FALSE;
+     }
+ 
++    nch = (data->mode & SIO_PLAY) ? par.pchan : par.rchan;
++
+     device->Frequency = par.rate;
+-    device->FmtChans = ((par.pchan==1) ? DevFmtMono : DevFmtStereo);
++    device->FmtChans = ((nch == 1) ? DevFmtMono : DevFmtStereo);
+ 
+     if(par.bits == 8 && par.sig == 1)
+         device->FmtType = DevFmtByte;
+@@ -211,10 +184,46 @@ static ALCboolean sndio_reset_playback(ALCdevice *devi
+     return ALC_TRUE;
+ }
+ 
++static ALCenum sndio_open_playback(ALCdevice *device, const ALCchar 
*deviceName)
++{
++    sndio_data *data;
++
++    if(!deviceName)
++        deviceName = sndio_device;
++    else if(strcmp(deviceName, sndio_device) != 0)
++        return ALC_INVALID_VALUE;
++
++    data = calloc(1, sizeof(*data));
++    data->killNow = 0;
++    data->started = 0;
++
++    data->mode = SIO_PLAY;
++    data->sndHandle = sio_open(NULL, data->mode, 0);
++    if(data->sndHandle == NULL)
++    {
++        free(data);
++        ERR("Could not open device\n");
++        return ALC_INVALID_VALUE;
++    }
++
++    al_string_copy_cstr(&device->DeviceName, deviceName);
++    device->ExtraData = data;
++
++    return ALC_NO_ERROR;
++}
++
++static ALCboolean sndio_reset_playback(ALCdevice *device)
++{
++    return sndio_setparams(device);
++}
++
+ static ALCboolean sndio_start_playback(ALCdevice *device)
+ {
+     sndio_data *data = device->ExtraData;
+ 
++    if (data->started)
++        return ALC_TRUE;
++
+     if(!sio_start(data->sndHandle))
+     {
+         ERR("Error starting playback\n");
+@@ -225,7 +234,7 @@ static ALCboolean sndio_start_playback(ALCdevice *devi
+     data->mix_data = calloc(1, data->data_size);
+ 
+     data->killNow = 0;
+-    if(althrd_create(&data->thread, sndio_proc, device) != althrd_success)
++    if(althrd_create(&data->thread, sndio_proc_playback, device) != 
althrd_success)
+     {
+         sio_stop(data->sndHandle);
+         free(data->mix_data);
+@@ -233,6 +242,7 @@ static ALCboolean sndio_start_playback(ALCdevice *devi
+         return ALC_FALSE;
+     }
+ 
++    data->started = 1;
+     return ALC_TRUE;
+ }
+ 
+@@ -241,6 +251,9 @@ static void sndio_stop_playback(ALCdevice *device)
+     sndio_data *data = device->ExtraData;
+     int res;
+ 
++    if (!data->started)
++        return;
++
+     if(data->killNow)
+         return;
+ 
+@@ -252,21 +265,215 @@ static void sndio_stop_playback(ALCdevice *device)
+ 
+     free(data->mix_data);
+     data->mix_data = NULL;
++    data->started = 0;
+ }
+ 
++static void sndio_close_playback(ALCdevice *device)
++{
++    sndio_data *data = device->ExtraData;
+ 
++    if (data->started)
++        sndio_stop_playback(device);
++    sio_close(data->sndHandle);
++    free(data);
++    device->ExtraData = NULL;
++}
++
++static int sndio_proc_capture(void *ptr)
++{
++    static char dummy[1024];
++    ALCdevice *device = ptr;
++    sndio_data *data = device->ExtraData;
++    ll_ringbuffer_data_t vec[2], *v;
++    ALsizei frameSize;
++    size_t n, todo, len;
++    char *buf;
++
++    SetRTPriority();
++    althrd_setname(althrd_current(), RECORD_THREAD_NAME);
++
++    frameSize = FrameSizeFromDevFmt(device->FmtChans, device->FmtType);
++
++    while(!data->killNow && device->Connected)
++    {
++        todo = device->UpdateSize * frameSize;
++        ll_ringbuffer_get_write_vector(data->ring, vec);
++
++      if (vec[0].len + vec[1].len < device->UpdateSize) {
++
++          /* we're out of free space, drop next block */
++          while (todo > 0) {
++              len = sizeof(dummy);
++              if (len > todo)
++                  len = todo;
++              n = sio_read(data->sndHandle, dummy, len);
++              if (n == 0) {
++                  ERR("sio_read failed\n");
++                  ALCdevice_Lock(device);
++                          aluHandleDisconnect(device);
++                  ALCdevice_Unlock(device);
++              }
++              todo -= n;
++          }
++
++      } else {
++
++          /* record into the ring */
++          v = vec;
++          buf = NULL;
++          len = 0;
++          while (todo > 0) {
++              if (len == 0) {
++                  buf = v->buf;
++                  len = v->len * frameSize;
++                  v++;
++              }
++              n = sio_read(data->sndHandle, buf, len);
++              if (n == 0) {
++                  ERR("sio_read failed\n");
++                  ALCdevice_Lock(device);
++                          aluHandleDisconnect(device);
++                  ALCdevice_Unlock(device);
++              }
++              len -= n;
++              buf += n;
++              todo -= n;
++          }
++          ll_ringbuffer_write_advance(data->ring, device->UpdateSize);
++
++      }
++    }
++
++    return 0;
++}
++
++static ALCenum sndio_open_capture(ALCdevice *device, const ALCchar 
*deviceName)
++{
++    sndio_data *data;
++
++    if(!deviceName)
++        deviceName = sndio_device;
++    else if(strcmp(deviceName, sndio_device) != 0)
++        return ALC_INVALID_VALUE;
++
++    data = calloc(1, sizeof(*data));
++    data->killNow = 0;
++    data->started = 0;
++    data->ring = NULL;
++    data->mode = SIO_REC;
++    data->sndHandle = sio_open(NULL, data->mode, 0);
++    if(data->sndHandle == NULL)
++    {
++        free(data);
++        ERR("Could not open device\n");
++        return ALC_INVALID_VALUE;
++    }
++
++    al_string_copy_cstr(&device->DeviceName, deviceName);
++    device->ExtraData = data;
++
++    if (!sndio_setparams(device)) {
++      sio_close(data->sndHandle);
++        free(data);
++        return ALC_INVALID_VALUE;
++    }
++
++    return ALC_NO_ERROR;
++}
++
++static void sndio_start_capture(ALCdevice *device)
++{
++    sndio_data *data = device->ExtraData;
++    int frameSize;
++
++    if (data->started)
++        return;
++
++    if(!sio_start(data->sndHandle))
++    {
++        ERR("Error starting capture\n");
++        return;
++    }
++
++    frameSize = FrameSizeFromDevFmt(device->FmtChans, device->FmtType);
++    data->ring = ll_ringbuffer_create(device->UpdateSize * 
device->NumUpdates, frameSize);
++    if (!data->ring) {
++        sio_stop(data->sndHandle);
++        return;
++    }
++
++    data->killNow = 0;
++    if(althrd_create(&data->thread, sndio_proc_capture, device) != 
althrd_success)
++    {
++        ll_ringbuffer_free(data->ring);
++        sio_stop(data->sndHandle);
++        return;
++    }
++
++    data->started = 1;
++    return;
++}
++
++static void sndio_stop_capture(ALCdevice *device)
++{
++    sndio_data *data = device->ExtraData;
++    int res;
++
++    if (!data->started)
++        return;
++
++    if(data->killNow)
++        return;
++
++    data->killNow = 1;
++    althrd_join(data->thread, &res);
++
++    if(!sio_stop(data->sndHandle))
++        ERR("Error stopping device\n");
++
++    ll_ringbuffer_free(data->ring);
++    data->ring = NULL;
++    data->started = 0;
++}
++
++static ALCenum sndio_captureSamples(ALCdevice *device, ALCvoid *buffer, 
ALCuint samples)
++{
++    sndio_data *data = device->ExtraData;
++
++    ll_ringbuffer_read(data->ring, buffer, samples);
++    return ALC_NO_ERROR;
++}
++
++static ALCuint sndio_availableSamples(ALCdevice *device)
++{
++    sndio_data *data = device->ExtraData;
++
++    return ll_ringbuffer_read_space(data->ring);
++}
++
++static void sndio_close_capture(ALCdevice *device)
++{
++    sndio_data *data = device->ExtraData;
++
++    if (data->started)
++        sndio_stop_capture(device);
++    sio_close(data->sndHandle);
++    free(data);
++    device->ExtraData = NULL;
++}
++
+ static const BackendFuncs sndio_funcs = {
+     sndio_open_playback,
+     sndio_close_playback,
+     sndio_reset_playback,
+     sndio_start_playback,
+     sndio_stop_playback,
+-    NULL,
+-    NULL,
+-    NULL,
+-    NULL,
+-    NULL,
+-    NULL
++    sndio_open_capture,
++    sndio_close_capture,
++    sndio_start_capture,
++    sndio_stop_capture,
++    sndio_captureSamples,
++    sndio_availableSamples
+ };
+ 
+ ALCboolean alc_sndio_init(BackendFuncs *func_list)
+@@ -289,6 +496,7 @@ void alc_sndio_probe(enum DevProbe type)
+             AppendAllDevicesList(sndio_device);
+             break;
+         case CAPTURE_DEVICE_PROBE:
++            AppendCaptureDeviceList(sndio_device);
+             break;
+     }
+ }

Reply via email to