[{"id":14083,"web_url":"https://patchwork.libcamera.org/comment/14083/","msgid":"","date":"2020-12-05T21:07:29","subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: pipeline: Pass\n\tlibcamera controls into pipeline_handler::start()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Fri, Dec 04, 2020 at 03:31:19PM +0000, Naushir Patuck wrote:\n> Applications now have the ability to pass in controls that need to be\n> applied on startup, rather than doing it through Request where there might\n> be some frames of delay in getting the controls applied.\n> \n> This commit adds the ability to pass in a set of libcamera controls into\n> the pipeline handlers through the pipeline_handler::start() method. These\n> controls are provided by the application through the camera::start()\n> public API.\n> \n> Signed-off-by: Naushir Patuck \n> Reviewed-by: David Plowman \n> Tested-by: David Plowman \n> Reviewed-by: Jacopo Mondi \n> ---\n> Documentation/guides/pipeline-handler.rst | 4 ++--\n> include/libcamera/camera.h | 2 +-\n> include/libcamera/internal/pipeline_handler.h | 2 +-\n> src/libcamera/camera.cpp | 11 ++++++-----\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 ++--\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--\n> src/libcamera/pipeline/simple/simple.cpp | 4 ++--\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 ++--\n> src/libcamera/pipeline/vimc/vimc.cpp | 4 ++--\n> src/libcamera/pipeline_handler.cpp | 1 +\n> 11 files changed, 23 insertions(+), 21 deletions(-)\n> \n> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> index 57aee455..63275a12 100644\n> --- a/Documentation/guides/pipeline-handler.rst\n> +++ b/Documentation/guides/pipeline-handler.rst\n> @@ -209,7 +209,7 @@ methods for the overridden class members.\n> int exportFrameBuffers(Camera *camera, Stream *stream,\n> std::vector> *buffers) override;\n> \n> - int start(Camera *camera) override;\n> + int start(Camera *camera, ControlList *controls) override;\n> void stop(Camera *camera) override;\n> \n> int queueRequestDevice(Camera *camera, Request *request) override;\n> @@ -239,7 +239,7 @@ methods for the overridden class members.\n> return -1;\n> }\n> \n> - int PipelineHandlerVivid::start(Camera *camera)\n> + int PipelineHandlerVivid::start(Camera *camera, ControlList *controls)\n> {\n> return -1;\n> }\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 5c5f1a05..f94f8599 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -103,7 +103,7 @@ public:\n> \tstd::unique_ptr createRequest(uint64_t cookie = 0);\n> \tint queueRequest(Request *request);\n> \n> -\tint start();\n> +\tint start(ControlList *controls = nullptr);\n> \tint stop();\n> \n> private:\n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index c12c8904..bd3c4a81 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -78,7 +78,7 @@ public:\n> \tvirtual int exportFrameBuffers(Camera *camera, Stream *stream,\n> \t\t\t\t std::vector> *buffers) = 0;\n> \n> -\tvirtual int start(Camera *camera) = 0;\n> +\tvirtual int start(Camera *camera, ControlList *controls) = 0;\n> \tvirtual void stop(Camera *camera) = 0;\n> \n> \tint queueRequest(Camera *camera, Request *request);\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index bcb7b046..ffb63fb2 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -1003,9 +1003,10 @@ int Camera::queueRequest(Request *request)\n> /**\n> * \\brief Start capture from camera\n\nMissing\n\n * \\param[in] controls Controls to be applied before starting the Camera\n\n> *\n> - * Start the camera capture session. Once the camera is started the application\n> - * can queue requests to the camera to process and return to the application\n> - * until the capture session is terminated with \\a stop().\n> + * Start the camera capture session, optionally providing a list of controls to\n> + * action before starting. Once the camera is started the application can queue\n\ns/action/apply/ ?\n\n> + * requests to the camera to process and return to the application until the\n> + * capture session is terminated with \\a stop().\n> *\n> * \\context This function may only be called when the camera is in the\n> * Configured state as defined in \\ref camera_operation, and shall be\n> @@ -1016,7 +1017,7 @@ int Camera::queueRequest(Request *request)\n> * \\retval -ENODEV The camera has been disconnected from the system\n> * \\retval -EACCES The camera is not in a state where it can be started\n> */\n> -int Camera::start()\n> +int Camera::start(ControlList *controls)\n> {\n> \tPrivate *const d = LIBCAMERA_D_PTR();\n> \n> @@ -1027,7 +1028,7 @@ int Camera::start()\n> \tLOG(Camera, Debug) << \"Starting capture\";\n> \n> \tret = d->pipe_->invokeMethod(&PipelineHandler::start,\n> -\t\t\t\t ConnectionTypeBlocking, this);\n> +\t\t\t\t ConnectionTypeBlocking, this, controls);\n> \tif (ret)\n> \t\treturn ret;\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 4cedb32b..8a1918d5 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -105,7 +105,7 @@ public:\n> \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> \t\t\t std::vector> *buffers) override;\n> \n> -\tint start(Camera *camera) override;\n> +\tint start(Camera *camera, ControlList *controls) override;\n> \tvoid stop(Camera *camera) override;\n> \n> \tint queueRequestDevice(Camera *camera, Request *request) override;\n> @@ -596,7 +596,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n> \treturn 0;\n> }\n> \n> -int PipelineHandlerIPU3::start(Camera *camera)\n> +int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *controls)\n\nCould you wrap the line ?\n\nint PipelineHandlerIPU3::start(Camera *camera,\n\t\t\t [[maybe_unused]] ControlList *controls)\n\nSame in a few places below.\n\nReviewed-by: Laurent Pinchart \n\n> {\n> \tIPU3CameraData *data = cameraData(camera);\n> \tCIO2Device *cio2 = &data->cio2_;\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 6fcdf557..9937db73 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -242,7 +242,7 @@ public:\n> \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> \t\t\t std::vector> *buffers) override;\n> \n> -\tint start(Camera *camera) override;\n> +\tint start(Camera *camera, ControlList *controls) override;\n> \tvoid stop(Camera *camera) override;\n> \n> \tint queueRequestDevice(Camera *camera, Request *request) override;\n> @@ -731,7 +731,7 @@ int PipelineHandlerRPi::exportFrameBuffers([[maybe_unused]] Camera *camera, Stre\n> \treturn ret;\n> }\n> \n> -int PipelineHandlerRPi::start(Camera *camera)\n> +int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *controls)\n> {\n> \tRPiCameraData *data = cameraData(camera);\n> \tint ret;\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 6e74a49a..4e959fde 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -187,7 +187,7 @@ public:\n> \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> \t\t\t std::vector> *buffers) override;\n> \n> -\tint start(Camera *camera) override;\n> +\tint start(Camera *camera, ControlList *controls) override;\n> \tvoid stop(Camera *camera) override;\n> \n> \tint queueRequestDevice(Camera *camera, Request *request) override;\n> @@ -832,7 +832,7 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n> \treturn 0;\n> }\n> \n> -int PipelineHandlerRkISP1::start(Camera *camera)\n> +int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *controls)\n> {\n> \tRkISP1CameraData *data = cameraData(camera);\n> \tint ret;\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 0d3078f7..b047aeb9 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -126,7 +126,7 @@ public:\n> \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> \t\t\t std::vector> *buffers) override;\n> \n> -\tint start(Camera *camera) override;\n> +\tint start(Camera *camera, ControlList *controls) override;\n> \tvoid stop(Camera *camera) override;\n> \n> \tbool match(DeviceEnumerator *enumerator) override;\n> @@ -646,7 +646,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> \t\treturn data->video_->exportBuffers(count, buffers);\n> }\n> \n> -int SimplePipelineHandler::start(Camera *camera)\n> +int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] ControlList *controls)\n> {\n> \tSimpleCameraData *data = cameraData(camera);\n> \tV4L2VideoDevice *video = data->video_;\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 0f3241cc..87b0f03d 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -76,7 +76,7 @@ public:\n> \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> \t\t\t std::vector> *buffers) override;\n> \n> -\tint start(Camera *camera) override;\n> +\tint start(Camera *camera, ControlList *controls) override;\n> \tvoid stop(Camera *camera) override;\n> \n> \tint queueRequestDevice(Camera *camera, Request *request) override;\n> @@ -236,7 +236,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,\n> \treturn data->video_->exportBuffers(count, buffers);\n> }\n> \n> -int PipelineHandlerUVC::start(Camera *camera)\n> +int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] ControlList *controls)\n> {\n> \tUVCCameraData *data = cameraData(camera);\n> \tunsigned int count = data->stream_.configuration().bufferCount;\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 914b6b54..d81b8598 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -92,7 +92,7 @@ public:\n> \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> \t\t\t std::vector> *buffers) override;\n> \n> -\tint start(Camera *camera) override;\n> +\tint start(Camera *camera, ControlList *controls) override;\n> \tvoid stop(Camera *camera) override;\n> \n> \tint queueRequestDevice(Camera *camera, Request *request) override;\n> @@ -313,7 +313,7 @@ int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,\n> \treturn data->video_->exportBuffers(count, buffers);\n> }\n> \n> -int PipelineHandlerVimc::start(Camera *camera)\n> +int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] ControlList *controls)\n> {\n> \tVimcCameraData *data = cameraData(camera);\n> \tunsigned int count = data->stream_.configuration().bufferCount;\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 894200ee..c75ebbd5 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -351,6 +351,7 @@ const ControlList &PipelineHandler::properties(const Camera *camera) const\n> * \\fn PipelineHandler::start()\n> * \\brief Start capturing from a group of streams\n> * \\param[in] camera The camera to start\n> + * \\param[in] controls Controls to be applied before starting the Camera\n> *\n> * Start the group of streams that have been configured for capture by\n> * \\a configure(). The intended caller of this method is the Camera class which","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 15FA3BDB1F\n\tfor ;\n\tSat, 5 Dec 2020 21:07:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8C437635F2;\n\tSat, 5 Dec 2020 22:07:33 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 29AAC60327\n\tfor ;\n\tSat, 5 Dec 2020 22:07:32 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9AE8BD2F;\n\tSat, 5 Dec 2020 22:07:31 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZG3rVZO0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1607202451;\n\tbh=0Z6SMlQxffc1gyvHeBLJSYcgZdwfkQMIccw1roVvjNI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZG3rVZO0nHaEE7T53Rixb6y6ANbEYrwy0TYN1EIBeN2XNvAoAhJfS4l+GZjDGVvaZ\n\tQHTrEfxzsOf+771Xi00YGQstYrxSVHzgVK7+u83MfbqAKEaD8A3fLcc6et0pmS5IGX\n\tJ6sF8vPtS6hLes1EWnQtLzrowocbdVFlHOGWJ3WI=","Date":"Sat, 5 Dec 2020 23:07:29 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20201204153121.66136-1-naush@raspberrypi.com>\n\t<20201204153121.66136-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201204153121.66136-2-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: pipeline: Pass\n\tlibcamera controls into pipeline_handler::start()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]