[libcamera-devel] [PATCH v3 09/14] libcamera: pipeline: uvcvideo: Add controls support
Jacopo Mondi
jacopo at jmondi.org
Mon Jul 1 19:03:53 CEST 2019
Hi Kieran, Laurent,
On Mon, Jul 01, 2019 at 02:38:12AM +0300, Laurent Pinchart wrote:
> From: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> Implement control support in the UVC pipeline handler by dynamically
> querying the V4L2 device for the supported V4L2 controls and populating
> the list of camera controls accordingly.
>
> Not-signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Why so?
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/libcamera/pipeline/uvcvideo.cpp | 129 +++++++++++++++++++++++++---
> 1 file changed, 115 insertions(+), 14 deletions(-)
>
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 2e22523d7cb1..f68dc5bd6f74 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -6,8 +6,11 @@
> */
>
> #include <algorithm>
> +#include <iomanip>
> +#include <tuple>
>
> #include <libcamera/camera.h>
> +#include <libcamera/controls.h>
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
>
> @@ -16,6 +19,7 @@
> #include "media_device.h"
> #include "pipeline_handler.h"
> #include "utils.h"
> +#include "v4l2_controls.h"
> #include "v4l2_videodevice.h"
>
> namespace libcamera {
> @@ -35,6 +39,7 @@ public:
> delete video_;
> }
>
> + int init(MediaEntity *entity);
> void bufferReady(Buffer *buffer);
>
> V4L2VideoDevice *video_;
> @@ -71,6 +76,8 @@ public:
> bool match(DeviceEnumerator *enumerator) override;
>
> private:
> + int processControls(UVCCameraData *data, Request *request);
> +
> UVCCameraData *cameraData(const Camera *camera)
> {
> return static_cast<UVCCameraData *>(
> @@ -216,6 +223,54 @@ void PipelineHandlerUVC::stop(Camera *camera)
> PipelineHandler::stop(camera);
> }
>
> +int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
> +{
> + V4L2ControlList controls;
> +
> + for (auto it : request->controls()) {
> + const ControlInfo *ci = it.first;
> + ControlValue &value = it.second;
It would really be nice if we could move to a simple wrapper to
replace the typedef and be able to
for (Control ctrl : request->controls()) {
const ControlInfo *ci = ctrl->info();
ControlValue &value = ctrl->value();
> +
> + switch (ci->id()) {
> + case Brightness:
> + controls.add(V4L2_CID_BRIGHTNESS, value.getInt());
> + break;
> +
> + case Contrast:
> + controls.add(V4L2_CID_CONTRAST, value.getInt());
> + break;
> +
> + case Saturation:
> + controls.add(V4L2_CID_SATURATION, value.getInt());
> + break;
> +
> + case ManualExposure:
> + controls.add(V4L2_CID_EXPOSURE_AUTO, 1);
> + controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.getInt());
> + break;
> +
> + case ManualGain:
> + controls.add(V4L2_CID_GAIN, value.getInt());
or even be able to
ctrl.getInt()
> + break;
> +
> + default:
> + break;
> + }
> + }
Will every pipeline handler have to do this translation by itself? I
guess so, as each platform would map a Libcamera control to possibly
different controls or parameters..
> +
> + for (const V4L2Control &ctrl : controls)
> + LOG(UVC, Debug)
> + << "Setting control 0x"
> + << std::hex << std::setw(8) << ctrl.id() << std::dec
> + << " to " << ctrl.value();
> +
> + int ret = data->video_->setControls(&controls);
> + if (ret)
> + LOG(UVC, Error) << "Failed to set controls";
Print out the return value as it would tell which control has failed
> +
> + return ret;
> +}
> +
> int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request)
> {
> UVCCameraData *data = cameraData(camera);
> @@ -227,7 +282,11 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request)
> return -ENOENT;
> }
>
> - int ret = data->video_->queueBuffer(buffer);
> + int ret = processControls(data, request);
> + if (ret < 0)
Not just < 0, as V4L2Device::setControls() can return a positive
integer
> + return ret;
> +
> + ret = data->video_->queueBuffer(buffer);
> if (ret < 0)
> return ret;
>
> @@ -247,24 +306,14 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
>
> std::unique_ptr<UVCCameraData> data = utils::make_unique<UVCCameraData>(this);
>
> - /* Locate and open the default video node. */
> + /* Locate and initialise the camera data with the default video node. */
> for (MediaEntity *entity : media->entities()) {
> if (entity->flags() & MEDIA_ENT_FL_DEFAULT) {
> - data->video_ = new V4L2VideoDevice(entity);
> - break;
> + if (data->init(entity))
> + return false;
You can break the loop here I guess
> }
> }
>
> - if (!data->video_) {
> - LOG(UVC, Error) << "Could not find a default video device";
> - return false;
> - }
> -
> - if (data->video_->open())
> - return false;
> -
> - data->video_->bufferReady.connect(data.get(), &UVCCameraData::bufferReady);
> -
> /* Create and register the camera. */
> std::set<Stream *> streams{ &data->stream_ };
> std::shared_ptr<Camera> camera = Camera::create(this, media->model(), streams);
> @@ -276,6 +325,58 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
> return true;
> }
>
> +int UVCCameraData::init(MediaEntity *entity)
> +{
> + int ret;
> +
> + /* Create and open the video device. */
> + video_ = new V4L2VideoDevice(entity);
> + if (!video_) {
> + LOG(UVC, Error) << "Could not find a default video device";
Update the error message
> + return -ENODEV;
> + }
> +
> + ret = video_->open();
> + if (ret)
> + return ret;
> +
> + video_->bufferReady.connect(this, &UVCCameraData::bufferReady);
> +
> + /* Initialise the supported controls. */
> + const V4L2ControlInfoMap &controls = video_->controls();
> + for (const auto &ctrl : controls) {
> + unsigned int v4l2Id = ctrl.first;
> + const V4L2ControlInfo &info = ctrl.second;
> + ControlId id;
> +
> + switch (v4l2Id) {
> + case V4L2_CID_BRIGHTNESS:
> + id = Brightness;
> + break;
> + case V4L2_CID_CONTRAST:
> + id = Contrast;
> + break;
> + case V4L2_CID_SATURATION:
> + id = Saturation;
> + break;
> + case V4L2_CID_EXPOSURE_ABSOLUTE:
> + id = ManualExposure;
> + break;
> + case V4L2_CID_GAIN:
> + id = ManualGain;
> + break;
> + default:
> + continue;
> + }
> +
> + controlInfo_.emplace(std::piecewise_construct,
> + std::forward_as_tuple(id),
> + std::forward_as_tuple(id, info.min(), info.max()));
Awesome! Thanks for clarifying offline.
Minors apart, I'm happy with the direction this is taking, if not for
the fact we have to access maps with the horrible first and second
accessors for both libcamera controls and v4l2 controls. A way to
abstract that away would make this very nice.
Thanks
j
> + }
> +
> + return 0;
> +}
> +
> void UVCCameraData::bufferReady(Buffer *buffer)
> {
> Request *request = queuedRequests_.front();
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190701/e0159fcf/attachment.sig>
More information about the libcamera-devel
mailing list