[libcamera-devel] [PATCH 5/5] libcamera: v4l2_device: Provide V4L2 Log helper

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thu, Feb 07, 2019 at 09:21:19PM +0000, Kieran Bingham wrote:
> Add a V4L2DEVICE_LOG Macro helper to prepend the V4L2 Video Device node to
> every log output.  This is particularly useful when more than one V4L2Device is
> utilised in the system.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

I've sent a patch that adds a Loggable class, it should cover your use
case in a more generic way.

> ---
>  src/libcamera/v4l2_device.cpp | 48 +++++++++++++++++++----------------
>  1 file changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index c2e4d0ea8db2..ce977613ff20 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -28,6 +28,8 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(V4L2)
>  
> +#define V4L2DEVICE_LOG(severity) LOG(V4L2, severity) << deviceNode_ << ": "
> +
>  /**
>   * \struct V4L2Capability
>   * \brief struct v4l2_capability object wrapper and helpers
> @@ -254,14 +256,14 @@ int V4L2Device::open()
>  	int ret;
>  
>  	if (isOpen()) {
> -		LOG(V4L2, Error) << "Device already open";
> +		V4L2DEVICE_LOG(Error) << "Device already open";
>  		return -EBUSY;
>  	}
>  
>  	ret = ::open(deviceNode_.c_str(), O_RDWR | O_NONBLOCK);
>  	if (ret < 0) {
>  		ret = -errno;
> -		LOG(V4L2, Error)
> +		V4L2DEVICE_LOG(Error)
>  			<< "Failed to open V4L2 device '" << deviceNode_
>  			<< "': " << strerror(-ret);
>  		return ret;
> @@ -271,24 +273,25 @@ int V4L2Device::open()
>  	ret = ioctl(fd_, VIDIOC_QUERYCAP, &caps_);
>  	if (ret < 0) {
>  		ret = -errno;
> -		LOG(V4L2, Error)
> +		V4L2DEVICE_LOG(Error)
>  			<< "Failed to query device capabilities: "
>  			<< strerror(-ret);
>  		return ret;
>  	}
>  
> -	LOG(V4L2, Debug)
> +	V4L2DEVICE_LOG(Debug)
>  		<< "Opened '" << deviceNode_ << "' "
>  		<< caps_.bus_info() << ": " << caps_.driver()
>  		<< ": " << caps_.card();
>  
>  	if (!caps_.isCapture() && !caps_.isOutput()) {
> -		LOG(V4L2, Debug) << "Device is not a supported type";
> +		V4L2DEVICE_LOG(Debug) << "Device is not a supported type";
>  		return -EINVAL;
>  	}
>  
>  	if (!caps_.hasStreaming()) {
> -		LOG(V4L2, Error) << "Device does not support streaming I/O";
> +		V4L2DEVICE_LOG(Error)
> +			<< "Device does not support streaming I/O";
>  		return -EINVAL;
>  	}
>  
> @@ -513,13 +516,13 @@ int V4L2Device::requestBuffers(unsigned int count)
>  	ret = ioctl(fd_, VIDIOC_REQBUFS, &rb);
>  	if (ret < 0) {
>  		ret = -errno;
> -		LOG(V4L2, Error)
> +		V4L2DEVICE_LOG(Error)
>  			<< "Unable to request " << count << " buffers: "
>  			<< strerror(-ret);
>  		return ret;
>  	}
>  
> -	LOG(V4L2, Debug)
> +	V4L2DEVICE_LOG(Debug)
>  		<< deviceNode_ << ":" << rb.count << " buffers requested.";
>  
>  	return rb.count;
> @@ -545,7 +548,8 @@ int V4L2Device::exportBuffers(BufferPool *pool)
>  
>  	allocatedBuffers = ret;
>  	if (allocatedBuffers < pool->count()) {
> -		LOG(V4L2, Error) << "Not enough buffers provided by V4L2Device";
> +		V4L2DEVICE_LOG(Error)
> +			<< "Not enough buffers provided by V4L2Device";
>  		requestBuffers(0);
>  		return -ENOMEM;
>  	}
> @@ -565,7 +569,7 @@ int V4L2Device::exportBuffers(BufferPool *pool)
>  		ret = ioctl(fd_, VIDIOC_QUERYBUF, &buf);
>  		if (ret < 0) {
>  			ret = -errno;
> -			LOG(V4L2, Error)
> +			V4L2DEVICE_LOG(Error)
>  				<< "Unable to query buffer " << i << ": "
>  				<< strerror(-ret);
>  			break;
> @@ -583,7 +587,7 @@ int V4L2Device::exportBuffers(BufferPool *pool)
>  		}
>  
>  		if (ret) {
> -			LOG(V4L2, Error) << "Failed to create plane";
> +			V4L2DEVICE_LOG(Error) << "Failed to create plane";
>  			break;
>  		}
>  	}
> @@ -605,7 +609,7 @@ int V4L2Device::createPlane(Buffer *buffer, unsigned int planeIndex,
>  	struct v4l2_exportbuffer expbuf = {};
>  	int ret;
>  
> -	LOG(V4L2, Debug)
> +	V4L2DEVICE_LOG(Debug)
>  		<< "Buffer " << buffer->index()
>  		<< " plane " << planeIndex
>  		<< ": length=" << length;
> @@ -618,7 +622,7 @@ int V4L2Device::createPlane(Buffer *buffer, unsigned int planeIndex,
>  	ret = ioctl(fd_, VIDIOC_EXPBUF, &expbuf);
>  	if (ret < 0) {
>  		ret = -errno;
> -		LOG(V4L2, Error)
> +		V4L2DEVICE_LOG(Error)
>  			<< "Failed to export buffer: " << strerror(-ret);
>  		return ret;
>  	}
> @@ -648,13 +652,13 @@ int V4L2Device::importBuffers(BufferPool *pool)
>  
>  	allocatedBuffers = ret;
>  	if (allocatedBuffers < pool->count()) {
> -		LOG(V4L2, Error)
> +		V4L2DEVICE_LOG(Error)
>  			<< "Not enough buffers provided by V4L2Device";
>  		requestBuffers(0);
>  		return -ENOMEM;
>  	}
>  
> -	LOG(V4L2, Debug) << "Device using an externally provided pool";
> +	V4L2DEVICE_LOG(Debug) << "Device using an externally provided pool";
>  	bufferPool_ = pool;
>  
>  	return 0;
> @@ -665,7 +669,7 @@ int V4L2Device::importBuffers(BufferPool *pool)
>   */
>  int V4L2Device::releaseBuffers()
>  {
> -	LOG(V4L2, Debug) << "Releasing bufferPool";
> +	V4L2DEVICE_LOG(Debug) << "Releasing bufferPool";
>  
>  	requestBuffers(0);
>  	bufferPool_ = nullptr;
> @@ -715,12 +719,12 @@ int V4L2Device::queueBuffer(Buffer *buffer)
>  		buf.m.planes = planes;
>  	}
>  
> -	LOG(V4L2, Debug) << "Queueing buffer " << buf.index;
> +	V4L2DEVICE_LOG(Debug) << "Queueing buffer " << buf.index;
>  
>  	ret = ioctl(fd_, VIDIOC_QBUF, &buf);
>  	if (ret < 0) {
>  		ret = -errno;
> -		LOG(V4L2, Error)
> +		V4L2DEVICE_LOG(Error)
>  			<< "Failed to queue buffer " << buf.index << ": "
>  			<< strerror(-ret);
>  		return ret;
> @@ -757,7 +761,7 @@ Buffer *V4L2Device::dequeueBuffer()
>  	ret = ioctl(fd_, VIDIOC_DQBUF, &buf);
>  	if (ret < 0) {
>  		ret = -errno;
> -		LOG(V4L2, Error)
> +		V4L2DEVICE_LOG(Error)
>  			<< "Failed to dequeue buffer: " << strerror(-ret);
>  		return nullptr;
>  	}
> @@ -793,7 +797,7 @@ void V4L2Device::bufferAvailable(EventNotifier *notifier)
>  	if (!buffer)
>  		return;
>  
> -	LOG(V4L2, Debug) << "Buffer " << buffer->index() << " is available";
> +	V4L2DEVICE_LOG(Debug) << "Buffer " << buffer->index() << " is available";
>  
>  	/* Notify anyone listening to the device. */
>  	bufferReady.emit(buffer);
> @@ -819,7 +823,7 @@ int V4L2Device::streamOn()
>  	ret = ioctl(fd_, VIDIOC_STREAMON, &bufferType_);
>  	if (ret < 0) {
>  		ret = -errno;
> -		LOG(V4L2, Error)
> +		V4L2DEVICE_LOG(Error)
>  			<< "Failed to start streaming: " << strerror(-ret);
>  		return ret;
>  	}
> @@ -841,7 +845,7 @@ int V4L2Device::streamOff()
>  	ret = ioctl(fd_, VIDIOC_STREAMOFF, &bufferType_);
>  	if (ret < 0) {
>  		ret = -errno;
> -		LOG(V4L2, Error)
> +		V4L2DEVICE_LOG(Error)
>  			<< "Failed to stop streaming: " << strerror(-ret);
>  		return ret;
>  	}

-- 
Regards,

Laurent Pinchart