[libcamera-devel,1/3] libcamera: media_device: Add DeviceInfo features

Message ID 20190102004903.24190-1-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • [libcamera-devel,1/3] libcamera: media_device: Add DeviceInfo features
Related show

Commit Message

Laurent Pinchart Jan. 2, 2019, 12:49 a.m. UTC
From: Jacopo Mondi <jacopo@jmondi.org>

Add the features of the DeviceInfo class needed to replace it with
MediaDevice.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/include/media_device.h |  5 +++
 src/libcamera/media_device.cpp       | 52 ++++++++++++++++++++++++++--
 2 files changed, 55 insertions(+), 2 deletions(-)

Comments

Jacopo Mondi Jan. 2, 2019, 10:15 a.m. UTC | #1
Hi Laurent,
   thanks for re-submitting these patches

On Wed, Jan 02, 2019 at 02:49:01AM +0200, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo@jmondi.org>
>
> Add the features of the DeviceInfo class needed to replace it with
> MediaDevice.
>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/include/media_device.h |  5 +++
>  src/libcamera/media_device.cpp       | 52 ++++++++++++++++++++++++++--
>  2 files changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h
> index d787be391882..3fcdb4b4d5f8 100644
> --- a/src/libcamera/include/media_device.h
> +++ b/src/libcamera/include/media_device.h
> @@ -24,6 +24,10 @@ public:
>  	MediaDevice(const std::string &devnode);
>  	~MediaDevice();
>
> +	bool acquire();
> +	void release() { acquired_ = false; }
> +	bool busy() const { return acquired_; }
> +
>  	int open();
>  	void close();
>
> @@ -41,6 +45,7 @@ private:
>  	std::string devnode_;
>  	int fd_;
>  	bool valid_;
> +	bool acquired_;
>
>  	std::map<unsigned int, MediaObject *> objects_;
>  	MediaObject *object(unsigned int id);
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index 1bd4f43a0c79..605e504be124 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -52,7 +52,11 @@ namespace libcamera {
>   * controller device node. It can be closed at any time with a call to close().
>   * This will not invalidate the media graph and all cached media objects remain
>   * valid and can be accessed normally. The device can then be later reopened if
> - * needed to perform other operations that interact with the device node.
> + * needed to perform other operations that interect with the device node.

s/interect/interact/

> + *
> + * Media device can be claimed for exclusive use with acquire(), released with
> + * release() and tested with busy(). This mechanism is aimed at pipeline
> + * managers to claim media devices they support during enumeration.
>   */
>
>  /**
> @@ -63,7 +67,7 @@ namespace libcamera {
>   * populated with open() and populate() before the media graph can be queried.
>   */
>  MediaDevice::MediaDevice(const std::string &devnode)
> -	: devnode_(devnode), fd_(-1), valid_(false)
> +	: devnode_(devnode), fd_(-1), valid_(false), acquired_(false)
>  {
>  }
>
> @@ -74,6 +78,50 @@ MediaDevice::~MediaDevice()
>  	clear();
>  }
>
> +/**
> + * \brief Claim a device for exclusive use
> + *
> + * The device claiming mechanism offers simple media device access arbitration
> + * between multiple users. When the media device is created, it is available to
> + * all users. Users can query the media graph to determine whether they can
> + * support the device and, if they do, claim the device for exclusive use. Other
> + * users are then expected to skip over media devices in use as reported by the
> + * busy() function.
> + *
> + * Once claimed the device shall be released by its user when not needed anymore
> + * by calling the release() function.
> + *
> + * Exclusive access is only guaranteed if all users of the media device abide by

s/by/to ?

> + * the device claiming mechanism, as it isn't enforced by the media device
> + * itself.
> + *
> + * \return true if the device was successfully claimed, or false if it was
> + * already in use
> + * \sa release(), busy()
> + */
> +bool MediaDevice::acquire()
> +{
> +	if (acquired_)
> +		return false;
> +
> +	acquired_ = true;
> +	return true;
> +}
> +
> +/**
> + * \fn MediaDevice::release()
> + * \brief Release a device previously claimed for exclusive use
> + * \sa acquire(), busy()
> + */
> +
> +/**
> + * \fn MediaDevice::busy()
> + * \brief Check if a device is in use
> + * \return true if the device has been claimed for exclusive user, or false if

s/user/use

> + * it is available
> + * \sa acquire(), release()
> + */
> +

