From patchwork Thu Jun 6 20:56:52 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 1375 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C881860BB7 for ; Thu, 6 Jun 2019 22:56:58 +0200 (CEST) Received: from localhost.localdomain (cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6399ED87; Thu, 6 Jun 2019 22:56:58 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1559854618; bh=jB7fjuFM07QGwg1K2Vzb0MDgOJEvI0E6GIBudIfQw+A=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WEc0BuIrxd/1vtV0qtD0zCZ2EYy9FDxqDggcx6CePwKt32GcNHpreuQoTK0Fg/vqb z6AOGFxocGjIuCSu4Cdd29749Tdk7UG1OpiqdfG2t2IedKwXPL7ccPuLkNyMBTR9u2 XBBoJMVTkQycVR7cHOD7U9iIGuuTya4UuA7gLdUQ= From: Kieran Bingham To: LibCamera Devel Date: Thu, 6 Jun 2019 21:56:52 +0100 Message-Id: <20190606205654.9311-4-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190606205654.9311-1-kieran.bingham@ideasonboard.com> References: <20190606205654.9311-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC PATCH 3/5] libcamera: pipeline: Add readControls(), writeControl() interfaces X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Jun 2019 20:56:59 -0000 Pipeline handlers must implement functions to handle controls as part of their interface. Extend the pipeline handler interface to declare this requirement and implement basic control functionality in the currently supported PipelineHandlers --- src/libcamera/include/pipeline_handler.h | 3 + src/libcamera/pipeline/ipu3/ipu3.cpp | 19 ++++ src/libcamera/pipeline/raspberrypi.cpp | 108 ++++++++++++++++++- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 ++++ src/libcamera/pipeline/uvcvideo.cpp | 127 ++++++++++++++++++++++- src/libcamera/pipeline/vimc.cpp | 19 ++++ 6 files changed, 293 insertions(+), 2 deletions(-) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 7da6df1ab2a0..4e306d964bff 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -72,6 +72,9 @@ public: virtual int start(Camera *camera) = 0; virtual void stop(Camera *camera); + virtual int readControls(Camera *camera, Request *request) = 0; + virtual int writeControls(Camera *camera, Request *request) = 0; + virtual int queueRequest(Camera *camera, Request *request); bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 11e25e276c81..811a7c85bfc8 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -213,6 +213,9 @@ public: int start(Camera *camera) override; void stop(Camera *camera) override; + int readControls(Camera *camera, Request *request) override; + int writeControls(Camera *camera, Request *request) override; + int queueRequest(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator) override; @@ -812,6 +815,22 @@ void PipelineHandlerIPU3::stop(Camera *camera) PipelineHandler::stop(camera); } +int PipelineHandlerIPU3::readControls(Camera *camera, Request *request) +{ + LOG(IPU3, Error) + << "NOT IMPLEMENTED"; + + return -EINVAL; +} + +int PipelineHandlerIPU3::writeControls(Camera *camera, Request *request) +{ + LOG(IPU3, Error) + << "NOT IMPLEMENTED"; + + return -EINVAL; +} + int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) { int error = 0; diff --git a/src/libcamera/pipeline/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi.cpp index d6749eaae759..337554501c82 100644 --- a/src/libcamera/pipeline/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi.cpp @@ -15,6 +15,7 @@ #include "media_device.h" #include "pipeline_handler.h" #include "utils.h" +#include "v4l2_controls.h" #include "v4l2_device.h" namespace libcamera { @@ -77,6 +78,9 @@ public: int start(Camera *camera) override; void stop(Camera *camera) override; + int writeControls(Camera *camera, Request *request); + int readControls(Camera *camera, Request *request); + int queueRequest(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator) override; @@ -293,6 +297,94 @@ void PipelineHandlerRPi::stop(Camera *camera) PipelineHandler::stop(camera); } +int PipelineHandlerRPi::writeControls(Camera *camera, Request *request) +{ + RPiCameraData *data = cameraData(camera); + + std::vector controls; + + for (Control c : request->controls()) { + if (c.value().isWrite()) { + switch (c.id()) { + case ManualGain: + break; + case ManualExposure: + break; + default: + LOG(RPI, Error) + << "Unhandled write control"; + break; + } + } + } + + /* Perhaps setControls could accept an empty vector/list as success? */ + if (!controls.empty()) + return data->unicam_->setControls(controls); + + return 0; +} + +/* This is becoming horrible. How can we improve this + * - Iterate reqeust controls to find controls to read + * - Add those to a new list + * - Query the device for that list to get controls + * - iterate returned list + * - For each control, search for corresponding control in request + * - Set control value to return value. + * - Return list. + * - Check for any values that were not updated? + */ +int PipelineHandlerRPi::readControls(Camera *camera, Request *request) +{ + RPiCameraData *data = cameraData(camera); + std::vector controlIDs; + std::vector controls; + + for (Control c : request->controls()) { + if (c.value().isRead()) { + LOG(RPI, Error) << "Read Control: " << c; + + switch (c.id()) { + case ManualGain: + controlIDs.push_back(V4L2_CID_ANALOGUE_GAIN); + break; + case ManualExposure: + controlIDs.push_back(V4L2_CID_EXPOSURE); + break; + default: + LOG(RPI, Error) + << "Unhandled write control"; + break; + } + } + } + + /* Perhaps setControls could accept an empty vector/list as success? */ + if (controlIDs.empty()) + return 0; + + controls = data->unicam_->getControls(controlIDs); + if (controls.empty()) + return -ENODATA; + + for (V4L2Control *ctrl : controls) { + switch(ctrl->id()) { + case V4L2_CID_ANALOGUE_GAIN: + //Control gain(ManualGain); //= request->controls().find(ManualGain); + auto it = request->controls().find(ManualGain); + Control gain = *it; + + //V4L2IntControl *c = static_cast(ctrl); + gain.value().set(0x88); + + break; + } + } + + return 0; +} + int PipelineHandlerRPi::queueRequest(Camera *camera, Request *request) { RPiCameraData *data = cameraData(camera); @@ -304,7 +396,15 @@ int PipelineHandlerRPi::queueRequest(Camera *camera, Request *request) return -ENOENT; } - int ret = data->isp_.capture->queueBuffer(buffer); + /* + * Handle 'set' controls first + * Todo: Validate ordering and timing. + */ + int ret = writeControls(camera, request); + if (ret < 0) + return ret; + + ret = data->isp_.capture->queueBuffer(buffer); if (ret < 0) return ret; @@ -394,6 +494,12 @@ void RPiCameraData::ispCaptureReady(Buffer *buffer) Request *request = queuedRequests_.front(); pipe_->completeBuffer(camera_, request, buffer); + + int ret = pipe_->readControls(camera_, request); + if (ret < 0) + LOG(RPI, Error) + << "Failed to read controls. No way to pass error"; + pipe_->completeRequest(camera_, request); } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 9b3eea2f6dd3..d618d9c34a22 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -88,6 +88,9 @@ public: int start(Camera *camera) override; void stop(Camera *camera) override; + int readControls(Camera *camera, Request *request) override; + int writeControls(Camera *camera, Request *request) override; + int queueRequest(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator) override; @@ -359,6 +362,22 @@ void PipelineHandlerRkISP1::stop(Camera *camera) activeCamera_ = nullptr; } +int PipelineHandlerRkISP1::readControls(Camera *camera, Request *request) +{ + LOG(RkISP1, Error) + << "NOT IMPLEMENTED"; + + return -EINVAL; +} + +int PipelineHandlerRkISP1::writeControls(Camera *camera, Request *request) +{ + LOG(RkISP1, Error) + << "NOT IMPLEMENTED"; + + return -EINVAL; +} + int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request *request) { RkISP1CameraData *data = cameraData(camera); diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index e2d02c0dafb8..216fbe237827 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -14,6 +14,7 @@ #include "media_device.h" #include "pipeline_handler.h" #include "utils.h" +#include "v4l2_controls.h" #include "v4l2_device.h" namespace libcamera { @@ -73,6 +74,9 @@ public: int start(Camera *camera) override; void stop(Camera *camera) override; + int readControls(Camera *camera, Request *request) override; + int writeControls(Camera *camera, Request *request) override; + int queueRequest(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator) override; @@ -252,6 +256,117 @@ void PipelineHandlerUVC::stop(Camera *camera) PipelineHandler::stop(camera); } +int PipelineHandlerUVC::writeControls(Camera *camera, Request *request) +{ + UVCCameraData *data = cameraData(camera); + + std::vector controls; + + for (Control c : request->controls()) { + if (c.value().isWrite()) { + switch (c.id()) { + case ManualBrightness: + controls.emplace_back(new V4L2IntControl(V4L2_CID_BRIGHTNESS, c.value().getInt())); + break; + + case IpaAwbEnable: + case ManualGain: + case ManualExposure: + break; + default: + LOG(UVC, Error) + << "Unhandled write control"; + break; + } + } + } + + /* Perhaps setControls could accept an empty vector/list as success? */ + if (!controls.empty()) { + int ret = data->video_->setControls(controls); + + /* It would be nice to avoid the need to manually delete + * the controls */ + for (V4L2Control *c : controls) + delete c; + + return ret; + } + + + return 0; +} + +int PipelineHandlerUVC::readControls(Camera *camera, Request *request) +{ + UVCCameraData *data = cameraData(camera); + std::vector controlIDs; + std::vector controls; + + for (Control c : request->controls()) { + if (c.value().isRead()) { + LOG(UVC, Error) << "Read Control: " << c; + + switch (c.id()) { + case ManualBrightness: + controlIDs.push_back(V4L2_CID_BRIGHTNESS); + break; + case ManualGain: + controlIDs.push_back(V4L2_CID_ANALOGUE_GAIN); + break; + case ManualExposure: + controlIDs.push_back(V4L2_CID_EXPOSURE); + break; + default: + LOG(UVC, Error) + << "Unhandled write control"; + break; + } + } + } + + /* Perhaps setControls could accept an empty vector/list as success? */ + if (controlIDs.empty()) + return 0; + + controls = data->video_->getControls(controlIDs); + if (controls.empty()) + return -ENODATA; + + for (V4L2Control *ctrl : controls) { + switch (ctrl->id()) { + case V4L2_CID_BRIGHTNESS: { + Control bright = *request->controls().find(ManualBrightness); + /* RFC: + * If the iterator doesn't find the control ^ + * then it causes a segfault, so this is really nasty... + * and where just a map might be better - (ornot?) as it + * will create the object at that key. + * Now , that *shouldn't* happen (we should only look + * for controls that were given to us in this + * function ... but + */ + V4L2IntControl *vc = static_cast(ctrl); + bright.value().set(vc->value()); + + LOG(UVC, Debug) << "Setting Brightness: " << bright; + + break; + } + case V4L2_CID_ANALOGUE_GAIN: { + Control gain = *request->controls().find(ManualGain); + V4L2IntControl *vc = static_cast(ctrl); + gain.value().set(vc->value()); + break; + } + default: + LOG(UVC, Warning) << "Unhandled V4L2 Control ID"; + } + } + + return 0; +} + int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) { UVCCameraData *data = cameraData(camera); @@ -263,7 +378,11 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) return -ENOENT; } - int ret = data->video_->queueBuffer(buffer); + int ret = writeControls(camera, request); + if (ret < 0) + return ret; + + ret = data->video_->queueBuffer(buffer); if (ret < 0) return ret; @@ -317,6 +436,12 @@ void UVCCameraData::bufferReady(Buffer *buffer) Request *request = queuedRequests_.front(); pipe_->completeBuffer(camera_, request, buffer); + + int ret = pipe_->readControls(camera_, request); + if (ret < 0) + LOG(UVC, Error) + << "Failed to read controls. No way to pass error"; + pipe_->completeRequest(camera_, request); } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 0e4eede351d8..9ecce48c0b12 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -67,6 +67,9 @@ public: int start(Camera *camera) override; void stop(Camera *camera) override; + int readControls(Camera *camera, Request *request) override; + int writeControls(Camera *camera, Request *request) override; + int queueRequest(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator) override; @@ -210,6 +213,22 @@ void PipelineHandlerVimc::stop(Camera *camera) PipelineHandler::stop(camera); } +int PipelineHandlerVimc::readControls(Camera *camera, Request *request) +{ + LOG(VIMC, Error) + << "NOT IMPLEMENTED"; + + return -EINVAL; +} + +int PipelineHandlerVimc::writeControls(Camera *camera, Request *request) +{ + LOG(VIMC, Error) + << "NOT IMPLEMENTED"; + + return -EINVAL; +} + int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) { VimcCameraData *data = cameraData(camera);