[libcamera-devel,5/5] libcamera: v4l2_device: Provide V4L2 Log helper
diff mbox series

Message ID 20190207212119.30299-6-kieran.bingham@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: Buffer Sharing
Related show

Commit Message

Kieran Bingham Feb. 7, 2019, 9:21 p.m. UTC 
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@ideasonboard.com>
---
 src/libcamera/v4l2_device.cpp | 48 +++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 22 deletions(-)

Comments

Jacopo Mondi Feb. 7, 2019, 10:56 p.m. UTC | #1 
Hi Kieran,
   while this goes in the right direction in my opnion,
could we maybe hide this behind a regular LOG, maybe adding an
optional third argument?

Something like:

------------------------------------------------------------------------------
diff --git a/src/libcamera/include/log.h b/src/libcamera/include/log.h
index ad8f845..91fe093 100644
--- a/src/libcamera/include/log.h
+++ b/src/libcamera/include/log.h
@@ -8,6 +8,7 @@
 #define __LIBCAMERA_LOG_H__

 #include <sstream>
+#include <string>

 namespace libcamera {

@@ -53,6 +54,8 @@ public:
                   LogSeverity severity);
        LogMessage(const char *fileName, unsigned int line,
                   const LogCategory &category, LogSeverity severity);
+       LogMessage(const char *fileName, unsigned int line, std::string id,
+                  const LogCategory &category, LogSeverity severity);
        LogMessage(const LogMessage &) = delete;
        ~LogMessage();

@@ -64,6 +67,7 @@ private:
        std::ostringstream msgStream_;
        const LogCategory &category_;
        LogSeverity severity_;
+       std::string id_;
 };

 #ifndef __DOXYGEN__
@@ -73,13 +77,15 @@ private:
        LogMessage(__FILE__, __LINE__, Log##severity).stream()
 #define _LOG2(category, severity) \
        LogMessage(__FILE__, __LINE__, _LOG_CATEGORY(category)(), Log##severity).stream()
+#define _LOG3(category, severity, id) \
+       LogMessage(__FILE__, __LINE__, id, _LOG_CATEGORY(category)(), Log##severity).stream()

 /*
  * Expand the LOG() macro to _LOG1() or _LOG2() based on the number of
  * arguments.
  */
-#define _LOG_MACRO(_1, _2, NAME, ...) NAME
-#define LOG(...) _LOG_MACRO(__VA_ARGS__, _LOG2, _LOG1)(__VA_ARGS__)
+#define _LOG_MACRO(_1, _2, _3, NAME, ...) NAME
+#define LOG(...) _LOG_MACRO(__VA_ARGS__, _LOG3, _LOG2, _LOG1)(__VA_ARGS__)
 #else /* __DOXYGEN___ */
 #define LOG(category, severity)
 #endif /* __DOXYGEN__ */
diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp
index edeb5ea..2e68a0d 100644
--- a/src/libcamera/log.cpp
+++ b/src/libcamera/log.cpp
@@ -380,7 +380,7 @@ static const char *log_severity_name(LogSeverity severity)
  */
 LogMessage::LogMessage(const char *fileName, unsigned int line,
                       LogSeverity severity)
-       : category_(LogCategory::defaultCategory()), severity_(severity)
+       : category_(LogCategory::defaultCategory()), severity_(severity), id_("")
 {
        init(fileName, line);
 }
@@ -400,11 +400,17 @@ LogMessage::LogMessage(const char *fileName, unsigned int line,
  */
 LogMessage::LogMessage(const char *fileName, unsigned int line,
                       const LogCategory &category, LogSeverity severity)
-       : category_(category), severity_(severity)
+       : category_(category), severity_(severity), id_("")
 {
        init(fileName, line);
 }

+LogMessage::LogMessage(const char *fileName, unsigned int line, std::string id,
+                      const LogCategory &category, LogSeverity severity)
+       : category_(category), severity_(severity), id_(id)
+{
+       init(fileName, line);
+}
 void LogMessage::init(const char *fileName, unsigned int line)
 {
        /* Log the timestamp, severity and file information. */
@@ -420,6 +426,7 @@ void LogMessage::init(const char *fileName, unsigned int line)
        msgStream_ << " " << log_severity_name(severity_);
        msgStream_ << " " << category_.name();
        msgStream_ << " " << basename(fileName) << ":" << line << " ";
+       msgStream_ << " " << id_ << " ";
 }

 LogMessage::~LogMessage()
 -----------------------------------------------------------------------------

This would produce something like:
[9:20:55.697520218]   ERR V4L2 v4l2_device.cpp:255  /dev/video0 Device already open

Thanks
   j


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@ideasonboard.com>
> ---
>  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;
>  	}
> --
> 2.19.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Feb. 8, 2019, 5:33 p.m. UTC | #2 
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@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;
>  	}
Kieran Bingham Feb. 11, 2019, 11:59 a.m. UTC | #3 
Hi Laurent,

On 08/02/2019 17:33, Laurent Pinchart wrote:
> 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@ideasonboard.com>
> 
> I've sent a patch that adds a Loggable class, it should cover your use
> case in a more generic way.

Sure, I'll just drop this.
--
Kieran


> 
>> ---
>>  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;
>>  	}
>

Patch
diff mbox series 

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;
 	}