With this minor things addressed, please push

Thanks
   j


>  /**
>   * \brief Open a media device and retrieve device information
>   *
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Jan. 2, 2019, 10:38 a.m. UTC | #2
Hi Jacopo,

On Wednesday, 2 January 2019 12:15:54 EET Jacopo Mondi wrote:
> Hi Laurent,
>    thanks for re-submitting these patches
> 
> On Wed, Jan 02, 2019 at 02:49:01AM +0200, Laurent Pinchart wrote:
> > From: Jacopo Mondi <jacopo@jmondi.org>
> > 
> > Add the features of the DeviceInfo class needed to replace it with
> > MediaDevice.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  src/libcamera/include/media_device.h |  5 +++
> >  src/libcamera/media_device.cpp       | 52 ++++++++++++++++++++++++++--
> >  2 files changed, 55 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/libcamera/include/media_device.h
> > b/src/libcamera/include/media_device.h index d787be391882..3fcdb4b4d5f8
> > 100644
> > --- a/src/libcamera/include/media_device.h
> > +++ b/src/libcamera/include/media_device.h
> > 
> > @@ -24,6 +24,10 @@ public:
> >  	MediaDevice(const std::string &devnode);
> >  	~MediaDevice();
> > 
> > +	bool acquire();
> > +	void release() { acquired_ = false; }
> > +	bool busy() const { return acquired_; }
> > +
> > 
> >  	int open();
> >  	void close();
> > 
> > @@ -41,6 +45,7 @@ private:
> >  	std::string devnode_;
> >  	int fd_;
> >  	bool valid_;
> > 
> > +	bool acquired_;
> > 
> >  	std::map<unsigned int, MediaObject *> objects_;
> >  	MediaObject *object(unsigned int id);
> > 
> > diff --git a/src/libcamera/media_device.cpp
> > b/src/libcamera/media_device.cpp index 1bd4f43a0c79..605e504be124 100644
> > --- a/src/libcamera/media_device.cpp
> > +++ b/src/libcamera/media_device.cpp
> > @@ -52,7 +52,11 @@ namespace libcamera {
> > 
> >   * controller device node. It can be closed at any time with a call to
> >   close(). * This will not invalidate the media graph and all cached
> >   media objects remain * valid and can be accessed normally. The device
> >   can then be later reopened if> 
> > - * needed to perform other operations that interact with the device node.
> > + * needed to perform other operations that interect with the device node.
> 
> s/interect/interact/

Change dropped.

> > + *
> > + * Media device can be claimed for exclusive use with acquire(), released
> > with + * release() and tested with busy(). This mechanism is aimed at
> > pipeline + * managers to claim media devices they support during
> > enumeration.
> >   */
> >  
> >  /**
> > @@ -63,7 +67,7 @@ namespace libcamera {
> >   * populated with open() and populate() before the media graph can be
> >   queried. */
> >  MediaDevice::MediaDevice(const std::string &devnode)
> > -	: devnode_(devnode), fd_(-1), valid_(false)
> > +	: devnode_(devnode), fd_(-1), valid_(false), acquired_(false)
> >  {
> >  }
> > 
> > @@ -74,6 +78,50 @@ MediaDevice::~MediaDevice()
> >  	clear();
> >  }
> > 
> > +/**
> > + * \brief Claim a device for exclusive use
> > + *
> > + * The device claiming mechanism offers simple media device access
> > arbitration + * between multiple users. When the media device is created,
> > it is available to + * all users. Users can query the media graph to
> > determine whether they can + * support the device and, if they do, claim
> > the device for exclusive use. Other + * users are then expected to skip
> > over media devices in use as reported by the + * busy() function.
> > + *
> > + * Once claimed the device shall be released by its user when not needed
> > anymore + * by calling the release() function.
> > + *
> > + * Exclusive access is only guaranteed if all users of the media device
> > abide by
> s/by/to ?

No, abide by :)

http://www.wordreference.com/enit/abide

> > + * the device claiming mechanism, as it isn't enforced by the media
> > device
> > + * itself.
> > + *
> > + * \return true if the device was successfully claimed, or false if it
> > was
> > + * already in use
> > + * \sa release(), busy()
> > + */
> > +bool MediaDevice::acquire()
> > +{
> > +	if (acquired_)
> > +		return false;
> > +
> > +	acquired_ = true;
> > +	return true;
> > +}
> > +
> > +/**
> > + * \fn MediaDevice::release()
> > + * \brief Release a device previously claimed for exclusive use
> > + * \sa acquire(), busy()
> > + */
> > +
> > +/**
> > + * \fn MediaDevice::busy()
> > + * \brief Check if a device is in use
> > + * \return true if the device has been claimed for exclusive user, or
> > false if
> 
> s/user/use

Fixed.

> > + * it is available
> > + * \sa acquire(), release()
> > + */
> > +
> 
> With this minor things addressed, please push
> 
> >  /**
> >   * \brief Open a media device and retrieve device information
> >   *

Patch

diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h
index d787be391882..3fcdb4b4d5f8 100644
--- a/src/libcamera/include/media_device.h
+++ b/src/libcamera/include/media_device.h
@@ -24,6 +24,10 @@  public:
 	MediaDevice(const std::string &devnode);
 	~MediaDevice();
 
+	bool acquire();
+	void release() { acquired_ = false; }
+	bool busy() const { return acquired_; }
+
 	int open();
 	void close();
 
@@ -41,6 +45,7 @@  private:
 	std::string devnode_;
 	int fd_;
 	bool valid_;
+	bool acquired_;
 
 	std::map<unsigned int, MediaObject *> objects_;
 	MediaObject *object(unsigned int id);
diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
index 1bd4f43a0c79..605e504be124 100644
--- a/src/libcamera/media_device.cpp
+++ b/src/libcamera/media_device.cpp
@@ -52,7 +52,11 @@  namespace libcamera {
  * controller device node. It can be closed at any time with a call to close().
  * This will not invalidate the media graph and all cached media objects remain
  * valid and can be accessed normally. The device can then be later reopened if
- * needed to perform other operations that interact with the device node.
+ * needed to perform other operations that interect with the device node.
+ *
+ * Media device can be claimed for exclusive use with acquire(), released with
+ * release() and tested with busy(). This mechanism is aimed at pipeline
+ * managers to claim media devices they support during enumeration.
  */
 
 /**
@@ -63,7 +67,7 @@  namespace libcamera {
  * populated with open() and populate() before the media graph can be queried.
  */
 MediaDevice::MediaDevice(const std::string &devnode)
