ao_oss: remove duplicate audio device open code

The code for reopening the audio device was separate, and duplicated
some of the "real" open code. This was very badly done, and major
required parts of initialization were skipped. Fix this by removing
the code duplication. This consists mainly of moving the code for
opening the device to a separate function, and adding some changes
to handle format changes gracefully. (We can't change the audio
format on the fly, but we can at least not explode and play noise
when that happens.)

As a minor change, actually always use SNDCTL_DSP_RESET when closing
the audio device. We don't want to wait until the rest of the buffer
is played.

Also, don't use strerror() when printing the error message that
reopening failed, simply because reopen_device() takes care of this,
and also errno might be clobbered at this point.

1 files changed, 108 insertions, 104 deletions

diff --git a/audio/out/ao_oss.c b/audio/out/ao_oss.c
index a2d2243ebd..3f7a6edb47 100644
--- a/audio/out/ao_oss.c
+++ b/audio/out/ao_oss.c
@@ -233,8 +233,7 @@ static int device_writable(struct ao *ao)
     return poll(&fd, 1, 0);
 }
 
-// close audio device
-static void uninit(struct ao *ao)
+static void close_device(struct ao *ao)
 {
     struct priv *p = ao->priv;
     if (p->audio_fd == -1)
@@ -246,51 +245,20 @@ static void uninit(struct ao *ao)
     p->audio_fd = -1;
 }
 
-// open & setup audio device
-// return: 0=success -1=fail
-static int init(struct ao *ao)
+// close audio device
+static void uninit(struct ao *ao)
+{
+    close_device(ao);
+}
+
+static int reopen_device(struct ao *ao, bool allow_format_changes)
 {
     struct priv *p = ao->priv;
     int oss_format;
 
-    const char *mchan = NULL;
-    if (p->cfg_oss_mixer_channel && p->cfg_oss_mixer_channel[0])
-        mchan = p->cfg_oss_mixer_channel;
-
-    if (mchan) {
-        int fd, devs, i;
-
-        if ((fd = open(p->oss_mixer_device, O_RDONLY)) == -1) {
-            MP_ERR(ao, "Can't open mixer device %s: %s\n",
-                   p->oss_mixer_device, strerror(errno));
-        } else {
-            ioctl(fd, SOUND_MIXER_READ_DEVMASK, &devs);
-            close(fd);
-
-            for (i = 0; i < SOUND_MIXER_NRDEVICES; i++) {
-                if (!strcasecmp(mixer_channels[i], mchan)) {
-                    if (!(devs & (1 << i))) {
-                        MP_ERR(ao, "Audio card mixer does not have "
-                               "channel '%s', using default.\n", mchan);
-                        i = SOUND_MIXER_NRDEVICES + 1;
-                        break;
-                    }
-                    p->oss_mixer_channel = i;
-                    break;
-                }
-            }
-            if (i == SOUND_MIXER_NRDEVICES) {
-                MP_ERR(ao, "Audio card mixer does not have "
-                       "channel '%s', using default.\n", mchan);
-            }
-        }
-    } else {
-        p->oss_mixer_channel = SOUND_MIXER_PCM;
-    }
-
-    MP_VERBOSE(ao, "using '%s' dsp device\n", p->dsp);
-    MP_VERBOSE(ao, "using '%s' mixer device\n", p->oss_mixer_device);
-    MP_VERBOSE(ao, "using '%s' mixer device\n", mixer_channels[p->oss_mixer_channel]);
+    int samplerate = ao->samplerate;
+    int format = ao->format;
+    struct mp_chmap channels = ao->channels;
 
 #ifdef __linux__
     p->audio_fd = open(p->dsp, O_WRONLY | O_NONBLOCK);
@@ -314,67 +282,65 @@ static int init(struct ao *ao)
     fcntl(p->audio_fd, F_SETFD, FD_CLOEXEC);
 #endif
 
-    ao->format = af_fmt_from_planar(ao->format);
-
-    if (AF_FORMAT_IS_AC3(ao->format)) {
-        ioctl(p->audio_fd, SNDCTL_DSP_SPEED, &ao->samplerate);
+    if (AF_FORMAT_IS_AC3(format)) {
+        ioctl(p->audio_fd, SNDCTL_DSP_SPEED, &samplerate);
     }
 
 ac3_retry:
-    if (AF_FORMAT_IS_AC3(ao->format))
-        ao->format = AF_FORMAT_AC3;
-    oss_format = format2oss(ao->format);
+    if (AF_FORMAT_IS_AC3(format))
+        format = AF_FORMAT_AC3;
+    oss_format = format2oss(format);
     if (oss_format == -1) {
         MP_VERBOSE(ao, "Unknown/not supported internal format: %s\n",
-                   af_fmt_to_str(ao->format));
+                   af_fmt_to_str(format));
 #if defined(AFMT_S32_LE) && defined(AFMT_S32_BE)
 #if BYTE_ORDER == BIG_ENDIAN
         oss_format = AFMT_S32_BE;
 #else
         oss_format = AFMT_S32_LE;
 #endif
-        ao->format = AF_FORMAT_S32;
+        format = AF_FORMAT_S32;
 #elif defined(AFMT_S24_LE) && defined(AFMT_S24_BE)
 #if BYTE_ORDER == BIG_ENDIAN
         oss_format = AFMT_S24_BE;
 #else
         oss_format = AFMT_S24_LE;
 #endif
-        ao->format = AF_FORMAT_S24;
+        format = AF_FORMAT_S24;
 #else
 #if BYTE_ORDER == BIG_ENDIAN
         oss_format = AFMT_S16_BE;
 #else
         oss_format = AFMT_S16_LE;
 #endif
-        ao->format = AF_FORMAT_S16;
+        format = AF_FORMAT_S16;
 #endif
     }
     if (ioctl(p->audio_fd, SNDCTL_DSP_SETFMT, &oss_format) < 0 ||
-        oss_format != format2oss(ao->format))
+        oss_format != format2oss(format))
     {
         MP_WARN(ao, "Can't set audio device %s to %s output, trying %s...\n",
-                p->dsp, af_fmt_to_str(ao->format),
+                p->dsp, af_fmt_to_str(format),
                 af_fmt_to_str(AF_FORMAT_S16));
-        ao->format = AF_FORMAT_S16;
+        format = AF_FORMAT_S16;
         goto ac3_retry;
     }
 
