From d86a6715e1d4402c699644fd94c645bdd91732b9 Mon Sep 17 00:00:00 2001 From: Xavier Claessens Date: Wed, 14 Feb 2024 11:44:22 -0500 Subject: [PATCH] aja: Replace global semaphore with per-device flock() The global semaphore was never closed/unlinked, causing permission denied issue if the device is later used by another user. Properly removing the semaphore when stopping the pipeline would still leave it open in case of a crash. With a GStreamer specific name, it was also not preventing other apps to access the device concurrently. Finally, if the system has multiple cards, the lock should be per card and not global (to be confirmed). Fixes: #3283. Sponsored-by: Netflix Inc. Part-of: --- .../gst-plugins-bad/sys/aja/gstajacommon.cpp | 45 +++++++++---------- .../gst-plugins-bad/sys/aja/gstajacommon.h | 9 ++-- .../gst-plugins-bad/sys/aja/gstajasink.cpp | 6 +-- .../gst-plugins-bad/sys/aja/gstajasrc.cpp | 6 +-- 4 files changed, 32 insertions(+), 34 deletions(-) diff --git a/subprojects/gst-plugins-bad/sys/aja/gstajacommon.cpp b/subprojects/gst-plugins-bad/sys/aja/gstajacommon.cpp index bd0e5fd41b..8b749a017b 100644 --- a/subprojects/gst-plugins-bad/sys/aja/gstajacommon.cpp +++ b/subprojects/gst-plugins-bad/sys/aja/gstajacommon.cpp @@ -22,7 +22,7 @@ #endif #include -#include +#include #include "gstajacommon.h" @@ -713,8 +713,20 @@ GstAjaNtv2Device *gst_aja_ntv2_device_obtain(const gchar *device_identifier) { return NULL; } + gchar *path = g_strdup_printf("/dev/ajantv2%d", device->GetIndexNumber()); + int fd = open(path, O_RDONLY); + if (fd < 0) { + GST_ERROR("Failed to open device node %s: %s", path, g_strerror(errno)); + delete device; + g_free(path); + return NULL; + } + GstAjaNtv2Device *dev = g_atomic_rc_box_new0(GstAjaNtv2Device); dev->device = device; + dev->fd = fd; + + g_free(path); return dev; } @@ -728,35 +740,18 @@ void gst_aja_ntv2_device_unref(GstAjaNtv2Device *device) { GstAjaNtv2Device *dev = (GstAjaNtv2Device *)data; delete dev->device; + close(dev->fd); }); } -static gpointer init_setup_mutex(gpointer data) { - sem_t *s = SEM_FAILED; - s = sem_open("/gstreamer-aja-sem", O_CREAT, S_IRUSR | S_IWUSR, 1); - if (s == SEM_FAILED) { - g_critical("Failed to create SHM semaphore for GStreamer AJA plugin: %s", - g_strerror(errno)); - } - return s; +GstAjaNtv2DeviceLocker::GstAjaNtv2DeviceLocker(GstAjaNtv2Device *device) { + _device = gst_aja_ntv2_device_ref(device); + flock(_device->fd, LOCK_EX); } -static sem_t *get_setup_mutex(void) { - static GOnce once = G_ONCE_INIT; - - g_once(&once, init_setup_mutex, NULL); - - return (sem_t *)once.retval; -} - -ShmMutexLocker::ShmMutexLocker() { - sem_t *s = get_setup_mutex(); - if (s != SEM_FAILED) sem_wait(s); -} - -ShmMutexLocker::~ShmMutexLocker() { - sem_t *s = get_setup_mutex(); - if (s != SEM_FAILED) sem_post(s); +GstAjaNtv2DeviceLocker::~GstAjaNtv2DeviceLocker() { + flock(_device->fd, LOCK_UN); + gst_aja_ntv2_device_unref(_device); } static guint gst_aja_device_get_frame_multiplier(GstAjaNtv2Device *device, diff --git a/subprojects/gst-plugins-bad/sys/aja/gstajacommon.h b/subprojects/gst-plugins-bad/sys/aja/gstajacommon.h index 1629cf08a2..ed1c638bf8 100644 --- a/subprojects/gst-plugins-bad/sys/aja/gstajacommon.h +++ b/subprojects/gst-plugins-bad/sys/aja/gstajacommon.h @@ -56,6 +56,7 @@ GstAjaAudioMeta *gst_buffer_add_aja_audio_meta(GstBuffer *buffer, typedef struct { CNTV2Card *device; + int fd; } GstAjaNtv2Device; G_GNUC_INTERNAL @@ -319,10 +320,12 @@ void gst_aja_common_init(void); G_END_DECLS -class ShmMutexLocker { +class GstAjaNtv2DeviceLocker { public: - ShmMutexLocker(); - ~ShmMutexLocker(); + GstAjaNtv2DeviceLocker(GstAjaNtv2Device *device); + ~GstAjaNtv2DeviceLocker(); + private: + GstAjaNtv2Device *_device; }; G_GNUC_INTERNAL diff --git a/subprojects/gst-plugins-bad/sys/aja/gstajasink.cpp b/subprojects/gst-plugins-bad/sys/aja/gstajasink.cpp index f071616a24..959856bdc0 100644 --- a/subprojects/gst-plugins-bad/sys/aja/gstajasink.cpp +++ b/subprojects/gst-plugins-bad/sys/aja/gstajasink.cpp @@ -724,7 +724,7 @@ static gboolean gst_aja_sink_set_caps(GstBaseSink *bsink, GstCaps *caps) { // Make sure to globally lock here as the routing settings and others are // global shared state - ShmMutexLocker locker; + GstAjaNtv2DeviceLocker locker(self->device); if (!::NTV2DeviceCanDoVideoFormat(self->device_id, video_format)) { GST_ERROR_OBJECT(self, "Device does not support mode %d", @@ -1931,7 +1931,7 @@ restart: { // Make sure to globally lock here as the routing settings and others are // global shared state - ShmMutexLocker locker; + GstAjaNtv2DeviceLocker locker(self->device); self->device->device->AutoCirculateStop(self->channel); @@ -2238,7 +2238,7 @@ restart: out: { // Make sure to globally lock here as the routing settings and others are // global shared state - ShmMutexLocker locker; + GstAjaNtv2DeviceLocker locker(self->device); self->device->device->AutoCirculateStop(self->channel); self->device->device->UnsubscribeOutputVerticalEvent(self->channel); diff --git a/subprojects/gst-plugins-bad/sys/aja/gstajasrc.cpp b/subprojects/gst-plugins-bad/sys/aja/gstajasrc.cpp index 9655ca526d..012c0ab8f7 100644 --- a/subprojects/gst-plugins-bad/sys/aja/gstajasrc.cpp +++ b/subprojects/gst-plugins-bad/sys/aja/gstajasrc.cpp @@ -623,7 +623,7 @@ static gboolean gst_aja_src_close(GstAjaSrc *self) { return TRUE; } -// Must be called with ShmMutexLocker +// Must be called with GstAjaNtv2DeviceLocker static gboolean gst_aja_src_configure(GstAjaSrc *self) { GST_DEBUG_OBJECT(self, "Starting"); @@ -2301,7 +2301,7 @@ restart: // Make sure to globally lock here as the routing settings and others are // global shared state - ShmMutexLocker locker; + GstAjaNtv2DeviceLocker locker(self->device); if (!gst_aja_src_configure(self)) { g_mutex_lock(&self->queue_lock); @@ -2788,7 +2788,7 @@ restart: out: { // Make sure to globally lock here as the routing settings and others are // global shared state - ShmMutexLocker locker; + GstAjaNtv2DeviceLocker locker(self->device); self->device->device->AutoCirculateStop(self->channel); self->device->device->UnsubscribeInputVerticalEvent(self->channel);