audio: Make sure to stop ringbuffer on error

Add gst_audio_ring_buffer_set_errored() that will mark the
ringbuffer as errored only if it is currently started or paused,
so gst_audio_ringbuffer_stop() can be sure that the error
state means that the ringbuffer was started and needs stop called.

Fixes a crash with osxaudiosrc if the source element posts
an error, because the ringbuffer would not get stopped and CoreAudio
would continue trying to do callbacks.

Also, anywhere that modifies the ringbuffer state, make sure to
use atomic operations, to guarantee their visibility

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5216>

diff --git a/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudiobasesrc.c b/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudiobasesrc.c
index 0dd7654e036a7bc18a742c7b0f36cad2787ca964..104570dec12552dedca62d6b64cc03b5cba13d18 100644 (file)
--- a/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudiobasesrc.c
+++ b/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudiobasesrc.c
@@ -45,6 +45,10 @@
 GST_DEBUG_CATEGORY_STATIC (gst_audio_base_src_debug);
 #define GST_CAT_DEFAULT gst_audio_base_src_debug
 
+/* This function is public in >= 1.23, but internal in 1.22 */
+G_GNUC_INTERNAL
+    void __gst_audio_ring_buffer_set_errored (GstAudioRingBuffer * buf);
+
 struct _GstAudioBaseSrcPrivate
 {
   /* the clock slaving algorithm in use */
@@ -1229,7 +1233,7 @@ gst_audio_base_src_post_message (GstElement * element, GstMessage * message)
      * flow error message */
     ret = GST_ELEMENT_CLASS (parent_class)->post_message (element, message);
 
-    g_atomic_int_set (&ringbuffer->state, GST_AUDIO_RING_BUFFER_STATE_ERROR);
+    __gst_audio_ring_buffer_set_errored (ringbuffer);
     GST_AUDIO_RING_BUFFER_SIGNAL (ringbuffer);
     gst_object_unref (ringbuffer);
   } else {

diff --git a/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioringbuffer.c b/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioringbuffer.c
index ac71dd06a75e08d4bf2046456f55a6a17f887ebb..d718b85d44d9c49a473141e06df4c4444d65733a 100644 (file)
--- a/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioringbuffer.c
+++ b/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioringbuffer.c
@@ -81,7 +81,7 @@ gst_audio_ring_buffer_init (GstAudioRingBuffer * ringbuffer)
 {
   ringbuffer->open = FALSE;
   ringbuffer->acquired = FALSE;
-  ringbuffer->state = GST_AUDIO_RING_BUFFER_STATE_STOPPED;
+  g_atomic_int_set (&ringbuffer->state, GST_AUDIO_RING_BUFFER_STATE_STOPPED);
   g_cond_init (&ringbuffer->cond);
   ringbuffer->waiting = 0;
   ringbuffer->empty_seg = NULL;
@@ -1005,7 +1005,7 @@ gst_audio_ring_buffer_start (GstAudioRingBuffer * buf)
   }
 
   if (G_UNLIKELY (!res)) {
-    buf->state = GST_AUDIO_RING_BUFFER_STATE_PAUSED;
+    g_atomic_int_set (&buf->state, GST_AUDIO_RING_BUFFER_STATE_PAUSED);
     GST_DEBUG_OBJECT (buf, "failed to start");
   } else {
     GST_DEBUG_OBJECT (buf, "started");
@@ -1036,6 +1036,40 @@ may_not_start:
   }
 }
 
+G_GNUC_INTERNAL
+    void __gst_audio_ring_buffer_set_errored (GstAudioRingBuffer * buf);
+
+/* __gst_audio_ring_buffer_set_errored:
+ * @buf: the #GstAudioRingBuffer that has encountered an error
+ *
+ * Mark the ringbuffer as errored after it has started.
+ *
+ * MT safe.
+
+ * Since: 1.24 (internal in 1.22)
+ */
+void
+__gst_audio_ring_buffer_set_errored (GstAudioRingBuffer * buf)
+{
+  gboolean res;
+
+  /* If started set to errored */
+  res = g_atomic_int_compare_and_exchange (&buf->state,
+      GST_AUDIO_RING_BUFFER_STATE_STARTED, GST_AUDIO_RING_BUFFER_STATE_ERROR);
+  if (!res) {
+    GST_DEBUG_OBJECT (buf, "ringbuffer was not started, checking paused");
+    res = g_atomic_int_compare_and_exchange (&buf->state,
+        GST_AUDIO_RING_BUFFER_STATE_PAUSED, GST_AUDIO_RING_BUFFER_STATE_ERROR);
+  }
+  if (res) {
+    GST_DEBUG_OBJECT (buf, "ringbuffer is errored");
+  } else {
+    GST_DEBUG_OBJECT (buf,
+        "Could not mark ringbuffer as errored. It must have been stopped or already errored (was state %d)",
+        g_atomic_int_get (&buf->state));
+  }
+}
+
 static gboolean
 gst_audio_ring_buffer_pause_unlocked (GstAudioRingBuffer * buf)
 {
@@ -1060,7 +1094,8 @@ gst_audio_ring_buffer_pause_unlocked (GstAudioRingBuffer * buf)
     res = rclass->pause (buf);
 
   if (G_UNLIKELY (!res)) {
-    buf->state = GST_AUDIO_RING_BUFFER_STATE_STARTED;
+    /* Restore started state */
+    g_atomic_int_set (&buf->state, GST_AUDIO_RING_BUFFER_STATE_STARTED);
     GST_DEBUG_OBJECT (buf, "failed to pause");
   } else {
     GST_DEBUG_OBJECT (buf, "paused");
@@ -1071,7 +1106,7 @@ gst_audio_ring_buffer_pause_unlocked (GstAudioRingBuffer * buf)
 not_started:
   {
     /* was not started */
-    GST_DEBUG_OBJECT (buf, "was not started");
+    GST_DEBUG_OBJECT (buf, "was not started (state %d)", buf->state);
     return TRUE;
   }
 }
@@ -1153,9 +1188,16 @@ gst_audio_ring_buffer_stop (GstAudioRingBuffer * buf)
         GST_AUDIO_RING_BUFFER_STATE_PAUSED,
         GST_AUDIO_RING_BUFFER_STATE_STOPPED);
     if (!res) {
-      /* was not paused either, must have been stopped then */
+      GST_DEBUG_OBJECT (buf, "was not paused, try errored");
+      res = g_atomic_int_compare_and_exchange (&buf->state,
+          GST_AUDIO_RING_BUFFER_STATE_ERROR,
+          GST_AUDIO_RING_BUFFER_STATE_STOPPED);
+    }
+    if (!res) {
+      /* was not paused or stopped either, must have been stopped then */
       res = TRUE;
-      GST_DEBUG_OBJECT (buf, "was not paused, must have been stopped");
+      GST_DEBUG_OBJECT (buf,
+          "was not paused or errored, must have been stopped");
       goto done;
     }
   }
@@ -1169,7 +1211,7 @@ gst_audio_ring_buffer_stop (GstAudioRingBuffer * buf)
     res = rclass->stop (buf);
 
   if (G_UNLIKELY (!res)) {
-    buf->state = GST_AUDIO_RING_BUFFER_STATE_STARTED;
+    g_atomic_int_set (&buf->state, GST_AUDIO_RING_BUFFER_STATE_STARTED);
     GST_DEBUG_OBJECT (buf, "failed to stop");
   } else {
     GST_DEBUG_OBJECT (buf, "stopped");