-	: devnode_(devnode), fd_(-1), valid_(false)
+	: devnode_(devnode), fd_(-1), valid_(false), acquired_(false)
 {
 }
 
@@ -74,6 +78,50 @@  MediaDevice::~MediaDevice()
 	clear();
 }
 
+/**
+ * \brief Claim a device for exclusive use
+ *
+ * The device claiming mechanism offers simple media device access arbitration
+ * between multiple users. When the media device is created, it is available to
+ * all users. Users can query the media graph to determine whether they can
+ * support the device and, if they do, claim the device for exclusive use. Other
+ * users are then expected to skip over media devices in use as reported by the
+ * busy() function.
+ *
+ * Once claimed the device shall be released by its user when not needed anymore
+ * by calling the release() function.
+ *
+ * Exclusive access is only guaranteed if all users of the media device abide by
+ * the device claiming mechanism, as it isn't enforced by the media device
+ * itself.
+ *
+ * \return true if the device was successfully claimed, or false if it was
+ * already in use
+ * \sa release(), busy()
+ */
+bool MediaDevice::acquire()
+{
+	if (acquired_)
+		return false;
+
+	acquired_ = true;
+	return true;
+}
+
+/**
+ * \fn MediaDevice::release()
+ * \brief Release a device previously claimed for exclusive use
+ * \sa acquire(), busy()
+ */
+
+/**
+ * \fn MediaDevice::busy()
+ * \brief Check if a device is in use
+ * \return true if the device has been claimed for exclusive user, or false if
+ * it is available
+ * \sa acquire(), release()
+ */
+
 /**
  * \brief Open a media device and retrieve device information
  *