-    ao->format = oss2format(oss_format);
-    if (ao->format == -1) {
+    format = oss2format(oss_format);
+    if (format == -1) {
         MP_ERR(ao, "Unknown/Unsupported OSS format: %x.\n", oss_format);
         goto fail;
     }
 
-    MP_VERBOSE(ao, "sample format: %s\n", af_fmt_to_str(ao->format));
+    MP_VERBOSE(ao, "sample format: %s\n", af_fmt_to_str(format));
 
-    if (!AF_FORMAT_IS_AC3(ao->format)) {
+    if (!AF_FORMAT_IS_AC3(format)) {
         struct mp_chmap_sel sel = {0};
         for (int n = 0; n < MP_NUM_CHANNELS + 1; n++)
             mp_chmap_sel_add_map(&sel, &oss_layouts[n]);
-        if (!ao_chmap_sel_adjust(ao, &sel, &ao->channels))
+        if (!ao_chmap_sel_adjust(ao, &sel, &channels))
             goto fail;
-        int reqchannels = ao->channels.num;
+        int reqchannels = channels.num;
         // We only use SNDCTL_DSP_CHANNELS for >2 channels, in case some drivers don't have it
         if (reqchannels > 2) {
             int nchannels = reqchannels;
@@ -392,14 +358,14 @@ ac3_retry:
                        reqchannels);
                 goto fail;
             }
-            if (!ao_chmap_sel_get_def(ao, &sel, &ao->channels, c + 1))
+            if (!ao_chmap_sel_get_def(ao, &sel, &channels, c + 1))
                 goto fail;
         }
         MP_VERBOSE(ao, "using %d channels (requested: %d)\n",
-                   ao->channels.num, reqchannels);
+                   channels.num, reqchannels);
         // set rate
-        ioctl(p->audio_fd, SNDCTL_DSP_SPEED, &ao->samplerate);
-        MP_VERBOSE(ao, "using %d Hz samplerate\n", ao->samplerate);
+        ioctl(p->audio_fd, SNDCTL_DSP_SPEED, &samplerate);
+        MP_VERBOSE(ao, "using %d Hz samplerate\n", samplerate);
     }
 
     if (ioctl(p->audio_fd, SNDCTL_DSP_GETOSPACE, &p->zz) == -1) {
@@ -418,6 +384,78 @@ ac3_retry:
         p->outburst = p->zz.fragsize;
     }
 
