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/5205>
This commit is contained in:
Jan Schmidt 2023-08-19 01:00:16 +10:00 committed by GStreamer Marge Bot
parent 1b0b1fc0fd
commit 6053650b85
4 changed files with 66 additions and 8 deletions

View file

@ -8165,6 +8165,21 @@ be called in when the ringbuffer is acquired.</doc>
</parameter> </parameter>
</parameters> </parameters>
</method> </method>
<method name="set_errored" c:identifier="gst_audio_ring_buffer_set_errored" version="1.24">
<doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioringbuffer.c">Mark the ringbuffer as errored after it has started.
MT safe.</doc>
<source-position filename="../subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioringbuffer.h"/>
<return-value transfer-ownership="none">
<type name="none" c:type="void"/>
</return-value>
<parameters>
<instance-parameter name="buf" transfer-ownership="none">
<doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioringbuffer.c">the #GstAudioRingBuffer that has encountered an error</doc>
<type name="AudioRingBuffer" c:type="GstAudioRingBuffer*"/>
</instance-parameter>
</parameters>
</method>
<method name="set_flushing" c:identifier="gst_audio_ring_buffer_set_flushing"> <method name="set_flushing" c:identifier="gst_audio_ring_buffer_set_flushing">
<doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioringbuffer.c">Set the ringbuffer to flushing mode or normal mode. <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioringbuffer.c">Set the ringbuffer to flushing mode or normal mode.

View file

@ -1229,7 +1229,7 @@ gst_audio_base_src_post_message (GstElement * element, GstMessage * message)
* flow error message */ * flow error message */
ret = GST_ELEMENT_CLASS (parent_class)->post_message (element, 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_AUDIO_RING_BUFFER_SIGNAL (ringbuffer);
gst_object_unref (ringbuffer); gst_object_unref (ringbuffer);
} else { } else {

View file

@ -82,7 +82,7 @@ gst_audio_ring_buffer_init (GstAudioRingBuffer * ringbuffer)
{ {
ringbuffer->open = FALSE; ringbuffer->open = FALSE;
ringbuffer->acquired = 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); g_cond_init (&ringbuffer->cond);
ringbuffer->waiting = 0; ringbuffer->waiting = 0;
ringbuffer->empty_seg = NULL; ringbuffer->empty_seg = NULL;
@ -1066,7 +1066,7 @@ gst_audio_ring_buffer_start (GstAudioRingBuffer * buf)
} }
if (G_UNLIKELY (!res)) { 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"); GST_DEBUG_OBJECT (buf, "failed to start");
} else { } else {
GST_DEBUG_OBJECT (buf, "started"); GST_DEBUG_OBJECT (buf, "started");
@ -1097,6 +1097,38 @@ may_not_start:
} }
} }
/**
* 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
*/
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 static gboolean
gst_audio_ring_buffer_pause_unlocked (GstAudioRingBuffer * buf) gst_audio_ring_buffer_pause_unlocked (GstAudioRingBuffer * buf)
{ {
@ -1121,7 +1153,8 @@ gst_audio_ring_buffer_pause_unlocked (GstAudioRingBuffer * buf)
res = rclass->pause (buf); res = rclass->pause (buf);
if (G_UNLIKELY (!res)) { 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"); GST_DEBUG_OBJECT (buf, "failed to pause");
} else { } else {
GST_DEBUG_OBJECT (buf, "paused"); GST_DEBUG_OBJECT (buf, "paused");
@ -1132,7 +1165,7 @@ gst_audio_ring_buffer_pause_unlocked (GstAudioRingBuffer * buf)
not_started: not_started:
{ {
/* was 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; return TRUE;
} }
} }
@ -1214,9 +1247,16 @@ gst_audio_ring_buffer_stop (GstAudioRingBuffer * buf)
GST_AUDIO_RING_BUFFER_STATE_PAUSED, GST_AUDIO_RING_BUFFER_STATE_PAUSED,
GST_AUDIO_RING_BUFFER_STATE_STOPPED); GST_AUDIO_RING_BUFFER_STATE_STOPPED);
if (!res) { 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; 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; goto done;
} }
} }
@ -1230,7 +1270,7 @@ gst_audio_ring_buffer_stop (GstAudioRingBuffer * buf)
res = rclass->stop (buf); res = rclass->stop (buf);
if (G_UNLIKELY (!res)) { 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"); GST_DEBUG_OBJECT (buf, "failed to stop");
} else { } else {
GST_DEBUG_OBJECT (buf, "stopped"); GST_DEBUG_OBJECT (buf, "stopped");

View file

@ -379,6 +379,9 @@ gboolean gst_audio_ring_buffer_pause (GstAudioRingBuffer *buf);
GST_AUDIO_API GST_AUDIO_API
gboolean gst_audio_ring_buffer_stop (GstAudioRingBuffer *buf); gboolean gst_audio_ring_buffer_stop (GstAudioRingBuffer *buf);
GST_AUDIO_API
void gst_audio_ring_buffer_set_errored (GstAudioRingBuffer *buf);
/* get status */ /* get status */
GST_AUDIO_API GST_AUDIO_API