[{"id":17515,"web_url":"https://patchwork.libcamera.org/comment/17515/","msgid":"","date":"2021-06-11T14:01:17","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi all,\n\nJust a nudge on this one....\n\nThanks,\nNaush\n\n\nOn Thu, 3 Jun 2021 at 13:43, Naushir Patuck wrote:\n\n> With the addition of FrameBuffer::cancel(), the logic to clear and return\n> pending requests can be simplified by not having to queue all the request\n> buffers to the device before calling streamOff().\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 46 ++++---------------\n> 1 file changed, 8 insertions(+), 38 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index a65b4568256c..887f8d0f7404 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -884,7 +884,9 @@ void PipelineHandlerRPi::stop(Camera *camera)\n> /* Disable SOF event generation. */\n> data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(false);\n>\n> - /* This also stops the streams. */\n> + for (auto const stream : data->streams_)\n> + stream->dev()->streamOff();\n> +\n> data->clearIncompleteRequests();\n> data->bayerQueue_ = {};\n> data->embeddedQueue_ = {};\n> @@ -1477,49 +1479,17 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer\n> *buffer)\n>\n> void RPiCameraData::clearIncompleteRequests()\n> {\n> - /*\n> - * Queue up any buffers passed in the request.\n> - * This is needed because streamOff() will then mark the buffers as\n> - * cancelled.\n> - */\n> - for (auto const request : requestQueue_) {\n> - for (auto const stream : streams_) {\n> - if (!stream->isExternal())\n> - continue;\n> -\n> - FrameBuffer *buffer = request->findBuffer(stream);\n> - if (buffer)\n> - stream->queueBuffer(buffer);\n> - }\n> - }\n> -\n> - /* Stop all streams. */\n> - for (auto const stream : streams_)\n> - stream->dev()->streamOff();\n> -\n> /*\n> * All outstanding requests (and associated buffers) must be\n> returned\n> - * back to the pipeline. The buffers would have been marked as\n> - * cancelled by the call to streamOff() earlier.\n> + * back to the pipeline.\n> */\n> while (!requestQueue_.empty()) {\n> Request *request = requestQueue_.front();\n> - /*\n> - * A request could be partially complete,\n> - * i.e. we have returned some buffers, but still waiting\n> - * for others or waiting for metadata.\n> - */\n> - for (auto const stream : streams_) {\n> - if (!stream->isExternal())\n> - continue;\n>\n> - FrameBuffer *buffer = request->findBuffer(stream);\n> - /*\n> - * Has the buffer already been handed back to the\n> - * request? If not, do so now.\n> - */\n> - if (buffer && buffer->request())\n> - pipe_->completeBuffer(request, buffer);\n> + for (auto &b : request->buffers()) {\n> + FrameBuffer *buffer = b.second;\n> + buffer->cancel();\n> + pipe_->completeBuffer(request, buffer);\n> }\n>\n> pipe_->completeRequest(request);\n> --\n> 2.25.1\n>\n>","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 97C0CBD78E\n\tfor ;\n\tFri, 11 Jun 2021 14:01:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EA0166892B;\n\tFri, 11 Jun 2021 16:01:35 +0200 (CEST)","from mail-lf1-x131.google.com (mail-lf1-x131.google.com\n\t[IPv6:2a00:1450:4864:20::131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E18C86029E\n\tfor ;\n\tFri, 11 Jun 2021 16:01:34 +0200 (CEST)","by mail-lf1-x131.google.com with SMTP id j20so8698146lfe.8\n\tfor ;\n\tFri, 11 Jun 2021 07:01:34 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"AnxEljdV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to;\n\tbh=fQHYZ/vn5P6pEKYRvEyr8IVvSnlH+zM0zuLvdklfNGc=;\n\tb=AnxEljdVMxm+HCEScEDL/61CistrAEtcjdRUPmiy9fgyQXfs8HKcv8fV35ZveSdZub\n\tuzMYqW6nC2k1u+99bGVDRMa5RxeumUadcmiRWVkGkkBKYHln3Aci5dvPqviGVJpCnr5X\n\tmiRtkfuJ9BuwAoCxc5NKv4xBOi7ly9h6BaD+9Buw1nKd7zTsvHq7XfftRREsDlL5Xebn\n\tEpXTs7GJPQjHEvpnUJMMWVol7jPCXTF8GPAgz4jGoG0AP2X1HtLsH5hEUUX7jquQehKG\n\tvRf7p5jAaxusB57J+uSPllhM78h/Xe37nnn86coUP5M0Ed8EI2W9OnB4Ty97meh4Ui7E\n\tCtMQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to;\n\tbh=fQHYZ/vn5P6pEKYRvEyr8IVvSnlH+zM0zuLvdklfNGc=;\n\tb=IMSpSQHVwOwXJvoel2/PkcadUpvapbuWyjgqZKufmEV6/2/BqbU4skb0iQLhKTH9ta\n\tuqdV0ZyB/rs7ddm+pKAWq05YLIPSJahoTlIEH0PXJ3tar9/mKiSVEyCAzLgqvMTzvddH\n\tOTE9ZVrfp0qG98D4QpOl+shRPBhrJ4ZaGGhlYeiGLwqJ6Q9H2/tUJ3IquAEGzhqhVh+h\n\tskzv35eV8Luz057NrsUa2V7p1FEDCCez35IjhtIWXj9umJhe4cGPO2FRCp+vGC7CL7uB\n\t4+k1rjef5fZkeAc/4EO4PuXxj4XT0g/ppibk5ncQ099AZkSV27rwSKMGau41poCALtsQ\n\tgbdQ==","X-Gm-Message-State":"AOAM532XX7Zxu6EaymznHzQOU7I0UsJuqzxl+PsuociJEoeI5u41cxTF\n\trykyl1thimizzRXlak0UHruAlhuiYW4vKR9lZsTotCTU8Kc=","X-Google-Smtp-Source":"ABdhPJzOlA+6UVek8/YFnOOp0yi3CKypeR5nQgfeQAEa1TeJd55OSsQbqeCc+cFahtj0TDvA0619iFOQJSPivE9WvGw=","X-Received":"by 2002:a05:6512:3fa5:: with SMTP id\n\tx37mr2756738lfa.617.1623420093708; \n\tFri, 11 Jun 2021 07:01:33 -0700 (PDT)","MIME-Version":"1.0","References":"<20210603124345.2492885-1-naush@raspberrypi.com>","In-Reply-To":"<20210603124345.2492885-1-naush@raspberrypi.com>","From":"Naushir Patuck ","Date":"Fri, 11 Jun 2021 15:01:17 +0100","Message-ID":"","To":"libcamera devel ","Content-Type":"multipart/alternative; boundary=\"00000000000001d40505c47df21a\"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17516,"web_url":"https://patchwork.libcamera.org/comment/17516/","msgid":"","date":"2021-06-11T14:38:17","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the tidy-up!\n\nI can't say I understand the details of this sufficiently to give a\nvery meaningful review, however, I get the point that you can simply\nstop the streams without having to queue buffers back first.\nNevertheless, I've been running with this for a while now with no ill\neffects, hence:\n\nTested-by: David Plowman \n\nThanks!\nDavid\n\nOn Thu, 3 Jun 2021 at 13:43, Naushir Patuck wrote:\n>\n> With the addition of FrameBuffer::cancel(), the logic to clear and return\n> pending requests can be simplified by not having to queue all the request\n> buffers to the device before calling streamOff().\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 46 ++++---------------\n> 1 file changed, 8 insertions(+), 38 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index a65b4568256c..887f8d0f7404 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -884,7 +884,9 @@ void PipelineHandlerRPi::stop(Camera *camera)\n> /* Disable SOF event generation. */\n> data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(false);\n>\n> - /* This also stops the streams. */\n> + for (auto const stream : data->streams_)\n> + stream->dev()->streamOff();\n> +\n> data->clearIncompleteRequests();\n> data->bayerQueue_ = {};\n> data->embeddedQueue_ = {};\n> @@ -1477,49 +1479,17 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n>\n> void RPiCameraData::clearIncompleteRequests()\n> {\n> - /*\n> - * Queue up any buffers passed in the request.\n> - * This is needed because streamOff() will then mark the buffers as\n> - * cancelled.\n> - */\n> - for (auto const request : requestQueue_) {\n> - for (auto const stream : streams_) {\n> - if (!stream->isExternal())\n> - continue;\n> -\n> - FrameBuffer *buffer = request->findBuffer(stream);\n> - if (buffer)\n> - stream->queueBuffer(buffer);\n> - }\n> - }\n> -\n> - /* Stop all streams. */\n> - for (auto const stream : streams_)\n> - stream->dev()->streamOff();\n> -\n> /*\n> * All outstanding requests (and associated buffers) must be returned\n> - * back to the pipeline. The buffers would have been marked as\n> - * cancelled by the call to streamOff() earlier.\n> + * back to the pipeline.\n> */\n> while (!requestQueue_.empty()) {\n> Request *request = requestQueue_.front();\n> - /*\n> - * A request could be partially complete,\n> - * i.e. we have returned some buffers, but still waiting\n> - * for others or waiting for metadata.\n> - */\n> - for (auto const stream : streams_) {\n> - if (!stream->isExternal())\n> - continue;\n>\n> - FrameBuffer *buffer = request->findBuffer(stream);\n> - /*\n> - * Has the buffer already been handed back to the\n> - * request? If not, do so now.\n> - */\n> - if (buffer && buffer->request())\n> - pipe_->completeBuffer(request, buffer);\n> + for (auto &b : request->buffers()) {\n> + FrameBuffer *buffer = b.second;\n> + buffer->cancel();\n> + pipe_->completeBuffer(request, buffer);\n> }\n>\n> pipe_->completeRequest(request);\n> --\n> 2.25.1\n>","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 8225DC320B\n\tfor ;\n\tFri, 11 Jun 2021 14:38:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E912E6892B;\n\tFri, 11 Jun 2021 16:38:30 +0200 (CEST)","from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com\n\t[IPv6:2a00:1450:4864:20::42f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6FD1D6029E\n\tfor ;\n\tFri, 11 Jun 2021 16:38:29 +0200 (CEST)","by mail-wr1-x42f.google.com with SMTP id a20so6373991wrc.0\n\tfor ;\n\tFri, 11 Jun 2021 07:38:29 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"iDL3L4Kq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=zVr39e1U/4lA8OfSVmrSdzNIK9FmGBK4EWONM2EhZao=;\n\tb=iDL3L4KqQW696LdbeShMbFRqefg1Ee0LT4bG+nYkw64Z53oLr6RgnUoGMvDqYF4nlq\n\to4J1HXRebMLFnH8GeAOFHMsYDIo5lwj+Qwp2LMruDGl7l9pWR3CB4BXW8S2JIF78BHWr\n\tOGrXLQFMK5um55YqNx8StnBraGlT9tVhTvUo8AF81zgtU4Vycj/+zVDp1499ybegeL3e\n\tWv8cSGllD1h4o2GNNCv7lSKUdzp8zOd4+T0r3flJzntcsJo/YDfdoDR0OeyH8y5N7Y2l\n\tkYycSXeuYLt5MvRgfYLxEvrzk3ckV/QID6SNfigxNGrw0oIisUB7SA8VkvmjtM54oWuY\n\tCMaA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=zVr39e1U/4lA8OfSVmrSdzNIK9FmGBK4EWONM2EhZao=;\n\tb=lUO96cnFwfS1UMk+87WXLPH6PBDI8dq/0E11ilLsExqTMytMynmD+Pjmly+PUGtAw+\n\tmjfPQtlT26i3u/m1vW5sC5ejuM6CjQiHRsbozEXtc6anCcUyT6vv8n/H/djhamvpthFi\n\tJbmbktSMCYMLMnB6Ycr6mtimp5ONbCwgLbF3JdYqy6A1+ZOedVi45e/rAwmTs13280uN\n\trhFAYW3PDC8gFoU89j2JnkrsEjjwotZ73fJeoWMPuvLHBQOPXTPdgKnRmKxwOrM+vT1Y\n\tlZ5NdtidPbdQ3bWgywnDydQXUfNwcNAhM6FYCX0T9QTvtzi8WjyxNuACPoj5h9yWCqiZ\n\tX2Yg==","X-Gm-Message-State":"AOAM533nE6BHF166OTwSCu87pGsHMdlgfM3PUbs9zd56/63xXTs+ZRB3\n\tTGddZ7KDOHs8r0FwDWO6U2uY0gl+Hzrga5L9TpkLtcyuLsszbg==","X-Google-Smtp-Source":"ABdhPJx2Sft5Ie4iE5N7U1fOei5DZoZtixIh6/buWhQCT+jZk25WyfteFnbAnqPaZ/kVtbP6ZTaZLPbLye1Qmwr1z5A=","X-Received":"by 2002:a5d:648a:: with SMTP id\n\to10mr4490345wri.274.1623422309038; \n\tFri, 11 Jun 2021 07:38:29 -0700 (PDT)","MIME-Version":"1.0","References":"<20210603124345.2492885-1-naush@raspberrypi.com>","In-Reply-To":"<20210603124345.2492885-1-naush@raspberrypi.com>","From":"David Plowman ","Date":"Fri, 11 Jun 2021 15:38:17 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17517,"web_url":"https://patchwork.libcamera.org/comment/17517/","msgid":"<96862d87-a6df-28fd-d40a-f256163f8dbf@ideasonboard.com>","date":"2021-06-11T14:51:03","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nOn 03/06/2021 13:43, Naushir Patuck wrote:\n> With the addition of FrameBuffer::cancel(), the logic to clear and return\n> pending requests can be simplified by not having to queue all the request\n> buffers to the device before calling streamOff().\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 46 ++++---------------\n> 1 file changed, 8 insertions(+), 38 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index a65b4568256c..887f8d0f7404 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -884,7 +884,9 @@ void PipelineHandlerRPi::stop(Camera *camera)\n> \t/* Disable SOF event generation. */\n> \tdata->unicam_[Unicam::Image].dev()->setFrameStartEnabled(false);\n> \n> -\t/* This also stops the streams. */\n> +\tfor (auto const stream : data->streams_)\n> +\t\tstream->dev()->streamOff();\n\nThat's nicer than hiding it in clearIncompleteRequests...\n\n\n> +\n> \tdata->clearIncompleteRequests();\n> \tdata->bayerQueue_ = {};\n> \tdata->embeddedQueue_ = {};\n> @@ -1477,49 +1479,17 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n> \n> void RPiCameraData::clearIncompleteRequests()\n> {\n> -\t/*\n> -\t * Queue up any buffers passed in the request.\n> -\t * This is needed because streamOff() will then mark the buffers as\n> -\t * cancelled.\n> -\t */\n> -\tfor (auto const request : requestQueue_) {\n> -\t\tfor (auto const stream : streams_) {\n> -\t\t\tif (!stream->isExternal())\n> -\t\t\t\tcontinue;\n> -\n> -\t\t\tFrameBuffer *buffer = request->findBuffer(stream);\n> -\t\t\tif (buffer)\n> -\t\t\t\tstream->queueBuffer(buffer);\n> -\t\t}\n> -\t}\n> -\n> -\t/* Stop all streams. */\n> -\tfor (auto const stream : streams_)\n> -\t\tstream->dev()->streamOff();\n> -\n> \t/*\n> \t * All outstanding requests (and associated buffers) must be returned\n> -\t * back to the pipeline. The buffers would have been marked as\n> -\t * cancelled by the call to streamOff() earlier.\n> +\t * back to the pipeline.\n> \t */\n> \twhile (!requestQueue_.empty()) {\n> \t\tRequest *request = requestQueue_.front();\n> -\t\t/*\n> -\t\t * A request could be partially complete,\n> -\t\t * i.e. we have returned some buffers, but still waiting\n> -\t\t * for others or waiting for metadata.\n> -\t\t */\n> -\t\tfor (auto const stream : streams_) {\n> -\t\t\tif (!stream->isExternal())\n> -\t\t\t\tcontinue;\n> \n> -\t\t\tFrameBuffer *buffer = request->findBuffer(stream);\n> -\t\t\t/*\n> -\t\t\t * Has the buffer already been handed back to the\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(request, buffer);\n> +\t\tfor (auto &b : request->buffers()) {\n> +\t\t\tFrameBuffer *buffer = b.second;\n> +\t\t\tbuffer->cancel();\n> +\t\t\tpipe_->completeBuffer(request, buffer);\n\nLooks good to me.\nAs far as I can tell, items on the requestQueue haven't been handed to\nany hardware yet, is that correct? (Which means there's no risk of any\nother action occurring from a stream on these buffers).\n\nAnd in fact, all streams are called with streamOff() before this anyway\n- so it should be good.\n\nReviewed-by: Kieran Bingham \n\n\n> \t\t}\n> \n> \t\tpipe_->completeRequest(request);\n>","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 B7B06BD78E\n\tfor ;\n\tFri, 11 Jun 2021 14:51:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 310A568931;\n\tFri, 11 Jun 2021 16:51:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 53D7C6029E\n\tfor ;\n\tFri, 11 Jun 2021 16:51:06 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C8B7A4AD;\n\tFri, 11 Jun 2021 16:51:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kqL2/oJ4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623423065;\n\tbh=IZXU5FlRLSgwNTfJMb71K/vQHBN1McyjwmSj9Ph/Gk8=;\n\th=Reply-To:To:References:From:Subject:Date:In-Reply-To:From;\n\tb=kqL2/oJ48ZIFnhdhoKmEH3bmi3eO3yrVCRVR4OxLVkXxmmy0rDSGoXhhR/trCCfs3\n\tPLUGMTZ6Ez2ueX8JWN4mRNxU9pGRus6S7DJ2v7G/L7A9P7Mbd4mAq2k9NXd9D5QCWr\n\tN+LsSr7HJA5U+g6zM5ntYpIXp1Y8URBMd4oTYNoo=","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210603124345.2492885-1-naush@raspberrypi.com>","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"<96862d87-a6df-28fd-d40a-f256163f8dbf@ideasonboard.com>","Date":"Fri, 11 Jun 2021 15:51:03 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.8.1","MIME-Version":"1.0","In-Reply-To":"<20210603124345.2492885-1-naush@raspberrypi.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","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","Reply-To":"kieran.bingham@ideasonboard.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17681,"web_url":"https://patchwork.libcamera.org/comment/17681/","msgid":"","date":"2021-06-22T08:36:43","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi all,\n\nAnother gentle nudge. This needs one more R-b tag to get merged.\n\nThanks,\nNaush\n\nOn Thu, 3 Jun 2021 at 13:43, Naushir Patuck wrote:\n\n> With the addition of FrameBuffer::cancel(), the logic to clear and return\n> pending requests can be simplified by not having to queue all the request\n> buffers to the device before calling streamOff().\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 46 ++++---------------\n> 1 file changed, 8 insertions(+), 38 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index a65b4568256c..887f8d0f7404 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -884,7 +884,9 @@ void PipelineHandlerRPi::stop(Camera *camera)\n> /* Disable SOF event generation. */\n> data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(false);\n>\n> - /* This also stops the streams. */\n> + for (auto const stream : data->streams_)\n> + stream->dev()->streamOff();\n> +\n> data->clearIncompleteRequests();\n> data->bayerQueue_ = {};\n> data->embeddedQueue_ = {};\n> @@ -1477,49 +1479,17 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer\n> *buffer)\n>\n> void RPiCameraData::clearIncompleteRequests()\n> {\n> - /*\n> - * Queue up any buffers passed in the request.\n> - * This is needed because streamOff() will then mark the buffers as\n> - * cancelled.\n> - */\n> - for (auto const request : requestQueue_) {\n> - for (auto const stream : streams_) {\n> - if (!stream->isExternal())\n> - continue;\n> -\n> - FrameBuffer *buffer = request->findBuffer(stream);\n> - if (buffer)\n> - stream->queueBuffer(buffer);\n> - }\n> - }\n> -\n> - /* Stop all streams. */\n> - for (auto const stream : streams_)\n> - stream->dev()->streamOff();\n> -\n> /*\n> * All outstanding requests (and associated buffers) must be\n> returned\n> - * back to the pipeline. The buffers would have been marked as\n> - * cancelled by the call to streamOff() earlier.\n> + * back to the pipeline.\n> */\n> while (!requestQueue_.empty()) {\n> Request *request = requestQueue_.front();\n> - /*\n> - * A request could be partially complete,\n> - * i.e. we have returned some buffers, but still waiting\n> - * for others or waiting for metadata.\n> - */\n> - for (auto const stream : streams_) {\n> - if (!stream->isExternal())\n> - continue;\n>\n> - FrameBuffer *buffer = request->findBuffer(stream);\n> - /*\n> - * Has the buffer already been handed back to the\n> - * request? If not, do so now.\n> - */\n> - if (buffer && buffer->request())\n> - pipe_->completeBuffer(request, buffer);\n> + for (auto &b : request->buffers()) {\n> + FrameBuffer *buffer = b.second;\n> + buffer->cancel();\n> + pipe_->completeBuffer(request, buffer);\n> }\n>\n> pipe_->completeRequest(request);\n> --\n> 2.25.1\n>\n>","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 E7CDFC321B\n\tfor ;\n\tTue, 22 Jun 2021 08:37:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2F58F6893C;\n\tTue, 22 Jun 2021 10:37:03 +0200 (CEST)","from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com\n\t[IPv6:2a00:1450:4864:20::12c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CA9B860297\n\tfor ;\n\tTue, 22 Jun 2021 10:37:00 +0200 (CEST)","by mail-lf1-x12c.google.com with SMTP id t17so14441731lfq.0\n\tfor ;\n\tTue, 22 Jun 2021 01:37:00 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"Z2TiYbRA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to;\n\tbh=Lxg4IFlD3fGuIgg1dnoVNOU2fhVb2EbZ+M/HI7kHBMg=;\n\tb=Z2TiYbRAG0irSZzNZF+d/PR7q/QPB7zkx7EUQDfXFJHeqYW6GHr82gW47Lle8zaJ6Z\n\t6YEe/BIyKO6p1eo7OZIOdF76+r2rHE6mMBxwrRIdWhb4QmazkOfXeIB54ka2VEzKyhTY\n\tgbImvFDM43KAZNc3jIZ5vlSSJ9h3b813tNPwu3QuzORnV5mZ/i25f2hIZJO/fMn29j3O\n\t20mWeef4mBJFJBK5y62JIvyTtokG/AQdDkfXiTrr3ub0ywHDKcFo3QcwJyPYoQnfnazM\n\txPchRXr2EY6ebu+B8AAVv6BS308IlSUh4gJPhByzhTwvvyyRvuK9Qjs/l8IQ64p/HqaS\n\t0PKg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to;\n\tbh=Lxg4IFlD3fGuIgg1dnoVNOU2fhVb2EbZ+M/HI7kHBMg=;\n\tb=SYP/hueiaefop8+pFHZTZyl2kwfdwooRokxHxMRJIMGFLwk5pXvFkAZmEWUjY8Y0ch\n\t1Gc1v1cfuV5+aec+tFvDlqez7mJFrV1BFnWNai+yBMSn4uwWHCRSlsVELXfvqakWVOAU\n\t5jde3C8f1kgWf3gbWTxBrcPi6+FnC/AGIa1u64I5xJmfAUmBeuDvMUZx/Pl18Efeg9Ae\n\tJl7FFy0gF8N4HWpNrg65SF30gSp0mSot82Uwt9ElR0zU9cWhftwr0YbGUIam7bAdZq7Q\n\t8qX+NICtbOWYngKZD+CD7lyvy8mJU3QOxuKpnULWWxY3hCiRjhdLZ8KL9AN0HqCm5FsS\n\tFdsQ==","X-Gm-Message-State":"AOAM532rxHt6khTVjFJW9DrLsNR/z4emQNfFWNXAM99E6Tzh+2NP+5Gs\n\tNGeazkSxpg0d/nlWO58lTZawsDglfcl2KeFFbckCOVG+Bn4=","X-Google-Smtp-Source":"ABdhPJzjJcMqjt7KPcmqFniTSOQIwhbd0JDXbobw7aVusNwkXUCf03XU2o/iB/ygS4A+TYBwkPShh4UYYZKFmmMrcEY=","X-Received":"by 2002:a05:6512:1184:: with SMTP id\n\tg4mr1992471lfr.567.1624351019836; \n\tTue, 22 Jun 2021 01:36:59 -0700 (PDT)","MIME-Version":"1.0","References":"<20210603124345.2492885-1-naush@raspberrypi.com>","In-Reply-To":"<20210603124345.2492885-1-naush@raspberrypi.com>","From":"Naushir Patuck ","Date":"Tue, 22 Jun 2021 09:36:43 +0100","Message-ID":"","To":"libcamera devel ","Content-Type":"multipart/alternative; boundary=\"000000000000873b7905c556b198\"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17731,"web_url":"https://patchwork.libcamera.org/comment/17731/","msgid":"<20210623132126.cqqt3kkj6e4dhfa5@uno.localdomain>","date":"2021-06-23T13:21:26","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Naush,\n\nOn Tue, Jun 22, 2021 at 09:36:43AM +0100, Naushir Patuck wrote:\n> Hi all,\n>\n> Another gentle nudge. This needs one more R-b tag to get merged.\n\nSorry for being late.\nThis looks good to me! One minor to be fixed when applying below\n\n\n>\n> Thanks,\n> Naush\n>\n> On Thu, 3 Jun 2021 at 13:43, Naushir Patuck wrote:\n>\n> > With the addition of FrameBuffer::cancel(), the logic to clear and return\n> > pending requests can be simplified by not having to queue all the request\n> > buffers to the device before calling streamOff().\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 46 ++++---------------\n> > 1 file changed, 8 insertions(+), 38 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index a65b4568256c..887f8d0f7404 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -884,7 +884,9 @@ void PipelineHandlerRPi::stop(Camera *camera)\n> > /* Disable SOF event generation. */\n> > data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(false);\n> >\n> > - /* This also stops the streams. */\n> > + for (auto const stream : data->streams_)\n> > + stream->dev()->streamOff();\n> > +\n> > data->clearIncompleteRequests();\n> > data->bayerQueue_ = {};\n> > data->embeddedQueue_ = {};\n> > @@ -1477,49 +1479,17 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer\n> > *buffer)\n> >\n> > void RPiCameraData::clearIncompleteRequests()\n> > {\n> > - /*\n> > - * Queue up any buffers passed in the request.\n> > - * This is needed because streamOff() will then mark the buffers as\n> > - * cancelled.\n> > - */\n> > - for (auto const request : requestQueue_) {\n> > - for (auto const stream : streams_) {\n> > - if (!stream->isExternal())\n> > - continue;\n> > -\n> > - FrameBuffer *buffer = request->findBuffer(stream);\n> > - if (buffer)\n> > - stream->queueBuffer(buffer);\n> > - }\n> > - }\n> > -\n> > - /* Stop all streams. */\n> > - for (auto const stream : streams_)\n> > - stream->dev()->streamOff();\n> > -\n> > /*\n> > * All outstanding requests (and associated buffers) must be\n> > returned\n> > - * back to the pipeline. The buffers would have been marked as\n> > - * cancelled by the call to streamOff() earlier.\n> > + * back to the pipeline.\n\nback to the 'camera' ? back to the 'application' ? we're actually in\nthe pipeline here :)\n\nApart from this:\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> > */\n> > while (!requestQueue_.empty()) {\n> > Request *request = requestQueue_.front();\n> > - /*\n> > - * A request could be partially complete,\n> > - * i.e. we have returned some buffers, but still waiting\n> > - * for others or waiting for metadata.\n> > - */\n> > - for (auto const stream : streams_) {\n> > - if (!stream->isExternal())\n> > - continue;\n> >\n> > - FrameBuffer *buffer = request->findBuffer(stream);\n> > - /*\n> > - * Has the buffer already been handed back to the\n> > - * request? If not, do so now.\n> > - */\n> > - if (buffer && buffer->request())\n> > - pipe_->completeBuffer(request, buffer);\n> > + for (auto &b : request->buffers()) {\n> > + FrameBuffer *buffer = b.second;\n> > + buffer->cancel();\n> > + pipe_->completeBuffer(request, buffer);\n> > }\n> >\n> > pipe_->completeRequest(request);\n> > --\n> > 2.25.1\n> >\n> >","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 22A13C321B\n\tfor ;\n\tWed, 23 Jun 2021 13:20:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CC61768937;\n\tWed, 23 Jun 2021 15:20:39 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 67F4968932\n\tfor ;\n\tWed, 23 Jun 2021 15:20:38 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 9407DE0010;\n\tWed, 23 Jun 2021 13:20:37 +0000 (UTC)"],"Date":"Wed, 23 Jun 2021 15:21:26 +0200","From":"Jacopo Mondi ","To":"Naushir Patuck ","Message-ID":"<20210623132126.cqqt3kkj6e4dhfa5@uno.localdomain>","References":"<20210603124345.2492885-1-naush@raspberrypi.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17732,"web_url":"https://patchwork.libcamera.org/comment/17732/","msgid":"","date":"2021-06-23T13:31:13","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nThank you for your review.\n\nOn Wed, 23 Jun 2021 at 14:20, Jacopo Mondi wrote:\n\n> Hi Naush,\n>\n> On Tue, Jun 22, 2021 at 09:36:43AM +0100, Naushir Patuck wrote:\n> > Hi all,\n> >\n> > Another gentle nudge. This needs one more R-b tag to get merged.\n>\n> Sorry for being late.\n> This looks good to me! One minor to be fixed when applying below\n>\n>\n> >\n> > Thanks,\n> > Naush\n> >\n> > On Thu, 3 Jun 2021 at 13:43, Naushir Patuck \n> wrote:\n> >\n> > > With the addition of FrameBuffer::cancel(), the logic to clear and\n> return\n> > > pending requests can be simplified by not having to queue all the\n> request\n> > > buffers to the device before calling streamOff().\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > .../pipeline/raspberrypi/raspberrypi.cpp | 46 ++++---------------\n> > > 1 file changed, 8 insertions(+), 38 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index a65b4568256c..887f8d0f7404 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -884,7 +884,9 @@ void PipelineHandlerRPi::stop(Camera *camera)\n> > > /* Disable SOF event generation. */\n> > >\n> data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(false);\n> > >\n> > > - /* This also stops the streams. */\n> > > + for (auto const stream : data->streams_)\n> > > + stream->dev()->streamOff();\n> > > +\n> > > data->clearIncompleteRequests();\n> > > data->bayerQueue_ = {};\n> > > data->embeddedQueue_ = {};\n> > > @@ -1477,49 +1479,17 @@ void\n> RPiCameraData::ispOutputDequeue(FrameBuffer\n> > > *buffer)\n> > >\n> > > void RPiCameraData::clearIncompleteRequests()\n> > > {\n> > > - /*\n> > > - * Queue up any buffers passed in the request.\n> > > - * This is needed because streamOff() will then mark the\n> buffers as\n> > > - * cancelled.\n> > > - */\n> > > - for (auto const request : requestQueue_) {\n> > > - for (auto const stream : streams_) {\n> > > - if (!stream->isExternal())\n> > > - continue;\n> > > -\n> > > - FrameBuffer *buffer =\n> request->findBuffer(stream);\n> > > - if (buffer)\n> > > - stream->queueBuffer(buffer);\n> > > - }\n> > > - }\n> > > -\n> > > - /* Stop all streams. */\n> > > - for (auto const stream : streams_)\n> > > - stream->dev()->streamOff();\n> > > -\n> > > /*\n> > > * All outstanding requests (and associated buffers) must be\n> > > returned\n> > > - * back to the pipeline. The buffers would have been marked as\n> > > - * cancelled by the call to streamOff() earlier.\n> > > + * back to the pipeline.\n>\n> back to the 'camera' ? back to the 'application' ? we're actually in\n> the pipeline here :)\n>\n\n\"back to the application\" is probably the right term here.\nWould you be able to change this before submitting, or should I post\nan updated patch?\n\nThanks!\nNaush\n\n\n\n>\n> Apart from this:\n> Reviewed-by: Jacopo Mondi \n>\n> Thanks\n> j\n>\n> > > */\n> > > while (!requestQueue_.empty()) {\n> > > Request *request = requestQueue_.front();\n> > > - /*\n> > > - * A request could be partially complete,\n> > > - * i.e. we have returned some buffers, but still\n> waiting\n> > > - * for others or waiting for metadata.\n> > > - */\n> > > - for (auto const stream : streams_) {\n> > > - if (!stream->isExternal())\n> > > - continue;\n> > >\n> > > - FrameBuffer *buffer =\n> request->findBuffer(stream);\n> > > - /*\n> > > - * Has the buffer already been handed back to\n> the\n> > > - * request? If not, do so now.\n> > > - */\n> > > - if (buffer && buffer->request())\n> > > - pipe_->completeBuffer(request, buffer);\n> > > + for (auto &b : request->buffers()) {\n> > > + FrameBuffer *buffer = b.second;\n> > > + buffer->cancel();\n> > > + pipe_->completeBuffer(request, buffer);\n> > > }\n> > >\n> > > pipe_->completeRequest(request);\n> > > --\n> > > 2.25.1\n> > >\n> > >\n>","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 8F897C321A\n\tfor ;\n\tWed, 23 Jun 2021 13:31:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1D79968937;\n\tWed, 23 Jun 2021 15:31:32 +0200 (CEST)","from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com\n\t[IPv6:2a00:1450:4864:20::12e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 50E9368932\n\tfor ;\n\tWed, 23 Jun 2021 15:31:30 +0200 (CEST)","by mail-lf1-x12e.google.com with SMTP id j2so4108311lfg.9\n\tfor ;\n\tWed, 23 Jun 2021 06:31:30 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"RFukOPlR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=bGpFSQrORYZknFJKQaF5k+65Xqi2mvVEeI2/pXqK018=;\n\tb=RFukOPlRC1b8WicZGIJ5S6mPe2jZrt2ayd8+eVdcAzd0TbYedlilCbrhLPcK2ORph3\n\tR6BaW6bRUEGjWRhblBG1FMeOZ0bu5vIBVXjE9NbZ32nUDKxUzphbkYRRxE7xkXDqqOp+\n\tAoXAWGiUktkFjYsT5UGHYYDMzwsXucxFBa5dgTqpcdXFYJleRCPkCOtpE3gwykkegBSU\n\twyaGbbEd4oeu8olHTeekU2U9aDAwBctnzHeD943XX8RpBKCNy5QsSFNv7I7JxkHimyXN\n\tU2UPQu9K4Nl2IPeinpdXfeeEWYfYdDoood2Hz7njRxZyojlBZJisvGyi/rh5X/ExIEot\n\tLrZA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=bGpFSQrORYZknFJKQaF5k+65Xqi2mvVEeI2/pXqK018=;\n\tb=LswUq66Fn37jDT8tH8gZdhPkXt1rsvsVDlZ8dYntCaeY2155JQ59tKpk1K4PHlR0x2\n\tjUhzUNcFGDw0KC4Zarl9Y9CrC35DPWwApLWTNdhv/NQL8+g26o6P1ggDof1y+bR4O0Wh\n\t9qQIlbXNWvjsmBeA6pkKAriPZM5OWLn1ouPdaDNRgVRw1ofxFAbjy9seCHfNQQyFCdRX\n\tkHdif51EdrKKtLRMQ378+oGHI+HmuPVAoqdjWUUqDYrOI0is3h7vSuJKl5NDbHq55mk/\n\t6RwYzpuUHSQJ5i/ifMf47EZgjJMqh1soc1jIu3mC3OI2gsojgll3iPu8SiOe7eidxw8a\n\tHGrg==","X-Gm-Message-State":"AOAM531bTd3nX0igKL9zUhrD++75xqCsvLtPydyXpdtSWk4KARtdqKhp\n\tc8Abl+mfB9gW+L0xo8YNVJkgH+gxLl27IuL+ei5RPQ==","X-Google-Smtp-Source":"ABdhPJx3mc6AyV7anEM5UFuhiA6wrK5jt9swrrY48vGr8fcQjTwN0Q4LYPn7/Mkmx9bSB6uxkr2e7dh4yugsqbtWd2I=","X-Received":"by 2002:ac2:548e:: with SMTP id\n\tt14mr7023590lfk.617.1624455089480; \n\tWed, 23 Jun 2021 06:31:29 -0700 (PDT)","MIME-Version":"1.0","References":"<20210603124345.2492885-1-naush@raspberrypi.com>\n\t\n\t<20210623132126.cqqt3kkj6e4dhfa5@uno.localdomain>","In-Reply-To":"<20210623132126.cqqt3kkj6e4dhfa5@uno.localdomain>","From":"Naushir Patuck ","Date":"Wed, 23 Jun 2021 14:31:13 +0100","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"0000000000008ff87305c56eeccf\"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17733,"web_url":"https://patchwork.libcamera.org/comment/17733/","msgid":"<20210623140622.ipth5trwo2jznxnn@uno.localdomain>","date":"2021-06-23T14:06:22","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"No worries, we'll fix when applying\n\nThanks\n j\n\nOn Wed, Jun 23, 2021 at 02:31:13PM +0100, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n> Thank you for your review.\n>\n> On Wed, 23 Jun 2021 at 14:20, Jacopo Mondi wrote:\n>\n> > Hi Naush,\n> >\n> > On Tue, Jun 22, 2021 at 09:36:43AM +0100, Naushir Patuck wrote:\n> > > Hi all,\n> > >\n> > > Another gentle nudge. This needs one more R-b tag to get merged.\n> >\n> > Sorry for being late.\n> > This looks good to me! One minor to be fixed when applying below\n> >\n> >\n> > >\n> > > Thanks,\n> > > Naush\n> > >\n> > > On Thu, 3 Jun 2021 at 13:43, Naushir Patuck \n> > wrote:\n> > >\n> > > > With the addition of FrameBuffer::cancel(), the logic to clear and\n> > return\n> > > > pending requests can be simplified by not having to queue all the\n> > request\n> > > > buffers to the device before calling streamOff().\n> > > >\n> > > > Signed-off-by: Naushir Patuck \n> > > > ---\n> > > > .../pipeline/raspberrypi/raspberrypi.cpp | 46 ++++---------------\n> > > > 1 file changed, 8 insertions(+), 38 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > index a65b4568256c..887f8d0f7404 100644\n> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > @@ -884,7 +884,9 @@ void PipelineHandlerRPi::stop(Camera *camera)\n> > > > /* Disable SOF event generation. */\n> > > >\n> > data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(false);\n> > > >\n> > > > - /* This also stops the streams. */\n> > > > + for (auto const stream : data->streams_)\n> > > > + stream->dev()->streamOff();\n> > > > +\n> > > > data->clearIncompleteRequests();\n> > > > data->bayerQueue_ = {};\n> > > > data->embeddedQueue_ = {};\n> > > > @@ -1477,49 +1479,17 @@ void\n> > RPiCameraData::ispOutputDequeue(FrameBuffer\n> > > > *buffer)\n> > > >\n> > > > void RPiCameraData::clearIncompleteRequests()\n> > > > {\n> > > > - /*\n> > > > - * Queue up any buffers passed in the request.\n> > > > - * This is needed because streamOff() will then mark the\n> > buffers as\n> > > > - * cancelled.\n> > > > - */\n> > > > - for (auto const request : requestQueue_) {\n> > > > - for (auto const stream : streams_) {\n> > > > - if (!stream->isExternal())\n> > > > - continue;\n> > > > -\n> > > > - FrameBuffer *buffer =\n> > request->findBuffer(stream);\n> > > > - if (buffer)\n> > > > - stream->queueBuffer(buffer);\n> > > > - }\n> > > > - }\n> > > > -\n> > > > - /* Stop all streams. */\n> > > > - for (auto const stream : streams_)\n> > > > - stream->dev()->streamOff();\n> > > > -\n> > > > /*\n> > > > * All outstanding requests (and associated buffers) must be\n> > > > returned\n> > > > - * back to the pipeline. The buffers would have been marked as\n> > > > - * cancelled by the call to streamOff() earlier.\n> > > > + * back to the pipeline.\n> >\n> > back to the 'camera' ? back to the 'application' ? we're actually in\n> > the pipeline here :)\n> >\n>\n> \"back to the application\" is probably the right term here.\n> Would you be able to change this before submitting, or should I post\n> an updated patch?\n>\n> Thanks!\n> Naush\n>\n>\n>\n> >\n> > Apart from this:\n> > Reviewed-by: Jacopo Mondi \n> >\n> > Thanks\n> > j\n> >\n> > > > */\n> > > > while (!requestQueue_.empty()) {\n> > > > Request *request = requestQueue_.front();\n> > > > - /*\n> > > > - * A request could be partially complete,\n> > > > - * i.e. we have returned some buffers, but still\n> > waiting\n> > > > - * for others or waiting for metadata.\n> > > > - */\n> > > > - for (auto const stream : streams_) {\n> > > > - if (!stream->isExternal())\n> > > > - continue;\n> > > >\n> > > > - FrameBuffer *buffer =\n> > request->findBuffer(stream);\n> > > > - /*\n> > > > - * Has the buffer already been handed back to\n> > the\n> > > > - * request? If not, do so now.\n> > > > - */\n> > > > - if (buffer && buffer->request())\n> > > > - pipe_->completeBuffer(request, buffer);\n> > > > + for (auto &b : request->buffers()) {\n> > > > + FrameBuffer *buffer = b.second;\n> > > > + buffer->cancel();\n> > > > + pipe_->completeBuffer(request, buffer);\n> > > > }\n> > > >\n> > > > pipe_->completeRequest(request);\n> > > > --\n> > > > 2.25.1\n> > > >\n> > > >\n> >","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 AE0DAC321B\n\tfor ;\n\tWed, 23 Jun 2021 14:05:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 30F1768937;\n\tWed, 23 Jun 2021 16:05:35 +0200 (CEST)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 24AA468932\n\tfor ;\n\tWed, 23 Jun 2021 16:05:34 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id 5BD4A240012;\n\tWed, 23 Jun 2021 14:05:32 +0000 (UTC)"],"Date":"Wed, 23 Jun 2021 16:06:22 +0200","From":"Jacopo Mondi ","To":"Naushir Patuck ","Message-ID":"<20210623140622.ipth5trwo2jznxnn@uno.localdomain>","References":"<20210603124345.2492885-1-naush@raspberrypi.com>\n\t\n\t<20210623132126.cqqt3kkj6e4dhfa5@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify\n\tRPiCameraData::clearIncompleteRequests()","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]