[libcamera-devel] [PATCH v1 5/5] v4l2: v4l2_camera_proxy: Use file description in debug messages
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Jan 10 17:46:55 CET 2022
Quoting Laurent Pinchart (2021-12-28 21:59:51)
> Debug messages in V4L2CameraProxy print the numerical file descriptor,
> which isn't very human-friendly. Replace it with the V4L2CameraFile
> description that includes the full path. While at it, refactor the
> messages to use __func__ instead of manually copying function names.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
This sounds pretty good to me.
I wonder if libcamera-v4l2 (libcameraify, libcamera-ify?) should have a
faciilty for setting these debug levels from the script ...
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> src/v4l2/v4l2_camera_proxy.cpp | 74 ++++++++++++++++++++------------
> src/v4l2/v4l2_camera_proxy.h | 7 +--
> src/v4l2/v4l2_compat_manager.cpp | 5 ++-
> 3 files changed, 54 insertions(+), 32 deletions(-)
>
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 80e6624bc9b7..5ccf40b9692c 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -46,7 +46,8 @@ V4L2CameraProxy::V4L2CameraProxy(unsigned int index,
>
> int V4L2CameraProxy::open(V4L2CameraFile *file)
> {
> - LOG(V4L2Compat, Debug) << "Servicing open fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> MutexLocker locker(proxyMutex_);
>
> @@ -80,7 +81,8 @@ int V4L2CameraProxy::open(V4L2CameraFile *file)
>
> void V4L2CameraProxy::close(V4L2CameraFile *file)
> {
> - LOG(V4L2Compat, Debug) << "Servicing close fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> MutexLocker locker(proxyMutex_);
>
> @@ -94,10 +96,11 @@ void V4L2CameraProxy::close(V4L2CameraFile *file)
> vcam_->close();
> }
>
> -void *V4L2CameraProxy::mmap(void *addr, size_t length, int prot, int flags,
> - off64_t offset)
> +void *V4L2CameraProxy::mmap(V4L2CameraFile *file, void *addr, size_t length,
> + int prot, int flags, off64_t offset)
> {
> - LOG(V4L2Compat, Debug) << "Servicing mmap";
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> MutexLocker locker(proxyMutex_);
>
> @@ -131,9 +134,10 @@ void *V4L2CameraProxy::mmap(void *addr, size_t length, int prot, int flags,
> return map;
> }
>
> -int V4L2CameraProxy::munmap(void *addr, size_t length)
> +int V4L2CameraProxy::munmap(V4L2CameraFile *file, void *addr, size_t length)
> {
> - LOG(V4L2Compat, Debug) << "Servicing munmap";
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> MutexLocker locker(proxyMutex_);
>
> @@ -233,9 +237,10 @@ void V4L2CameraProxy::updateBuffers()
> }
> }
>
> -int V4L2CameraProxy::vidioc_querycap(struct v4l2_capability *arg)
> +int V4L2CameraProxy::vidioc_querycap(V4L2CameraFile *file, struct v4l2_capability *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_querycap";
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> *arg = capabilities_;
>
> @@ -244,7 +249,8 @@ int V4L2CameraProxy::vidioc_querycap(struct v4l2_capability *arg)
>
> int V4L2CameraProxy::vidioc_enum_framesizes(V4L2CameraFile *file, struct v4l2_frmsizeenum *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_enum_framesizes fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> V4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->pixel_format);
> PixelFormat format = v4l2Format.toPixelFormat();
> @@ -267,7 +273,8 @@ int V4L2CameraProxy::vidioc_enum_framesizes(V4L2CameraFile *file, struct v4l2_fr
>
> int V4L2CameraProxy::vidioc_enum_fmt(V4L2CameraFile *file, struct v4l2_fmtdesc *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_enum_fmt fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> if (!validateBufferType(arg->type) ||
> arg->index >= streamConfig_.formats().pixelformats().size())
> @@ -288,7 +295,8 @@ int V4L2CameraProxy::vidioc_enum_fmt(V4L2CameraFile *file, struct v4l2_fmtdesc *
>
> int V4L2CameraProxy::vidioc_g_fmt(V4L2CameraFile *file, struct v4l2_format *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_g_fmt fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> if (!validateBufferType(arg->type))
> return -EINVAL;
> @@ -331,7 +339,8 @@ int V4L2CameraProxy::tryFormat(struct v4l2_format *arg)
>
> int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_s_fmt fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> if (!validateBufferType(arg->type))
> return -EINVAL;
> @@ -361,7 +370,8 @@ int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg)
>
> int V4L2CameraProxy::vidioc_try_fmt(V4L2CameraFile *file, struct v4l2_format *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_try_fmt fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> if (!validateBufferType(arg->type))
> return -EINVAL;
> @@ -384,7 +394,8 @@ enum v4l2_priority V4L2CameraProxy::maxPriority()
>
> int V4L2CameraProxy::vidioc_g_priority(V4L2CameraFile *file, enum v4l2_priority *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_g_priority fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> *arg = maxPriority();
>
> @@ -394,7 +405,7 @@ int V4L2CameraProxy::vidioc_g_priority(V4L2CameraFile *file, enum v4l2_priority
> int V4L2CameraProxy::vidioc_s_priority(V4L2CameraFile *file, enum v4l2_priority *arg)
> {
> LOG(V4L2Compat, Debug)
> - << "Servicing vidioc_s_priority fd = " << file->efd();
> + << "[" << file->description() << "] " << __func__ << "()";
>
> if (*arg > V4L2_PRIORITY_RECORD)
> return -EINVAL;
> @@ -409,7 +420,8 @@ int V4L2CameraProxy::vidioc_s_priority(V4L2CameraFile *file, enum v4l2_priority
>
> int V4L2CameraProxy::vidioc_enuminput(V4L2CameraFile *file, struct v4l2_input *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_enuminput fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> if (arg->index != 0)
> return -EINVAL;
> @@ -426,7 +438,8 @@ int V4L2CameraProxy::vidioc_enuminput(V4L2CameraFile *file, struct v4l2_input *a
>
> int V4L2CameraProxy::vidioc_g_input(V4L2CameraFile *file, int *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_g_input fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> *arg = 0;
>
> @@ -435,7 +448,8 @@ int V4L2CameraProxy::vidioc_g_input(V4L2CameraFile *file, int *arg)
>
> int V4L2CameraProxy::vidioc_s_input(V4L2CameraFile *file, int *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_s_input fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> if (*arg != 0)
> return -EINVAL;
> @@ -452,7 +466,8 @@ void V4L2CameraProxy::freeBuffers()
>
> int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_reqbufs fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> if (!validateBufferType(arg->type) ||
> !validateMemoryType(arg->memory))
> @@ -526,7 +541,8 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
>
> int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_querybuf fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> if (arg->index >= bufferCount_)
> return -EINVAL;
> @@ -544,8 +560,9 @@ int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *a
>
> int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_qbuf, index = "
> - << arg->index << " fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__
> + << "(index=" << arg->index << ")";
>
> if (arg->index >= bufferCount_)
> return -EINVAL;
> @@ -575,7 +592,8 @@ int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg)
> int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,
> Mutex *lock)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_dqbuf fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> if (arg->index >= bufferCount_)
> return -EINVAL;
> @@ -624,7 +642,8 @@ int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,
>
> int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_streamon fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> if (bufferCount_ == 0)
> return -EINVAL;
> @@ -648,7 +667,8 @@ int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg)
>
> int V4L2CameraProxy::vidioc_streamoff(V4L2CameraFile *file, int *arg)
> {
> - LOG(V4L2Compat, Debug) << "Servicing vidioc_streamoff fd = " << file->efd();
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
>
> if (!validateBufferType(*arg))
> return -EINVAL;
> @@ -709,7 +729,7 @@ int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *ar
> int ret;
> switch (request) {
> case VIDIOC_QUERYCAP:
> - ret = vidioc_querycap(static_cast<struct v4l2_capability *>(arg));
> + ret = vidioc_querycap(file, static_cast<struct v4l2_capability *>(arg));
> break;
> case VIDIOC_ENUM_FRAMESIZES:
> ret = vidioc_enum_framesizes(file, static_cast<struct v4l2_frmsizeenum *>(arg));
> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> index fa0a49e0439b..08f0295a92dd 100644
> --- a/src/v4l2/v4l2_camera_proxy.h
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -29,8 +29,9 @@ public:
>
> int open(V4L2CameraFile *file);
> void close(V4L2CameraFile *file);
> - void *mmap(void *addr, size_t length, int prot, int flags, off64_t offset);
> - int munmap(void *addr, size_t length);
> + void *mmap(V4L2CameraFile *file, void *addr, size_t length, int prot,
> + int flags, off64_t offset);
> + int munmap(V4L2CameraFile *file, void *addr, size_t length);
>
> int ioctl(V4L2CameraFile *file, unsigned long request, void *arg);
>
> @@ -44,7 +45,7 @@ private:
> void updateBuffers();
> void freeBuffers();
>
> - int vidioc_querycap(struct v4l2_capability *arg);
> + int vidioc_querycap(V4L2CameraFile *file, struct v4l2_capability *arg);
> int vidioc_enum_framesizes(V4L2CameraFile *file, struct v4l2_frmsizeenum *arg);
> int vidioc_enum_fmt(V4L2CameraFile *file, struct v4l2_fmtdesc *arg);
> int vidioc_g_fmt(V4L2CameraFile *file, struct v4l2_format *arg);
> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp
> index ded568beb60e..0f7575c54b22 100644
> --- a/src/v4l2/v4l2_compat_manager.cpp
> +++ b/src/v4l2/v4l2_compat_manager.cpp
> @@ -194,7 +194,8 @@ void *V4L2CompatManager::mmap(void *addr, size_t length, int prot, int flags,
> if (!file)
> return fops_.mmap(addr, length, prot, flags, fd, offset);
>
> - void *map = file->proxy()->mmap(addr, length, prot, flags, offset);
> + void *map = file->proxy()->mmap(file.get(), addr, length, prot, flags,
> + offset);
> if (map == MAP_FAILED)
> return map;
>
> @@ -210,7 +211,7 @@ int V4L2CompatManager::munmap(void *addr, size_t length)
>
> V4L2CameraFile *file = device->second.get();
>
> - int ret = file->proxy()->munmap(addr, length);
> + int ret = file->proxy()->munmap(file, addr, length);
> if (ret < 0)
> return ret;
>
> --
> Regards,
>
> Laurent Pinchart
>
More information about the libcamera-devel
mailing list