[libcamera-devel] [PATCH v3 1/8] libcamera: v4l2_subdevice: Store media entity

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2019-02-26 17:26:34 +0100, Jacopo Mondi wrote:
> Store the media entity backing the V4L2Subdevice and add a deviceName()
> method to retrieve the human readable name of the subdevice, which is
> created using the name of the associated media entity.
> 
> Use the deviceName() in error messages where appropriate, as the entity
> name is sometimes more informative than the device node path.

I would keep the device node in the error messages instead of the device 
name. As a user diagnosing the problem I would find it easier to 
associate the errno to a device node rather then a name. Also going from 
the device node to the device name is easier then the reveres using v4l2 
tools. Possibly you could print both.

With this fixed,

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/include/v4l2_subdevice.h |  8 +++++---
>  src/libcamera/v4l2_subdevice.cpp       | 17 ++++++++++++-----
>  2 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> index 82fa6685ab52..eac699a06109 100644
> --- a/src/libcamera/include/v4l2_subdevice.h
> +++ b/src/libcamera/include/v4l2_subdevice.h
> @@ -9,9 +9,10 @@
>  
>  #include <string>
>  
> +#include "media_object.h"
> +
>  namespace libcamera {
>  
> -class MediaEntity;
>  struct Rectangle;
>  
>  struct V4L2SubdeviceFormat {
> @@ -31,7 +32,8 @@ public:
>  	bool isOpen() const;
>  	void close();
>  
> -	std::string deviceNode() const { return deviceNode_; }
> +	std::string deviceNode() const { return entity_->deviceNode(); }
> +	std::string deviceName() const { return entity_->name(); }
>  
>  	int setCrop(unsigned int pad, Rectangle *rect);
>  	int setCompose(unsigned int pad, Rectangle *rect);
> @@ -43,7 +45,7 @@ private:
>  	int setSelection(unsigned int pad, unsigned int target,
>  			 Rectangle *rect);
>  
> -	std::string deviceNode_;
> +	const MediaEntity *entity_;
>  	int fd_;
>  };
>  
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index b436f73cc75f..56ecf3851cb0 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -88,7 +88,7 @@ LOG_DEFINE_CATEGORY(V4L2Subdev)
>   * path
>   */
>  V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity)
> -	: deviceNode_(entity->deviceNode()), fd_(-1)
> +	: entity_(entity), fd_(-1)
>  {
>  }
>  
> @@ -106,11 +106,11 @@ int V4L2Subdevice::open()
>  		return -EBUSY;
>  	}
>  
> -	ret = ::open(deviceNode_.c_str(), O_RDWR);
> +	ret = ::open(deviceNode().c_str(), O_RDWR);
>  	if (ret < 0) {
>  		ret = -errno;
>  		LOG(V4L2Subdev, Error)
> -			<< "Failed to open V4L2 subdevice '" << deviceNode_
> +			<< "Failed to open V4L2 subdevice '" << deviceNode()
>  			<< "': " << strerror(-ret);
>  		return ret;
>  	}
> @@ -147,6 +147,13 @@ void V4L2Subdevice::close()
>   * \return The subdevice's device node system path
>   */
>  
> +/**
> + * \fn V4L2Subdevice::deviceName()
> + * \brief Retrieve the name of the media entity associated with the subdevice
> + *
> + * \return The name of the media entity the subdevice is associated to
> + */
> +
>  /**
>   * \brief Set a crop rectangle on one of the V4L2 subdevice pads
>   * \param[in] pad The 0-indexed pad number the rectangle is to be applied to
> @@ -189,7 +196,7 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format)
>  		ret = -errno;
>  		LOG(V4L2Subdev, Error)
>  			<< "Unable to get format on pad " << pad
> -			<< " of " << deviceNode_ << ": " << strerror(-ret);
> +			<< " of " << deviceName() << ": " << strerror(-ret);
>  		return ret;
>  	}
>  
> @@ -255,7 +262,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
>  		ret = -errno;
>  		LOG(V4L2Subdev, Error)
>  			<< "Unable to set rectangle " << target << " on pad "
> -			<< pad << " of " << deviceNode_ << ": "
> +			<< pad << " of " << deviceName() << ": "
>  			<< strerror(-ret);
>  		return ret;
>  	}
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund