From 9cbe9a52fedd3adbc23763c54fc01529a19cfb3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Brzezi=C5=84ski?= Date: Fri, 8 Sep 2023 00:26:51 +0200 Subject: [PATCH] vtenc: Fix deadlock after GST_FLOW_ERROR is received on frame push This was easy to trigger when testing with e.g. vtenc ! vtdec ! glimagesink and closing the sink via window button, causing GST_FLOW_ERROR to be received by the output loop, stopping it with the queue still full. This made the enqueue_buffer() callback to lock waiting for space in our queue, while handle_frame() was waiting for the internal VideoToolbox queue to free up, so that VTCompressionSessionEncodeFrame could finish. As the output loop was not running, both functions waited forever. Fixed by 1) immediately emptying our queue when GST_FLOW_ERROR is received (like we already did with _FLUSHING) and 2) unconditionally setting the flushing flag in finish_encoding() when it sees the output loop stopped because of GST_FLOW_ERROR, so that enqueue_buffer() will immediately discard any new frames coming out of VideoToolbox. Both of those make sure we never run into the both-queues-full scenario. Part-of: --- .../gst-plugins-bad/sys/applemedia/vtenc.c | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/subprojects/gst-plugins-bad/sys/applemedia/vtenc.c b/subprojects/gst-plugins-bad/sys/applemedia/vtenc.c index 064eaae2ab..4c2fb147b5 100644 --- a/subprojects/gst-plugins-bad/sys/applemedia/vtenc.c +++ b/subprojects/gst-plugins-bad/sys/applemedia/vtenc.c @@ -698,6 +698,15 @@ gst_vtenc_pause_output_loop (GstVTEnc * self) g_mutex_unlock (&self->queue_mutex); } +static void +gst_vtenc_set_flushing_flag (GstVTEnc * self) +{ + g_mutex_lock (&self->queue_mutex); + self->is_flushing = TRUE; + g_cond_signal (&self->queue_cond); + g_mutex_unlock (&self->queue_mutex); +} + static GstFlowReturn gst_vtenc_finish_encoding (GstVTEnc * self, gboolean is_flushing) { @@ -712,17 +721,15 @@ gst_vtenc_finish_encoding (GstVTEnc * self, gboolean is_flushing) /* If output loop failed to push things downstream */ if (self->downstream_ret != GST_FLOW_OK && self->downstream_ret != GST_FLOW_FLUSHING) { + /* Tells enqueue_buffer() to instantly discard any new encoded frames */ + gst_vtenc_set_flushing_flag (self); GST_WARNING_OBJECT (self, "Output loop stopped with error (%s), leaving", gst_flow_get_name (self->downstream_ret)); return self->downstream_ret; } - if (is_flushing) { - g_mutex_lock (&self->queue_mutex); - self->is_flushing = TRUE; - g_cond_signal (&self->queue_cond); - g_mutex_unlock (&self->queue_mutex); - } + if (is_flushing) + gst_vtenc_set_flushing_flag (self); if (!gst_vtenc_ensure_output_loop (self)) { GST_ERROR_OBJECT (self, "Output loop failed to resume"); @@ -2107,7 +2114,7 @@ gst_vtenc_loop (GstVTEnc * self) /* We need to empty the queue immediately so that enqueue_buffer() * can push out the current buffer, otherwise it can block other * encoder callbacks completely */ - if (ret == GST_FLOW_FLUSHING) { + if (ret != GST_FLOW_OK) { g_mutex_lock (&self->queue_mutex); while ((outframe = gst_queue_array_pop_head (self->output_queue))) {