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: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/6117>
This commit is contained in:
Xavier Claessens 2024-02-14 11:44:22 -05:00 committed by GStreamer Marge Bot
parent d26194db0e
commit d86a6715e1
4 changed files with 32 additions and 34 deletions

View file

@ -22,7 +22,7 @@
#endif #endif
#include <fcntl.h> #include <fcntl.h>
#include <semaphore.h> #include <sys/file.h>
#include "gstajacommon.h" #include "gstajacommon.h"
@ -713,8 +713,20 @@ GstAjaNtv2Device *gst_aja_ntv2_device_obtain(const gchar *device_identifier) {
return NULL; 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); GstAjaNtv2Device *dev = g_atomic_rc_box_new0(GstAjaNtv2Device);
dev->device = device; dev->device = device;
dev->fd = fd;
g_free(path);
return dev; return dev;
} }
@ -728,35 +740,18 @@ void gst_aja_ntv2_device_unref(GstAjaNtv2Device *device) {
GstAjaNtv2Device *dev = (GstAjaNtv2Device *)data; GstAjaNtv2Device *dev = (GstAjaNtv2Device *)data;
delete dev->device; delete dev->device;
close(dev->fd);
}); });
} }
static gpointer init_setup_mutex(gpointer data) { GstAjaNtv2DeviceLocker::GstAjaNtv2DeviceLocker(GstAjaNtv2Device *device) {
sem_t *s = SEM_FAILED; _device = gst_aja_ntv2_device_ref(device);
s = sem_open("/gstreamer-aja-sem", O_CREAT, S_IRUSR | S_IWUSR, 1); flock(_device->fd, LOCK_EX);
if (s == SEM_FAILED) {
g_critical("Failed to create SHM semaphore for GStreamer AJA plugin: %s",
g_strerror(errno));
}
return s;
} }
static sem_t *get_setup_mutex(void) { GstAjaNtv2DeviceLocker::~GstAjaNtv2DeviceLocker() {
static GOnce once = G_ONCE_INIT; flock(_device->fd, LOCK_UN);
gst_aja_ntv2_device_unref(_device);
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);
} }
static guint gst_aja_device_get_frame_multiplier(GstAjaNtv2Device *device, static guint gst_aja_device_get_frame_multiplier(GstAjaNtv2Device *device,

View file

@ -56,6 +56,7 @@ GstAjaAudioMeta *gst_buffer_add_aja_audio_meta(GstBuffer *buffer,
typedef struct { typedef struct {
CNTV2Card *device; CNTV2Card *device;
int fd;
} GstAjaNtv2Device; } GstAjaNtv2Device;
G_GNUC_INTERNAL G_GNUC_INTERNAL
@ -319,10 +320,12 @@ void gst_aja_common_init(void);
G_END_DECLS G_END_DECLS
class ShmMutexLocker { class GstAjaNtv2DeviceLocker {
public: public:
ShmMutexLocker(); GstAjaNtv2DeviceLocker(GstAjaNtv2Device *device);
~ShmMutexLocker(); ~GstAjaNtv2DeviceLocker();
private:
GstAjaNtv2Device *_device;
}; };
G_GNUC_INTERNAL G_GNUC_INTERNAL

View file

@ -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 // Make sure to globally lock here as the routing settings and others are
// global shared state // global shared state
ShmMutexLocker locker; GstAjaNtv2DeviceLocker locker(self->device);
if (!::NTV2DeviceCanDoVideoFormat(self->device_id, video_format)) { if (!::NTV2DeviceCanDoVideoFormat(self->device_id, video_format)) {
GST_ERROR_OBJECT(self, "Device does not support mode %d", 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 // Make sure to globally lock here as the routing settings and others are
// global shared state // global shared state
ShmMutexLocker locker; GstAjaNtv2DeviceLocker locker(self->device);
self->device->device->AutoCirculateStop(self->channel); self->device->device->AutoCirculateStop(self->channel);
@ -2238,7 +2238,7 @@ restart:
out: { out: {
// Make sure to globally lock here as the routing settings and others are // Make sure to globally lock here as the routing settings and others are
// global shared state // global shared state
ShmMutexLocker locker; GstAjaNtv2DeviceLocker locker(self->device);
self->device->device->AutoCirculateStop(self->channel); self->device->device->AutoCirculateStop(self->channel);
self->device->device->UnsubscribeOutputVerticalEvent(self->channel); self->device->device->UnsubscribeOutputVerticalEvent(self->channel);

View file

@ -623,7 +623,7 @@ static gboolean gst_aja_src_close(GstAjaSrc *self) {
return TRUE; return TRUE;
} }
// Must be called with ShmMutexLocker // Must be called with GstAjaNtv2DeviceLocker
static gboolean gst_aja_src_configure(GstAjaSrc *self) { static gboolean gst_aja_src_configure(GstAjaSrc *self) {
GST_DEBUG_OBJECT(self, "Starting"); GST_DEBUG_OBJECT(self, "Starting");
@ -2301,7 +2301,7 @@ restart:
// Make sure to globally lock here as the routing settings and others are // Make sure to globally lock here as the routing settings and others are
// global shared state // global shared state
ShmMutexLocker locker; GstAjaNtv2DeviceLocker locker(self->device);
if (!gst_aja_src_configure(self)) { if (!gst_aja_src_configure(self)) {
g_mutex_lock(&self->queue_lock); g_mutex_lock(&self->queue_lock);
@ -2788,7 +2788,7 @@ restart:
out: { out: {
// Make sure to globally lock here as the routing settings and others are // Make sure to globally lock here as the routing settings and others are
// global shared state // global shared state
ShmMutexLocker locker; GstAjaNtv2DeviceLocker locker(self->device);
self->device->device->AutoCirculateStop(self->channel); self->device->device->AutoCirculateStop(self->channel);
self->device->device->UnsubscribeInputVerticalEvent(self->channel); self->device->device->UnsubscribeInputVerticalEvent(self->channel);