[libcamera-devel] [PATCH] v4l2: Support setting frame rate in the V4L2 Adaptation layer
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Feb 25 09:47:37 CET 2022
Hi Nejc,
Thank you for the patch.
On Tue, Feb 15, 2022 at 12:42:22PM +0100, Nejc Galof wrote:
> The V4L2 adaptation layer can already support streaming with components
> such as OpenCV, however it is not accepting, or handling any requests to
> configure the frame rate.
>
> In V4L2 the frame rate is set by configuring the timeperframe component
> of the v4l2_streamparm structure through the VIDIOC_S_PARM ioctl.
>
> Extend the V4L2 compatibility layer to accept the VIDIOC_S_PARM ioctls
> and provide an interface for setting controls on the V4L2Camera class to
> set the requested rate when starting the camera.
>
> Signed-off-by: Nejc Galof <galof.nejc at gmail.com>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> src/v4l2/v4l2_camera.cpp | 12 +++++++++---
> src/v4l2/v4l2_camera.h | 7 +++++++
> src/v4l2/v4l2_camera_proxy.cpp | 25 +++++++++++++++++++++++++
> src/v4l2/v4l2_camera_proxy.h | 1 +
> 4 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index e922b9e6..e4eb3a2b 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -12,13 +12,15 @@
>
> #include <libcamera/base/log.h>
>
> +#include <libcamera/control_ids.h>
> +
> using namespace libcamera;
>
> LOG_DECLARE_CATEGORY(V4L2Compat)
>
> V4L2Camera::V4L2Camera(std::shared_ptr<Camera> camera)
> - : camera_(camera), isRunning_(false), bufferAllocator_(nullptr),
> - efd_(-1), bufferAvailableCount_(0)
> + : camera_(camera), controls_(controls::controls), isRunning_(false),
> + bufferAllocator_(nullptr), efd_(-1), bufferAvailableCount_(0)
> {
> camera_->requestCompleted.connect(this, &V4L2Camera::requestComplete);
> }
> @@ -203,10 +205,12 @@ int V4L2Camera::streamOn()
> if (isRunning_)
> return 0;
>
> - int ret = camera_->start();
> + int ret = camera_->start(&controls_);
> if (ret < 0)
> return ret == -EACCES ? -EBUSY : ret;
>
> + controls_.clear();
> +
> isRunning_ = true;
>
> for (Request *req : pendingRequests_) {
> @@ -266,6 +270,8 @@ int V4L2Camera::qbuf(unsigned int index)
> return 0;
> }
>
> + request->controls().merge(std::move(controls_));
> +
> ret = camera_->queueRequest(request);
> if (ret < 0) {
> LOG(V4L2Compat, Error) << "Can't queue request";
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index 03e74118..9e6c895a 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -10,11 +10,14 @@
> #include <deque>
> #include <utility>
>
> +#include <linux/videodev2.h>
I don't think you need to include this here.
> +
> #include <libcamera/base/mutex.h>
> #include <libcamera/base/semaphore.h>
> #include <libcamera/base/shared_fd.h>
>
> #include <libcamera/camera.h>
> +#include <libcamera/controls.h>
> #include <libcamera/framebuffer.h>
> #include <libcamera/framebuffer_allocator.h>
>
> @@ -49,6 +52,8 @@ public:
> const libcamera::Size &size,
> libcamera::StreamConfiguration *streamConfigOut);
>
> + libcamera::ControlList &controls() { return controls_; }
I may have gone for a setControls() function, but this works too.
> +
> int allocBuffers(unsigned int count);
> void freeBuffers();
> int getBufferFd(unsigned int index);
> @@ -69,6 +74,8 @@ private:
> std::shared_ptr<libcamera::Camera> camera_;
> std::unique_ptr<libcamera::CameraConfiguration> config_;
>
> + libcamera::ControlList controls_;
> +
> bool isRunning_;
>
> libcamera::Mutex bufferLock_;
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index e114d09f..f005b21c 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -18,6 +18,8 @@
> #include <unistd.h>
>
> #include <libcamera/camera.h>
> +#include <libcamera/controls.h>
> +#include <libcamera/control_ids.h>
> #include <libcamera/formats.h>
>
> #include <libcamera/base/log.h>
> @@ -33,6 +35,7 @@
> #define KERNEL_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + (c))
>
> using namespace libcamera;
> +using namespace std::literals::chrono_literals;
>
> LOG_DECLARE_CATEGORY(V4L2Compat)
>
> @@ -755,6 +758,24 @@ int V4L2CameraProxy::vidioc_streamoff(V4L2CameraFile *file, int *arg)
> return ret;
> }
>
> +int V4L2CameraProxy::vidioc_s_param(V4L2CameraFile *file, struct v4l2_streamparm *arg)
> +{
> + LOG(V4L2Compat, Debug)
> + << "[" << file->description() << "] " << __func__ << "()";
> +
> + if (!validateBufferType(arg->type))
> + return -EINVAL;
> +
> + struct v4l2_fract *timeperframe = &arg->parm.capture.timeperframe;
> + utils::Duration frameDuration = 1s * (static_cast<double>(timeperframe->numerator) /
> + static_cast<double>(timeperframe->denominator));
Does the following work ?
utils::Duration frameDuration = 1.0s * timeperframe->numerator
/ timeperframe->denominator;
> +
> + int64_t uDuration = frameDuration.get<std::micro>();
> + vcam_->controls().set(controls::FrameDurationLimits, { uDuration, uDuration });
Looks like we should move the FrameDurationLimits control to using the
Duration type. Not a candidate for this patch of course.
> +
> + return 0;
> +}
Getting the frame rate back from the camera to implement vidioc_g_param
is a more difficult task, but would it make sense to cache the frame
rate in the V4L2CameraProxy and implement vidioc_g_param based on that
already ? The question of what frame rate to expose by default will be
an interesting one, but I think you can start with a hardcoded default
value of 1/30. This could be done in this patch if easy, or in a
separate patch.
> +
> const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {
> VIDIOC_QUERYCAP,
> VIDIOC_ENUM_FRAMESIZES,
> @@ -775,6 +796,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {
> VIDIOC_EXPBUF,
> VIDIOC_STREAMON,
> VIDIOC_STREAMOFF,
> + VIDIOC_S_PARM,
> };
>
> int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *arg)
> @@ -852,6 +874,9 @@ int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *ar
> case VIDIOC_STREAMOFF:
> ret = vidioc_streamoff(file, static_cast<int *>(arg));
> break;
> + case VIDIOC_S_PARM:
> + ret = vidioc_s_param(file, static_cast<struct v4l2_streamparm *>(arg));
vidioc_s_parm, to match the ioctl name.
All in all it looks fairly good. With the two small changes requested
above,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Will you send a v2 ?
> + break;
> default:
> ret = -ENOTTY;
> break;
> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> index 76ca2d8a..30a3f492 100644
> --- a/src/v4l2/v4l2_camera_proxy.h
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -65,6 +65,7 @@ private:
> int vidioc_expbuf(V4L2CameraFile *file, struct v4l2_exportbuffer *arg);
> int vidioc_streamon(V4L2CameraFile *file, int *arg);
> int vidioc_streamoff(V4L2CameraFile *file, int *arg);
> + int vidioc_s_param(V4L2CameraFile *file, struct v4l2_streamparm *arg);
>
> bool hasOwnership(V4L2CameraFile *file);
> int acquire(V4L2CameraFile *file);
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list