+    if (allow_format_changes) {
+        ao->format = format;
+        ao->samplerate = samplerate;
+        ao->channels = channels;
+    } else {
+        if (format != ao->format || samplerate != ao->samplerate ||
+            !mp_chmap_equals(&channels, &ao->channels))
+        {
+            MP_ERR(ao, "Could not reselect previous audio format.\n");
+            goto fail;
+        }
+    }
+
+    p->outburst -= p->outburst % (channels.num * af_fmt2bps(format)); // round down
+
+    return 0;
+
+fail:
+    close_device(ao);
+    return -1;
+}
+
+// open & setup audio device
+// return: 0=success -1=fail
+static int init(struct ao *ao)
+{
+    struct priv *p = ao->priv;
+
+    const char *mchan = NULL;
+    if (p->cfg_oss_mixer_channel && p->cfg_oss_mixer_channel[0])
+        mchan = p->cfg_oss_mixer_channel;
+
+    if (mchan) {
+        int fd, devs, i;
+
+        if ((fd = open(p->oss_mixer_device, O_RDONLY)) == -1) {
+            MP_ERR(ao, "Can't open mixer device %s: %s\n",
+                   p->oss_mixer_device, strerror(errno));
+        } else {
+            ioctl(fd, SOUND_MIXER_READ_DEVMASK, &devs);
+            close(fd);
+
+            for (i = 0; i < SOUND_MIXER_NRDEVICES; i++) {
+                if (!strcasecmp(mixer_channels[i], mchan)) {
+                    if (!(devs & (1 << i))) {
+                        MP_ERR(ao, "Audio card mixer does not have "
+                               "channel '%s', using default.\n", mchan);
+                        i = SOUND_MIXER_NRDEVICES + 1;
+                        break;
+                    }
+                    p->oss_mixer_channel = i;
+                    break;
+                }
+            }
+            if (i == SOUND_MIXER_NRDEVICES) {
+                MP_ERR(ao, "Audio card mixer does not have "
+                       "channel '%s', using default.\n", mchan);
+            }
+        }
+    } else {
+        p->oss_mixer_channel = SOUND_MIXER_PCM;
+    }
+
+    MP_VERBOSE(ao, "using '%s' dsp device\n", p->dsp);
+    MP_VERBOSE(ao, "using '%s' mixer device\n", p->oss_mixer_device);
+    MP_VERBOSE(ao, "using '%s' mixer device\n", mixer_channels[p->oss_mixer_channel]);
+
+    ao->format = af_fmt_from_planar(ao->format);
+
+    if (reopen_device(ao, true) < 0)
+        goto fail;
+
     if (p->buffersize == -1) {
         // Measuring buffer size:
         void *data = malloc(p->outburst);
@@ -438,10 +476,6 @@ ac3_retry:
         }
     }
 
-    ao->bps = ao->channels.num * af_fmt2bps(ao->format);
-    p->outburst -= p->outburst % ao->bps; // round down
-    ao->bps *= ao->samplerate;
-
     return 0;
 
 fail:
@@ -459,48 +493,18 @@ static void drain(struct ao *ao)
 #endif
 }
 
-#if !KEEP_DEVICE
-static void close_device(struct ao *ao)
-{
-    struct priv *p = ao->priv;
-    close(p->audio_fd);
-    p->audio_fd = -1;
-}
-#endif
-
 // stop playing and empty buffers (for seeking/pause)
 static void reset(struct ao *ao)
 {
-    struct priv *p = ao->priv;
 #if KEEP_DEVICE
+    struct priv *p = ao->priv;
     ioctl(p->audio_fd, SNDCTL_DSP_RESET, NULL);
 #else
     close_device(ao);
-    p->audio_fd = open(p->dsp, O_WRONLY);
-    if (p->audio_fd < 0) {
-        MP_ERR(ao, "Fatal error: *** CANNOT "
-               "RE-OPEN / RESET AUDIO DEVICE *** %s\n", strerror(errno));
+    if (reopen_device(ao, false) < 0) {
+        MP_ERR(ao, "Fatal error: *** CANNOT RE-OPEN / RESET AUDIO DEVICE ***\n");
         return;
     }
-
-#if defined(FD_CLOEXEC) && defined(F_SETFD)
-    fcntl(p->audio_fd, F_SETFD, FD_CLOEXEC);
-#endif
-
-    int oss_format = format2oss(ao->format);
-    if (AF_FORMAT_IS_AC3(ao->format))
-        ioctl(p->audio_fd, SNDCTL_DSP_SPEED, &ao->samplerate);
-    ioctl(p->audio_fd, SNDCTL_DSP_SETFMT, &oss_format);
-    if (!AF_FORMAT_IS_AC3(ao->format)) {
-        int c = ao->channels.num;
-        if (ao->channels.num > 2)
-            ioctl(p->audio_fd, SNDCTL_DSP_CHANNELS, &c);
-        else {
-            c--;
-            ioctl(p->audio_fd, SNDCTL_DSP_STEREO, &c);
-        }
-        ioctl(p->audio_fd, SNDCTL_DSP_SPEED, &ao->samplerate);
-    }
 #endif
 }