Patch Detail
Show a patch.
GET /api/1.1/patches/10691/?format=api
{ "id": 10691, "url": "https://patchwork.libcamera.org/api/1.1/patches/10691/?format=api", "web_url": "https://patchwork.libcamera.org/patch/10691/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20201221235345.1965878-2-niklas.soderlund@ragnatech.se>", "date": "2020-12-21T23:53:44", "name": "[libcamera-devel,v2,1/2] libcamera: pipeline_handler: Remove Camera argument from request handling", "commit_ref": "8992b3ffbba91b092945193c92fca38b5a24154a", "pull_url": null, "state": "accepted", "archived": false, "hash": "dcbcc457c9287cb0588154b6dda2435c73186426", "submitter": { "id": 5, "url": "https://patchwork.libcamera.org/api/1.1/people/5/?format=api", "name": "Niklas Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "delegate": { "id": 16, "url": "https://patchwork.libcamera.org/api/1.1/users/16/?format=api", "username": "neg", "first_name": "Niklas", "last_name": "Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "mbox": "https://patchwork.libcamera.org/patch/10691/mbox/", "series": [ { "id": 1539, "url": "https://patchwork.libcamera.org/api/1.1/series/1539/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1539", "date": "2020-12-21T23:53:43", "name": "libcamera: pipeline_handler: Remove Camera pointer from Request handling", "version": 2, "mbox": "https://patchwork.libcamera.org/series/1539/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/10691/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/10691/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 CF08BC0F1A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 21 Dec 2020 23:54:05 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A71A0615D5;\n\tTue, 22 Dec 2020 00:54:05 +0100 (CET)", "from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net\n\t[195.74.38.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B4EB76159A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 22 Dec 2020 00:54:03 +0100 (CET)", "from wyvern.dyn.berto.se (h-209-203.a463.priv.bahnhof.se\n\t[155.4.209.203]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA\n\tid cc7ee80e-43e7-11eb-a076-005056917f90;\n\tTue, 22 Dec 2020 00:54:01 +0100 (CET)" ], "X-Halon-ID": "cc7ee80e-43e7-11eb-a076-005056917f90", "Authorized-sender": "niklas.soderlund@fsdn.se", "From": "=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Tue, 22 Dec 2020 00:53:44 +0100", "Message-Id": "<20201221235345.1965878-2-niklas.soderlund@ragnatech.se>", "X-Mailer": "git-send-email 2.29.2", "In-Reply-To": "<20201221235345.1965878-1-niklas.soderlund@ragnatech.se>", "References": "<20201221235345.1965878-1-niklas.soderlund@ragnatech.se>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH v2 1/2] libcamera: pipeline_handler:\n\tRemove Camera argument from request handling", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Content-Type": "text/plain; charset=\"utf-8\"", "Content-Transfer-Encoding": "base64", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "There is no need to pass the Camera pointer to queueRequest(),\ncompleteBuffer() and completeRequest() as the Request also passed\ncontains the same information. Remove the Camera argument to avoid\nsituations where the information in the Request and the argument differ.\n\nThere is no functional change and no public API change as the interface\nis only used between the Camera and PipelineHandler.\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n Documentation/guides/pipeline-handler.rst | 4 ++--\n include/libcamera/internal/pipeline_handler.h | 7 +++----\n src/libcamera/camera.cpp | 2 +-\n src/libcamera/pipeline/ipu3/ipu3.cpp | 8 ++++----\n .../pipeline/raspberrypi/raspberrypi.cpp | 8 ++++----\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 ++---\n src/libcamera/pipeline/simple/simple.cpp | 12 ++++++------\n src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 ++--\n src/libcamera/pipeline/vimc/vimc.cpp | 4 ++--\n src/libcamera/pipeline_handler.cpp | 16 ++++++++--------\n 10 files changed, 34 insertions(+), 36 deletions(-)", "diff": "diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\nindex 63275a12e81bcd08..f4a31d3c9588f890 100644\n--- a/Documentation/guides/pipeline-handler.rst\n+++ b/Documentation/guides/pipeline-handler.rst\n@@ -1421,8 +1421,8 @@ code-base.\n {\n Request *request = buffer->request();\n \n- pipe_->completeBuffer(camera_, request, buffer);\n- pipe_->completeRequest(camera_, request);\n+ pipe_->completeBuffer(request, buffer);\n+ pipe_->completeRequest(request);\n }\n \n Testing a pipeline handler\ndiff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\nindex bd3c4a81d6389db6..5f9a26be1f7a9e9e 100644\n--- a/include/libcamera/internal/pipeline_handler.h\n+++ b/include/libcamera/internal/pipeline_handler.h\n@@ -81,11 +81,10 @@ public:\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+\tint queueRequest(Request *request);\n \n-\tbool completeBuffer(Camera *camera, Request *request,\n-\t\t\t FrameBuffer *buffer);\n-\tvoid completeRequest(Camera *camera, Request *request);\n+\tbool completeBuffer(Request *request, FrameBuffer *buffer);\n+\tvoid completeRequest(Request *request);\n \n \tconst char *name() const { return name_; }\n \ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex c1de1fee701a3240..d9b817871cbf5b85 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -997,7 +997,7 @@ int Camera::queueRequest(Request *request)\n \t}\n \n \treturn d->pipe_->invokeMethod(&PipelineHandler::queueRequest,\n-\t\t\t\t ConnectionTypeQueued, this, request);\n+\t\t\t\t ConnectionTypeQueued, request);\n }\n \n /**\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 8a1918d5e4c5b815..f1151733d9fe81ff 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -836,13 +836,13 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n {\n \tRequest *request = buffer->request();\n \n-\tif (!pipe_->completeBuffer(camera_, request, buffer))\n+\tif (!pipe_->completeBuffer(request, buffer))\n \t\t/* Request not completed yet, return here. */\n \t\treturn;\n \n \t/* Mark the request as complete. */\n \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n-\tpipe_->completeRequest(camera_, request);\n+\tpipe_->completeRequest(request);\n }\n \n /**\n@@ -865,10 +865,10 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n \t * now as there's no need for ImgU processing.\n \t */\n \tif (request->findBuffer(&rawStream_)) {\n-\t\tbool isComplete = pipe_->completeBuffer(camera_, request, buffer);\n+\t\tbool isComplete = pipe_->completeBuffer(request, buffer);\n \t\tif (isComplete) {\n \t\t\trequest->metadata().set(controls::draft::PipelineDepth, 2);\n-\t\t\tpipe_->completeRequest(camera_, request);\n+\t\t\tpipe_->completeRequest(request);\n \t\t\treturn;\n \t\t}\n \t}\ndiff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 7a5f5881b9b30129..f121328ee9a9b6f2 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -1507,10 +1507,10 @@ void RPiCameraData::clearIncompleteRequests()\n \t\t\t * request? If not, do so now.\n \t\t\t */\n \t\t\tif (buffer && buffer->request())\n-\t\t\t\tpipe_->completeBuffer(camera_, request, buffer);\n+\t\t\t\tpipe_->completeBuffer(request, buffer);\n \t\t}\n \n-\t\tpipe_->completeRequest(camera_, request);\n+\t\tpipe_->completeRequest(request);\n \t\trequestQueue_.pop_front();\n \t}\n }\n@@ -1534,7 +1534,7 @@ void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream)\n \t\t\t * Tag the buffer as completed, returning it to the\n \t\t\t * application.\n \t\t\t */\n-\t\t\tpipe_->completeBuffer(camera_, request, buffer);\n+\t\t\tpipe_->completeBuffer(request, buffer);\n \t\t} else {\n \t\t\t/*\n \t\t\t * This buffer was not part of the Request, or there is no\n@@ -1597,7 +1597,7 @@ void RPiCameraData::checkRequestCompleted()\n \t\tif (state_ != State::IpaComplete)\n \t\t\treturn;\n \n-\t\tpipe_->completeRequest(camera_, request);\n+\t\tpipe_->completeRequest(request);\n \t\trequestQueue_.pop_front();\n \t\trequestCompleted = true;\n \t}\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 021d0ffe3ffb39ed..5ce37febc1d7c11c 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -1135,15 +1135,14 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)\n \n \tdata->frameInfo_.destroy(info->frame);\n \n-\tcompleteRequest(activeCamera_, request);\n+\tcompleteRequest(request);\n }\n \n void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n {\n-\tASSERT(activeCamera_);\n \tRequest *request = buffer->request();\n \n-\tcompleteBuffer(activeCamera_, request, buffer);\n+\tcompleteBuffer(request, buffer);\n \ttryCompleteRequest(request);\n }\n \ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex b719171ab6d1a0be..6eb9ab0ce6ecd60a 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -899,8 +899,8 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)\n \t\t}\n \n \t\tRequest *request = buffer->request();\n-\t\tcompleteBuffer(activeCamera_, request, buffer);\n-\t\tcompleteRequest(activeCamera_, request);\n+\t\tcompleteBuffer(request, buffer);\n+\t\tcompleteRequest(request);\n \t\treturn;\n \t}\n \n@@ -924,8 +924,8 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)\n \n \t/* Otherwise simply complete the request. */\n \tRequest *request = buffer->request();\n-\tcompleteBuffer(activeCamera_, request, buffer);\n-\tcompleteRequest(activeCamera_, request);\n+\tcompleteBuffer(request, buffer);\n+\tcompleteRequest(request);\n }\n \n void SimplePipelineHandler::converterDone(FrameBuffer *input,\n@@ -936,8 +936,8 @@ void SimplePipelineHandler::converterDone(FrameBuffer *input,\n \n \t/* Complete the request. */\n \tRequest *request = output->request();\n-\tcompleteBuffer(activeCamera_, request, output);\n-\tcompleteRequest(activeCamera_, request);\n+\tcompleteBuffer(request, output);\n+\tcompleteRequest(request);\n \n \t/* Queue the input buffer back for capture. */\n \tdata->video_->queueBuffer(input);\ndiff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\nindex e54f45bb8825f11f..7cb310e205114333 100644\n--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n@@ -646,8 +646,8 @@ void UVCCameraData::bufferReady(FrameBuffer *buffer)\n {\n \tRequest *request = buffer->request();\n \n-\tpipe_->completeBuffer(camera_, request, buffer);\n-\tpipe_->completeRequest(camera_, request);\n+\tpipe_->completeBuffer(request, buffer);\n+\tpipe_->completeRequest(request);\n }\n \n REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC)\ndiff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\nindex 8bda746f3136427c..36325ffbbd7d01cc 100644\n--- a/src/libcamera/pipeline/vimc/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc/vimc.cpp\n@@ -519,8 +519,8 @@ void VimcCameraData::bufferReady(FrameBuffer *buffer)\n {\n \tRequest *request = buffer->request();\n \n-\tpipe_->completeBuffer(camera_, request, buffer);\n-\tpipe_->completeRequest(camera_, request);\n+\tpipe_->completeBuffer(request, buffer);\n+\tpipe_->completeRequest(request);\n }\n \n REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc)\ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex c75ebbd5a443cb87..26d6c07d2d6e69e9 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -376,8 +376,7 @@ const ControlList &PipelineHandler::properties(const Camera *camera) const\n \n /**\n * \\fn PipelineHandler::queueRequest()\n- * \\brief Queue a request to the camera\n- * \\param[in] camera The camera to queue the request to\n+ * \\brief Queue a request\n * \\param[in] request The request to queue\n *\n * This method queues a capture request to the pipeline handler for processing.\n@@ -392,8 +391,9 @@ const ControlList &PipelineHandler::properties(const Camera *camera) const\n *\n * \\return 0 on success or a negative error code otherwise\n */\n-int PipelineHandler::queueRequest(Camera *camera, Request *request)\n+int PipelineHandler::queueRequest(Request *request)\n {\n+\tCamera *camera = request->camera_;\n \tCameraData *data = cameraData(camera);\n \tdata->queuedRequests_.push_back(request);\n \n@@ -423,7 +423,6 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request)\n \n /**\n * \\brief Complete a buffer for a request\n- * \\param[in] camera The camera the request belongs to\n * \\param[in] request The request the buffer belongs to\n * \\param[in] buffer The buffer that has completed\n *\n@@ -439,16 +438,15 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request)\n * \\return True if all buffers contained in the request have completed, false\n * otherwise\n */\n-bool PipelineHandler::completeBuffer(Camera *camera, Request *request,\n-\t\t\t\t FrameBuffer *buffer)\n+bool PipelineHandler::completeBuffer(Request *request, FrameBuffer *buffer)\n {\n+\tCamera *camera = request->camera_;\n \tcamera->bufferCompleted.emit(request, buffer);\n \treturn request->completeBuffer(buffer);\n }\n \n /**\n * \\brief Signal request completion\n- * \\param[in] camera The camera that the request belongs to\n * \\param[in] request The request that has completed\n *\n * The pipeline handler shall call this method to notify the \\a camera that the\n@@ -461,8 +459,10 @@ bool PipelineHandler::completeBuffer(Camera *camera, Request *request,\n *\n * \\context This function shall be called from the CameraManager thread.\n */\n-void PipelineHandler::completeRequest(Camera *camera, Request *request)\n+void PipelineHandler::completeRequest(Request *request)\n {\n+\tCamera *camera = request->camera_;\n+\n \trequest->complete();\n \n \tCameraData *data = cameraData(camera);\n", "prefixes": [ "libcamera-devel", "v2", "1/2" ] }