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: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5303>
This commit is contained in:
Piotr Brzeziński 2023-09-08 00:26:51 +02:00 committed by GStreamer Marge Bot
parent d9a89cce06
commit 9cbe9a52fe

View file

@ -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))) {