From 1a28d541a4e59898b5dae074e451db4248f735e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Brzezi=C5=84ski?= Date: Tue, 3 Sep 2024 17:00:09 +0200 Subject: [PATCH] vtenc: Restart encoding session when certain errors are detected Sometimes under certain loads, VT can error out with kVTVideoEncoderMalfunctionErr or kVTVideoEncoderNotAvailableNowErr. These have been reported to happen more often than usual if CopyProperty/SetProperty() is used close to the encode call. Both can be worked around by restarting the encoding session. These errors can be returned either directly from VTCompressionSessionEncodeFrame() or later in the encoding callback. This patch handles both scenarios the same way - a session restart is be attempted on the next encode_frame() call. If the error is returned immediately by the encode call, it's possible that some correct frames will still be given to the output callback, but for simplicity (+ because I wasn't able to verify this scenario) let's just discard those. In addition, this commit also simplifies the beach/drop logic in enqueue_buffer. Related bug reports in other projects: http://www.openradar.me/45889262 https://github.com/aws/amazon-chime-sdk-ios/issues/170#issuecomment-741908622 Part-of: --- .../gst-plugins-bad/sys/applemedia/vtenc.c | 117 +++++++++++++----- .../gst-plugins-bad/sys/applemedia/vtenc.h | 4 + 2 files changed, 92 insertions(+), 29 deletions(-) diff --git a/subprojects/gst-plugins-bad/sys/applemedia/vtenc.c b/subprojects/gst-plugins-bad/sys/applemedia/vtenc.c index ce5c6024d0..6b8f9cc9b7 100644 --- a/subprojects/gst-plugins-bad/sys/applemedia/vtenc.c +++ b/subprojects/gst-plugins-bad/sys/applemedia/vtenc.c @@ -151,7 +151,7 @@ static void gst_vtenc_finalize (GObject * obj); static gboolean gst_vtenc_start (GstVideoEncoder * enc); static gboolean gst_vtenc_stop (GstVideoEncoder * enc); -static void gst_vtenc_loop (GstVTEnc * self); +static void gst_vtenc_output_loop (GstVTEnc * self); static gboolean gst_vtenc_set_format (GstVideoEncoder * enc, GstVideoCodecState * input_state); static GstFlowReturn gst_vtenc_handle_frame (GstVideoEncoder * enc, @@ -780,6 +780,7 @@ gst_vtenc_start (GstVideoEncoder * enc) self->is_flushing = FALSE; self->downstream_ret = GST_FLOW_OK; + g_atomic_int_set (&self->require_restart, FALSE); self->output_queue = gst_queue_array_new (VTENC_OUTPUT_QUEUE_SIZE); /* Set clear_func to unref all remaining frames in gst_queue_array_free() */ @@ -789,7 +790,7 @@ gst_vtenc_start (GstVideoEncoder * enc) /* Create the output task, but pause it immediately */ self->pause_task = TRUE; if (!gst_pad_start_task (GST_VIDEO_ENCODER_SRC_PAD (enc), - (GstTaskFunction) gst_vtenc_loop, self, NULL)) { + (GstTaskFunction) gst_vtenc_output_loop, self, NULL)) { GST_ERROR_OBJECT (self, "failed to start output thread"); return FALSE; } @@ -1545,6 +1546,7 @@ beach: static void gst_vtenc_destroy_session (GstVTEnc * self, VTCompressionSessionRef * session) { + GST_DEBUG_OBJECT (self, "Destroying VT session"); VTCompressionSessionInvalidate (*session); if (*session != NULL) { CFRelease (*session); @@ -1753,6 +1755,49 @@ gst_vtenc_update_timestamps (GstVTEnc * self, GstVideoCodecFrame * frame, } } +static Boolean +gst_vtenc_is_recoverable_error (OSStatus status) +{ + return status == kVTVideoEncoderMalfunctionErr + || status == kVTVideoEncoderNotAvailableNowErr; +} + +static void +gst_vtenc_restart_session (GstVTEnc * self) +{ + OSStatus status; + VTCompressionSessionRef session; + + /* We need to push out all frames still inside the encoder, + * otherwise destroy_session() will wait for all callbacks to fire + * and very likely deadlock due to the object lock being taken */ + GST_VIDEO_ENCODER_STREAM_UNLOCK (self); + status = VTCompressionSessionCompleteFrames (self->session, + kCMTimePositiveInfinity); + if (status != noErr) { + GST_WARNING_OBJECT (self, + "Error when emptying encoder before restart: %d, will retry on next frame encode", + (int) status); + GST_VIDEO_ENCODER_STREAM_LOCK (self); + return; + } else { + GST_DEBUG_OBJECT (self, "All frames out, restarting encoder session"); + } + GST_VIDEO_ENCODER_STREAM_LOCK (self); + + GST_OBJECT_LOCK (self); + gst_vtenc_destroy_session (self, &self->session); + GST_OBJECT_UNLOCK (self); + + session = gst_vtenc_create_session (self); + + GST_OBJECT_LOCK (self); + self->session = session; + GST_OBJECT_UNLOCK (self); + + g_atomic_int_set (&self->require_restart, FALSE); +} + static GstFlowReturn gst_vtenc_encode_frame (GstVTEnc * self, GstVideoCodecFrame * frame) { @@ -1802,6 +1847,11 @@ gst_vtenc_encode_frame (GstVTEnc * self, GstVideoCodecFrame * frame) } } + /* Flushes all remaining frames out of the encoder + * and recreates the encoding session. */ + if (g_atomic_int_get (&self->require_restart)) + gst_vtenc_restart_session (self); + if (GST_VIDEO_CODEC_FRAME_IS_FORCE_KEYFRAME (frame)) { GST_INFO_OBJECT (self, "received force-keyframe-event, will force intra"); frame_props = self->keyframe_props; @@ -1960,9 +2010,16 @@ gst_vtenc_encode_frame (GstVTEnc * self, GstVideoCodecFrame * frame) GINT_TO_POINTER (frame->system_frame_number), NULL); GST_VIDEO_ENCODER_STREAM_LOCK (self); - if (vt_status != noErr) { - GST_WARNING_OBJECT (self, "VTCompressionSessionEncodeFrame returned %d", - (int) vt_status); + if (gst_vtenc_is_recoverable_error (vt_status)) { + GST_ELEMENT_WARNING (self, LIBRARY, ENCODE, (NULL), + ("Failed to encode frame %d: %d, restarting session on next frame encode", + frame->system_frame_number, (int) vt_status)); + + g_atomic_int_set (&self->require_restart, TRUE); + } else if (vt_status != noErr) { + GST_ELEMENT_ERROR (self, LIBRARY, ENCODE, (NULL), + ("Failed to encode frame %d: %d", frame->system_frame_number, + (int) vt_status)); } gst_video_codec_frame_unref (frame); @@ -1991,14 +2048,23 @@ gst_vtenc_enqueue_buffer (void *outputCallbackRefCon, { GstVTEnc *self = outputCallbackRefCon; GstVideoCodecFrame *frame; - gboolean is_flushing; frame = gst_video_encoder_get_frame (GST_VIDEO_ENCODER_CAST (self), GPOINTER_TO_INT (sourceFrameRefCon)); + if (g_atomic_int_get (&self->require_restart)) { + GST_DEBUG_OBJECT (self, "Ignoring frame because of scheduled restart"); + goto drop; + } + if (status != noErr) { - if (frame) { + if (gst_vtenc_is_recoverable_error (status)) { + GST_ELEMENT_WARNING (self, LIBRARY, ENCODE, (NULL), + ("Failed to encode frame (%d), restarting session on next frame encode", + (int) status)); + g_atomic_int_set (&self->require_restart, TRUE); + } else if (frame) { GST_ELEMENT_ERROR (self, LIBRARY, ENCODE, (NULL), ("Failed to encode frame %d: %d", frame->system_frame_number, (int) status)); @@ -2006,26 +2072,29 @@ gst_vtenc_enqueue_buffer (void *outputCallbackRefCon, GST_ELEMENT_ERROR (self, LIBRARY, ENCODE, (NULL), ("Failed to encode (frame unknown): %d", (int) status)); } - goto beach; + + goto drop; } if (!frame) { GST_WARNING_OBJECT (self, "No corresponding frame found!"); - goto beach; + return; } g_mutex_lock (&self->queue_mutex); - is_flushing = self->is_flushing; - g_mutex_unlock (&self->queue_mutex); - if (is_flushing) { + if (self->is_flushing) { GST_DEBUG_OBJECT (self, "Ignoring frame %d because we're flushing", frame->system_frame_number); - goto beach; + + gst_video_codec_frame_unref (frame); + g_mutex_unlock (&self->queue_mutex); + return; } + g_mutex_unlock (&self->queue_mutex); /* This may happen if we don't have enough bitrate */ if (sampleBuffer == NULL) - goto beach; + goto drop; if (gst_vtenc_buffer_is_keyframe (self, sampleBuffer)) GST_VIDEO_CODEC_FRAME_SET_SYNC_POINT (frame); @@ -2043,28 +2112,18 @@ gst_vtenc_enqueue_buffer (void *outputCallbackRefCon, VTENC_OUTPUT_QUEUE_SIZE) { g_cond_wait (&self->queue_cond, &self->queue_mutex); } - g_mutex_unlock (&self->queue_mutex); -beach: - if (!frame) - return; - - g_mutex_lock (&self->queue_mutex); - if (self->is_flushing) { - /* We can discard the frame here, no need to have the output loop do that */ - gst_video_codec_frame_unref (frame); - g_mutex_unlock (&self->queue_mutex); - return; - } - - /* Buffer-less frames will be discarded in the output loop */ gst_queue_array_push_tail (self->output_queue, frame); g_cond_signal (&self->queue_cond); g_mutex_unlock (&self->queue_mutex); + return; + +drop: + gst_video_codec_frame_unref (frame); } static void -gst_vtenc_loop (GstVTEnc * self) +gst_vtenc_output_loop (GstVTEnc * self) { GstVideoCodecFrame *outframe; GstCoreMediaMeta *meta; diff --git a/subprojects/gst-plugins-bad/sys/applemedia/vtenc.h b/subprojects/gst-plugins-bad/sys/applemedia/vtenc.h index ebeccfb625..cfd4e12bd1 100644 --- a/subprojects/gst-plugins-bad/sys/applemedia/vtenc.h +++ b/subprojects/gst-plugins-bad/sys/applemedia/vtenc.h @@ -92,6 +92,10 @@ struct _GstVTEnc gboolean negotiate_downstream; gboolean is_flushing; gboolean pause_task; + + /* If we get an EncoderMalfunctionErr or similar, we restart the session + * before the next encode call */ + gboolean require_restart; }; void gst_vtenc_register_elements (GstPlugin * plugin);