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

[Laurent Pinchart]

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 at jmondi.org>
> > 
> > Add the features of the DeviceInfo class needed to replace it with
> > MediaDevice.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at 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
> >   *

-- 
Regards,

Laurent Pinchart