[libcamera-devel,RFC,09/11] libcamera: media_device: Add functions to lock device for other processes

Message ID 20190414013506.10515-10-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamerea: Add support for exclusive access to cameras between processes
Related show

Commit Message

Niklas Söderlund April 14, 2019, 1:35 a.m. UTC
Add lock() and unlock() which backed by lockf() allows an instance of
libcamera to claim exclusive access to a media device. These functions
is the base to allow multiple users of libcamera to coexist in the
system without stepping on each others toes.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/include/media_device.h |  4 ++++
 src/libcamera/media_device.cpp       | 29 +++++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart April 17, 2019, 11:02 a.m. UTC | #1
Hi Niklas,

Thank you for the patch.

On Sun, Apr 14, 2019 at 03:35:04AM +0200, Niklas Söderlund wrote:
> Add lock() and unlock() which backed by lockf() allows an instance of
> libcamera to claim exclusive access to a media device. These functions
> is the base to allow multiple users of libcamera to coexist in the
> system without stepping on each others toes.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/libcamera/include/media_device.h |  4 ++++
>  src/libcamera/media_device.cpp       | 29 +++++++++++++++++++++++++++-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h
> index 5d28a7bb8214ef4a..e2956549fa201ea3 100644
> --- a/src/libcamera/include/media_device.h
> +++ b/src/libcamera/include/media_device.h
> @@ -64,6 +64,10 @@ private:
>  	bool acquire();
>  	void release();
>  
> +	bool lockOwner_;
> +	bool lock();
> +	void unlock();
> +

We usually put functions first, and variables next.

>  	std::map<unsigned int, MediaObject *> objects_;
>  	MediaObject *object(unsigned int id);
>  	bool addObject(MediaObject *object);
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index 46931f52688f6c82..5cea4e63715125c8 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -62,7 +62,8 @@ LOG_DEFINE_CATEGORY(MediaDevice)
>   * populate() before the media graph can be queried.
>   */
>  MediaDevice::MediaDevice(const std::string &deviceNode)
> -	: deviceNode_(deviceNode), fd_(-1), valid_(false), acquired_(false)
> +	: deviceNode_(deviceNode), fd_(-1), valid_(false), acquired_(false),
> +	  lockOwner_(false)
>  {
>  }
>  
> @@ -421,6 +422,32 @@ void MediaDevice::release()
>  	acquired_ = false;
>  }
>  
> +bool MediaDevice::lock()

Could you please document those two functions, and especially the pre
and post conditions ?

> +{
> +	if (fd_ == -1)
> +		return false;
> +
> +	if (lockOwner_)
> +		return true;
> +
> +	if (lockf(fd_, F_TLOCK, 0))
> +		return false;
> +
> +	lockOwner_ = true;
> +
> +	return true;
> +}
> +
> +void MediaDevice::unlock()
> +{
> +	if (fd_ == -1)
> +		return;
> +
> +	lockOwner_ = false;
> +
> +	lockf(fd_, F_ULOCK, 0);
> +}
> +
>  /**
>   * \var MediaDevice::objects_
>   * \brief Global map of media objects (entities, pads, links) keyed by their

Patch

diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h
index 5d28a7bb8214ef4a..e2956549fa201ea3 100644
--- a/src/libcamera/include/media_device.h
+++ b/src/libcamera/include/media_device.h
@@ -64,6 +64,10 @@  private:
 	bool acquire();
 	void release();
 
+	bool lockOwner_;
+	bool lock();
+	void unlock();
+
 	std::map<unsigned int, MediaObject *> objects_;
 	MediaObject *object(unsigned int id);
 	bool addObject(MediaObject *object);
diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
index 46931f52688f6c82..5cea4e63715125c8 100644
--- a/src/libcamera/media_device.cpp
+++ b/src/libcamera/media_device.cpp
@@ -62,7 +62,8 @@  LOG_DEFINE_CATEGORY(MediaDevice)
  * populate() before the media graph can be queried.
  */
 MediaDevice::MediaDevice(const std::string &deviceNode)
-	: deviceNode_(deviceNode), fd_(-1), valid_(false), acquired_(false)
+	: deviceNode_(deviceNode), fd_(-1), valid_(false), acquired_(false),
+	  lockOwner_(false)
 {
 }
 
@@ -421,6 +422,32 @@  void MediaDevice::release()
 	acquired_ = false;
 }
 
+bool MediaDevice::lock()
+{
+	if (fd_ == -1)
+		return false;
+
+	if (lockOwner_)
+		return true;
+
+	if (lockf(fd_, F_TLOCK, 0))
+		return false;
+
+	lockOwner_ = true;
+
+	return true;
+}
+
+void MediaDevice::unlock()
+{
+	if (fd_ == -1)
+		return;
+
+	lockOwner_ = false;
+
+	lockf(fd_, F_ULOCK, 0);
+}
+
 /**
  * \var MediaDevice::objects_
  * \brief Global map of media objects (entities, pads, links) keyed by their