{"id":1684,"url":"https://patchwork.libcamera.org/api/1.1/patches/1684/?format=json","web_url":"https://patchwork.libcamera.org/patch/1684/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20190713172351.25452-4-laurent.pinchart@ideasonboard.com>","date":"2019-07-13T17:23:38","name":"[libcamera-devel,v2,03/16] libcamera: pipeline_handler: Simplify request completion","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"319267e9d63aabc22097cc70dd338906348bcd32","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/1684/mbox/","series":[{"id":430,"url":"https://patchwork.libcamera.org/api/1.1/series/430/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=430","date":"2019-07-13T17:23:35","name":"Add support for external buffers","version":2,"mbox":"https://patchwork.libcamera.org/series/430/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/1684/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/1684/checks/","tags":{},"headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AAEB361764\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 13 Jul 2019 19:24:29 +0200 (CEST)","from pendragon.ideasonboard.com (softbank126209254147.bbtec.net\n\t[126.209.254.147])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7F419443\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 13 Jul 2019 19:24:28 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1563038669;\n\tbh=JpS0md5byVimHIwfq6tkafs+RmoUx4IkuE22MpNVQzA=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=Cu96yvCw4WvZn2Rn3cE8c+ULoyozfrrX3ztRzfkE75Peo8kHJsdRUBLa7GX50KGPf\n\tVG4ttl+qfBxkfLxlOYbi8lNVueqR7T+8sO6WcogW5dNfZw3+lUNwVW/Vbj9xjxRGHR\n\twGIJ205qRdkWhA6p2I3LCa4Zm5NgTHGFX0DGZwJA=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Sat, 13 Jul 2019 20:23:38 +0300","Message-Id":"<20190713172351.25452-4-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.21.0","In-Reply-To":"<20190713172351.25452-1-laurent.pinchart@ideasonboard.com>","References":"<20190713172351.25452-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v2 03/16] libcamera: pipeline_handler:\n\tSimplify request completion","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Sat, 13 Jul 2019 17:24:29 -0000"},"content":"libcamera guarantees that requests complete in sequence. This\nrequirement is currently pushed down to pipeline handlers. Three out of\nfour of our pipeline handlers implement that requirement based on the\nsole assumption that buffers will always complete in sequeuence, while\nthe IPU3 pipeline handler implements a more complex logic.\n\nIt turns out that the logic can be moved to the base PipelineHandler\nclass with support from the Request class. Do so to simplify the\npipeline handlers.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/libcamera/pipeline/ipu3/ipu3.cpp     | 10 ++-------\n src/libcamera/pipeline/rkisp1/rkisp1.cpp |  4 +---\n src/libcamera/pipeline/uvcvideo.cpp      |  2 +-\n src/libcamera/pipeline/vimc.cpp          |  2 +-\n src/libcamera/pipeline_handler.cpp       | 26 ++++++++++++++++--------\n 5 files changed, 22 insertions(+), 22 deletions(-)","diff":"diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex e4efb9722f76..1abd20e5fee5 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -927,14 +927,8 @@ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer)\n \t\t/* Request not completed yet, return here. */\n \t\treturn;\n \n-\t/* Complete the pending requests in queueing order. */\n-\twhile (1) {\n-\t\trequest = queuedRequests_.front();\n-\t\tif (request->hasPendingBuffers())\n-\t\t\tbreak;\n-\n-\t\tpipe_->completeRequest(camera_, request);\n-\t}\n+\t/* Mark the request as complete. */\n+\tpipe_->completeRequest(camera_, request);\n }\n \n /**\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 4a5898d25f91..383236435a7f 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -491,9 +491,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n void PipelineHandlerRkISP1::bufferReady(Buffer *buffer)\n {\n \tASSERT(activeCamera_);\n-\n-\tRkISP1CameraData *data = cameraData(activeCamera_);\n-\tRequest *request = data->queuedRequests_.front();\n+\tRequest *request = buffer->request();\n \n \tcompleteBuffer(activeCamera_, request, buffer);\n \tcompleteRequest(activeCamera_, request);\ndiff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\nindex 72313cfd246f..387d71ef567c 100644\n--- a/src/libcamera/pipeline/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo.cpp\n@@ -382,7 +382,7 @@ int UVCCameraData::init(MediaEntity *entity)\n \n void UVCCameraData::bufferReady(Buffer *buffer)\n {\n-\tRequest *request = queuedRequests_.front();\n+\tRequest *request = buffer->request();\n \n \tpipe_->completeBuffer(camera_, request, buffer);\n \tpipe_->completeRequest(camera_, request);\ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex f8a58be060bb..ff2529c974fd 100644\n--- a/src/libcamera/pipeline/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc.cpp\n@@ -376,7 +376,7 @@ int VimcCameraData::init(MediaDevice *media)\n \n void VimcCameraData::bufferReady(Buffer *buffer)\n {\n-\tRequest *request = queuedRequests_.front();\n+\tRequest *request = buffer->request();\n \n \tpipe_->completeBuffer(camera_, request, buffer);\n \tpipe_->completeRequest(camera_, request);\ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex 0283e4e5ad51..c609200252f1 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -391,8 +391,9 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request)\n  * This method shall be called by pipeline handlers to signal completion of the\n  * \\a buffer part of the \\a request. It notifies applications of buffer\n  * completion and updates the request's internal buffer tracking. The request\n- * is not completed automatically when the last buffer completes, pipeline\n- * handlers shall complete requests explicitly with completeRequest().\n+ * is not completed automatically when the last buffer completes to give\n+ * pipeline handlers a chance to perform any operation that may still be\n+ * needed. They shall complete requests explicitly with completeRequest().\n  *\n  * \\return True if all buffers contained in the request have completed, false\n  * otherwise\n@@ -413,17 +414,24 @@ bool PipelineHandler::completeBuffer(Camera *camera, Request *request,\n  * request has completed. The request is deleted and shall not be accessed once\n  * this method returns.\n  *\n- * The pipeline handler shall ensure that requests complete in the same order\n- * they are submitted.\n+ * This method ensures that requests will be returned to the application in\n+ * submission order, the pipeline handler may call it on any complete request\n+ * without any ordering constraint.\n  */\n void PipelineHandler::completeRequest(Camera *camera, Request *request)\n {\n-\tCameraData *data = cameraData(camera);\n-\tASSERT(request == data->queuedRequests_.front());\n-\tdata->queuedRequests_.pop_front();\n-\n \trequest->complete(Request::RequestComplete);\n-\tcamera->requestComplete(request);\n+\n+\tCameraData *data = cameraData(camera);\n+\n+\twhile (!data->queuedRequests_.empty()) {\n+\t\trequest = data->queuedRequests_.front();\n+\t\tif (request->hasPendingBuffers())\n+\t\t\tbreak;\n+\n+\t\tdata->queuedRequests_.pop_front();\n+\t\tcamera->requestComplete(request);\n+\t}\n }\n \n /**\n","prefixes":["libcamera-devel","v2","03/16"]}