[{"id":24179,"web_url":"https://patchwork.libcamera.org/comment/24179/","msgid":"<5aa5111e-68d7-6812-8419-d70717db1743@ideasonboard.com>","date":"2022-07-27T08:41:12","subject":"Re: [libcamera-devel] [PATCH v3 4/9] ipu3: Add StillCapture stream\n\tand imgu1 param buffers","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Harvey,\n\nOn 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:\n> From: Harvey Yang <chenghaoyang@chromium.org>\n>\n> This patch adds the StillCapture stream and imgu1 param buffers.\n\n\nIt might be worth to add that we will ignore statistics for still \ncapture, as they aren't needed for 3A here, right?\n\n> The following patches will enable imgu1 to handle StillCapture stream\n> specifically, when the imgu0 is needed to handle video/preview streams.\n>\n> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>\n> ---\n>   src/libcamera/pipeline/ipu3/frames.cpp | 23 +++++++++++++++++++++--\n>   src/libcamera/pipeline/ipu3/frames.h   |  8 +++++++-\n>   src/libcamera/pipeline/ipu3/ipu3.cpp   | 20 +++++++++++++++++++-\n>   3 files changed, 47 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp\n> index a4c3477c..f9b705a5 100644\n> --- a/src/libcamera/pipeline/ipu3/frames.cpp\n> +++ b/src/libcamera/pipeline/ipu3/frames.cpp\n> @@ -23,7 +23,8 @@ IPU3Frames::IPU3Frames()\n>   }\n>   \n>   void IPU3Frames::init(const std::vector<std::unique_ptr<FrameBuffer>> &paramBuffers,\n> -\t\t      const std::vector<std::unique_ptr<FrameBuffer>> &statBuffers)\n> +\t\t      const std::vector<std::unique_ptr<FrameBuffer>> &statBuffers,\n> +\t\t      const std::vector<std::unique_ptr<FrameBuffer>> &captureParamBuffers)\n>   {\n>   \tfor (const std::unique_ptr<FrameBuffer> &buffer : paramBuffers)\n>   \t\tavailableParamBuffers_.push(buffer.get());\n> @@ -31,6 +32,9 @@ void IPU3Frames::init(const std::vector<std::unique_ptr<FrameBuffer>> &paramBuff\n>   \tfor (const std::unique_ptr<FrameBuffer> &buffer : statBuffers)\n>   \t\tavailableStatBuffers_.push(buffer.get());\n>   \n> +\tfor (const std::unique_ptr<FrameBuffer> &buffer : captureParamBuffers)\n> +\t\tavailableCaptureParamBuffers_.push(buffer.get());\n> +\n>   \tframeInfo_.clear();\n>   }\n>   \n> @@ -38,6 +42,7 @@ void IPU3Frames::clear()\n>   {\n>   \tavailableParamBuffers_ = {};\n\n\nalternatively should we rename this to, availableVfParamBuffers_ to mark \nthe distinction?\n\n>   \tavailableStatBuffers_ = {};\n> +\tavailableCaptureParamBuffers_ = {};\n>   }\n>   \n>   IPU3Frames::Info *IPU3Frames::create(Request *request)\n> @@ -54,14 +59,22 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)\n>   \t\treturn nullptr;\n>   \t}\n>   \n> +\tif (availableCaptureParamBuffers_.empty()) {\n> +\t\tLOG(IPU3, Debug) << \"Capture parameters buffer underrun\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n>   \tFrameBuffer *paramBuffer = availableParamBuffers_.front();\n\n\nTies into the earlier comment, maybe vfParamBuffer ? ... and so on below..\n\n>   \tFrameBuffer *statBuffer = availableStatBuffers_.front();\n> +\tFrameBuffer *captureParamBuffer = availableCaptureParamBuffers_.front();\n>   \n>   \tparamBuffer->_d()->setRequest(request);\n>   \tstatBuffer->_d()->setRequest(request);\n> +\tcaptureParamBuffer->_d()->setRequest(request);\n>   \n>   \tavailableParamBuffers_.pop();\n>   \tavailableStatBuffers_.pop();\n> +\tavailableCaptureParamBuffers_.pop();\n>   \n>   \t/* \\todo Remove the dynamic allocation of Info */\n>   \tstd::unique_ptr<Info> info = std::make_unique<Info>();\n> @@ -71,7 +84,9 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)\n>   \tinfo->rawBuffer = nullptr;\n>   \tinfo->paramBuffer = paramBuffer;\n>   \tinfo->statBuffer = statBuffer;\n> +\tinfo->captureParamBuffer = captureParamBuffer;\n>   \tinfo->paramDequeued = false;\n> +\tinfo->captureParamDequeued = false;\n>   \tinfo->metadataProcessed = false;\n>   \n>   \tframeInfo_[id] = std::move(info);\n> @@ -84,6 +99,7 @@ void IPU3Frames::remove(IPU3Frames::Info *info)\n>   \t/* Return params and stat buffer for reuse. */\n>   \tavailableParamBuffers_.push(info->paramBuffer);\n>   \tavailableStatBuffers_.push(info->statBuffer);\n> +\tavailableCaptureParamBuffers_.push(info->captureParamBuffer);\n>   \n>   \t/* Delete the extended frame information. */\n>   \tframeInfo_.erase(info->id);\n> @@ -102,6 +118,9 @@ bool IPU3Frames::tryComplete(IPU3Frames::Info *info)\n>   \tif (!info->paramDequeued)\n>   \t\treturn false;\n>   \n> +\tif (!info->captureParamDequeued)\n> +\t\treturn false;\n> +\n>   \tremove(info);\n>   \n>   \tbufferAvailable.emit();\n> @@ -131,7 +150,7 @@ IPU3Frames::Info *IPU3Frames::find(FrameBuffer *buffer)\n>   \t\t\t\treturn info;\n>   \n>   \t\tif (info->rawBuffer == buffer || info->paramBuffer == buffer ||\n> -\t\t    info->statBuffer == buffer)\n> +\t\t    info->statBuffer == buffer || info->captureParamBuffer == buffer)\n>   \t\t\treturn info;\n>   \t}\n>   \n> diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h\n> index 6e3cb915..8fcb8a14 100644\n> --- a/src/libcamera/pipeline/ipu3/frames.h\n> +++ b/src/libcamera/pipeline/ipu3/frames.h\n> @@ -36,16 +36,20 @@ public:\n>   \t\tFrameBuffer *paramBuffer;\n>   \t\tFrameBuffer *statBuffer;\n>   \n> +\t\tFrameBuffer *captureParamBuffer;\n> +\n>   \t\tControlList effectiveSensorControls;\n>   \n>   \t\tbool paramDequeued;\n> +\t\tbool captureParamDequeued;\n>   \t\tbool metadataProcessed;\n>   \t};\n>   \n>   \tIPU3Frames();\n>   \n>   \tvoid init(const std::vector<std::unique_ptr<FrameBuffer>> &paramBuffers,\n> -\t\t  const std::vector<std::unique_ptr<FrameBuffer>> &statBuffers);\n> +\t\t  const std::vector<std::unique_ptr<FrameBuffer>> &statBuffers,\n> +\t\t  const std::vector<std::unique_ptr<FrameBuffer>> &captureParamBuffers);\n>   \tvoid clear();\n>   \n>   \tInfo *create(Request *request);\n> @@ -61,6 +65,8 @@ private:\n>   \tstd::queue<FrameBuffer *> availableParamBuffers_;\n>   \tstd::queue<FrameBuffer *> availableStatBuffers_;\n>   \n> +\tstd::queue<FrameBuffer *> availableCaptureParamBuffers_;\n> +\n>   \tstd::map<unsigned int, std::unique_ptr<Info>> frameInfo_;\n>   };\n>   \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index e219f704..a201c5ca 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -70,6 +70,7 @@ public:\n>   \tStream outStream_;\n>   \tStream vfStream_;\n>   \tStream rawStream_;\n> +\tStream outCaptureStream_;\n\n\nIf there is a outCaptureStream_ now, should we be dropping outStream_ \nand associated buffer allocations for outStream_ ?\n\nMy guess is yes, let's see what happens in subsequent patches.\n\nFor this one,\n\nReviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n\n>   \n>   \tRectangle cropRegion_;\n>   \tbool supportsFlips_;\n> @@ -696,6 +697,8 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,\n>   \t\treturn imgu0_.viewfinder_->exportBuffers(count, buffers);\n>   \telse if (stream == &data->rawStream_)\n>   \t\treturn data->cio2_.exportBuffers(count, buffers);\n> +\telse if (stream == &data->outCaptureStream_)\n> +\t\treturn imgu1_.output_->exportBuffers(count, buffers);\n>   \n>   \treturn -EINVAL;\n>   }\n> @@ -718,11 +721,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n>   \t\tdata->outStream_.configuration().bufferCount,\n>   \t\tdata->vfStream_.configuration().bufferCount,\n>   \t\tdata->rawStream_.configuration().bufferCount,\n> +\t\tdata->outCaptureStream_.configuration().bufferCount,\n>   \t});\n>   \n>   \tret = imgu0_.allocateBuffers(bufferCount);\n>   \tif (ret < 0)\n>   \t\treturn ret;\n> +\tret = imgu1_.allocateBuffers(bufferCount);\n> +\tif (ret < 0) {\n> +\t\timgu0_.freeBuffers();\n> +\t\treturn ret;\n> +\t}\n>   \n>   \t/* Map buffers to the IPA. */\n>   \tunsigned int ipaBufferId = 1;\n> @@ -737,9 +746,14 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n>   \t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n>   \t}\n>   \n> +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu1_.paramBuffers_) {\n> +\t\tbuffer->setCookie(ipaBufferId++);\n> +\t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> +\t}\n> +\n>   \tdata->ipa_->mapBuffers(ipaBuffers_);\n>   \n> -\tdata->frameInfos_.init(imgu0_.paramBuffers_, imgu0_.statBuffers_);\n> +\tdata->frameInfos_.init(imgu0_.paramBuffers_, imgu0_.statBuffers_, imgu1_.paramBuffers_);\n>   \tdata->frameInfos_.bufferAvailable.connect(\n>   \t\tdata, &IPU3CameraData::queuePendingRequests);\n>   \n> @@ -760,6 +774,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n>   \tipaBuffers_.clear();\n>   \n>   \timgu0_.freeBuffers();\n> +\timgu1_.freeBuffers();\n>   \n>   \treturn 0;\n>   }\n> @@ -1143,6 +1158,7 @@ int PipelineHandlerIPU3::registerCameras()\n>   \t\t\t&data->outStream_,\n>   \t\t\t&data->vfStream_,\n>   \t\t\t&data->rawStream_,\n> +\t\t\t&data->outCaptureStream_,\n>   \t\t};\n>   \t\tCIO2Device *cio2 = &data->cio2_;\n>   \n> @@ -1318,6 +1334,8 @@ void IPU3CameraData::paramsBufferReady(unsigned int id)\n>   \timgu0_->param_->queueBuffer(info->paramBuffer);\n>   \timgu0_->stat_->queueBuffer(info->statBuffer);\n>   \timgu0_->input_->queueBuffer(info->rawBuffer);\n> +\n> +\tinfo->captureParamDequeued = true;\n>   }\n>   \n>   void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata)","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 07F36BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Jul 2022 08:41:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CBB3E63316;\n\tWed, 27 Jul 2022 10:41:20 +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 18FDC6330E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Jul 2022 10:41:20 +0200 (CEST)","from [IPV6:2401:4900:1f3e:f7a:bc8f:12ed:b45f:c35d] (unknown\n\t[IPv6:2401:4900:1f3e:f7a:bc8f:12ed:b45f:c35d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 43CEF835;\n\tWed, 27 Jul 2022 10:41:17 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658911280;\n\tbh=f7xIkt+A5gUfF4LsvKT1En9tMqER2aJ/2CFa09pBS7U=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=jVpRTFVBruGhLw2LgotQ+HjDXJnQUrPwdDp1GSVV3R987YUmObRndp3I2FBxKI6dg\n\tfwCfnGcLfZgockMXbgPGkwIymYt3/GDLUi5MbGEJFTLGUrS3idrQo/hFG249F/rvGq\n\tEJFubzAut7uRv4fjHvJsvHERDtddJ6y/FmuWuMzOOz1XRM1zzUOEsVdhVZFfSzUPxM\n\tBnECOpdN/bLy1e5MCBRQyWM7RcWFl/0GdWKpvEKs7YqJ2maKLhWesZ5rpkMZgQ4yw5\n\tIQ8DiR0z3n6oG1etJf3bovOoShqCw4XqZSOEY7D0XFcXQm0hCxmOph65aNUeZCd5y7\n\th7Lrc913gcMhg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658911279;\n\tbh=f7xIkt+A5gUfF4LsvKT1En9tMqER2aJ/2CFa09pBS7U=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=RUxXiAwP3TiNGps+G7tqBFiLtKEDdwGbXATdIKQ7JiTh7TdOz4UyQW7xnNTlCd9oJ\n\tbWXGIJkb4FiRC+goch8W8vbxmtsaNt4YCRRKIHSrlVdRUpLZARpgWo8djo4FKMZNqK\n\t4Mu1p04+Gu9jOVzLkr4VZnSzjEok69m5/FB2sJTE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"RUxXiAwP\"; dkim-atps=neutral","Message-ID":"<5aa5111e-68d7-6812-8419-d70717db1743@ideasonboard.com>","Date":"Wed, 27 Jul 2022 14:11:12 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Harvey Yang <chenghaoyang@chromium.org>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220629103018.4025635-1-chenghaoyang@google.com>\n\t<20220629103018.4025635-5-chenghaoyang@google.com>","In-Reply-To":"<20220629103018.4025635-5-chenghaoyang@google.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 4/9] ipu3: Add StillCapture stream\n\tand imgu1 param buffers","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>","From":"Umang Jain via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Umang Jain <umang.jain@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24297,"web_url":"https://patchwork.libcamera.org/comment/24297/","msgid":"<CAEB1ahtwOh5NPJLHeXyFxvVE=Xushe6TAuhAGMsMv+ehDSOmWA@mail.gmail.com>","date":"2022-08-02T10:30:47","subject":"Re: [libcamera-devel] [PATCH v3 4/9] ipu3: Add StillCapture stream\n\tand imgu1 param buffers","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Umang,\n\nOn Wed, Jul 27, 2022 at 4:41 PM Umang Jain <umang.jain@ideasonboard.com>\nwrote:\n\n> Hi Harvey,\n>\n> On 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:\n> > From: Harvey Yang <chenghaoyang@chromium.org>\n> >\n> > This patch adds the StillCapture stream and imgu1 param buffers.\n>\n>\n> It might be worth to add that we will ignore statistics for still\n> capture, as they aren't needed for 3A here, right?\n>\n>\nExactly, thanks!\n\n\n> > The following patches will enable imgu1 to handle StillCapture stream\n> > specifically, when the imgu0 is needed to handle video/preview streams.\n> >\n> > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>\n> > ---\n> >   src/libcamera/pipeline/ipu3/frames.cpp | 23 +++++++++++++++++++++--\n> >   src/libcamera/pipeline/ipu3/frames.h   |  8 +++++++-\n> >   src/libcamera/pipeline/ipu3/ipu3.cpp   | 20 +++++++++++++++++++-\n> >   3 files changed, 47 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/frames.cpp\n> b/src/libcamera/pipeline/ipu3/frames.cpp\n> > index a4c3477c..f9b705a5 100644\n> > --- a/src/libcamera/pipeline/ipu3/frames.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/frames.cpp\n> > @@ -23,7 +23,8 @@ IPU3Frames::IPU3Frames()\n> >   }\n> >\n> >   void IPU3Frames::init(const std::vector<std::unique_ptr<FrameBuffer>>\n> &paramBuffers,\n> > -                   const std::vector<std::unique_ptr<FrameBuffer>>\n> &statBuffers)\n> > +                   const std::vector<std::unique_ptr<FrameBuffer>>\n> &statBuffers,\n> > +                   const std::vector<std::unique_ptr<FrameBuffer>>\n> &captureParamBuffers)\n> >   {\n> >       for (const std::unique_ptr<FrameBuffer> &buffer : paramBuffers)\n> >               availableParamBuffers_.push(buffer.get());\n> > @@ -31,6 +32,9 @@ void IPU3Frames::init(const\n> std::vector<std::unique_ptr<FrameBuffer>> &paramBuff\n> >       for (const std::unique_ptr<FrameBuffer> &buffer : statBuffers)\n> >               availableStatBuffers_.push(buffer.get());\n> >\n> > +     for (const std::unique_ptr<FrameBuffer> &buffer :\n> captureParamBuffers)\n> > +             availableCaptureParamBuffers_.push(buffer.get());\n> > +\n> >       frameInfo_.clear();\n> >   }\n> >\n> > @@ -38,6 +42,7 @@ void IPU3Frames::clear()\n> >   {\n> >       availableParamBuffers_ = {};\n>\n>\n> alternatively should we rename this to, availableVfParamBuffers_ to mark\n> the distinction?\n>\n>\nI'm not sure if \"availableVfParamBuffers_\" describes it better though...\nIt'll be used as ImgU0's param buffers, and ImgU0 is used for video\nstreams. It might not always be used for preview, especially when a JPEG\nstream is not requested. IIUC \"viewfinder\" is mostly describing the\n\"preview\" stream, right? (Although Han-lin told me that the viewfinder node\nin ImgU might be more powerful, in terms of field of view, than the output\nnode, which confuses me a lot)\n\n\n\n> >       availableStatBuffers_ = {};\n> > +     availableCaptureParamBuffers_ = {};\n> >   }\n> >\n> >   IPU3Frames::Info *IPU3Frames::create(Request *request)\n> > @@ -54,14 +59,22 @@ IPU3Frames::Info *IPU3Frames::create(Request\n> *request)\n> >               return nullptr;\n> >       }\n> >\n> > +     if (availableCaptureParamBuffers_.empty()) {\n> > +             LOG(IPU3, Debug) << \"Capture parameters buffer underrun\";\n> > +             return nullptr;\n> > +     }\n> > +\n> >       FrameBuffer *paramBuffer = availableParamBuffers_.front();\n>\n>\n> Ties into the earlier comment, maybe vfParamBuffer ? ... and so on below..\n>\n>\nditto\n\n\n> >       FrameBuffer *statBuffer = availableStatBuffers_.front();\n> > +     FrameBuffer *captureParamBuffer =\n> availableCaptureParamBuffers_.front();\n> >\n> >       paramBuffer->_d()->setRequest(request);\n> >       statBuffer->_d()->setRequest(request);\n> > +     captureParamBuffer->_d()->setRequest(request);\n> >\n> >       availableParamBuffers_.pop();\n> >       availableStatBuffers_.pop();\n> > +     availableCaptureParamBuffers_.pop();\n> >\n> >       /* \\todo Remove the dynamic allocation of Info */\n> >       std::unique_ptr<Info> info = std::make_unique<Info>();\n> > @@ -71,7 +84,9 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)\n> >       info->rawBuffer = nullptr;\n> >       info->paramBuffer = paramBuffer;\n> >       info->statBuffer = statBuffer;\n> > +     info->captureParamBuffer = captureParamBuffer;\n> >       info->paramDequeued = false;\n> > +     info->captureParamDequeued = false;\n> >       info->metadataProcessed = false;\n> >\n> >       frameInfo_[id] = std::move(info);\n> > @@ -84,6 +99,7 @@ void IPU3Frames::remove(IPU3Frames::Info *info)\n> >       /* Return params and stat buffer for reuse. */\n> >       availableParamBuffers_.push(info->paramBuffer);\n> >       availableStatBuffers_.push(info->statBuffer);\n> > +     availableCaptureParamBuffers_.push(info->captureParamBuffer);\n> >\n> >       /* Delete the extended frame information. */\n> >       frameInfo_.erase(info->id);\n> > @@ -102,6 +118,9 @@ bool IPU3Frames::tryComplete(IPU3Frames::Info *info)\n> >       if (!info->paramDequeued)\n> >               return false;\n> >\n> > +     if (!info->captureParamDequeued)\n> > +             return false;\n> > +\n> >       remove(info);\n> >\n> >       bufferAvailable.emit();\n> > @@ -131,7 +150,7 @@ IPU3Frames::Info *IPU3Frames::find(FrameBuffer\n> *buffer)\n> >                               return info;\n> >\n> >               if (info->rawBuffer == buffer || info->paramBuffer ==\n> buffer ||\n> > -                 info->statBuffer == buffer)\n> > +                 info->statBuffer == buffer || info->captureParamBuffer\n> == buffer)\n> >                       return info;\n> >       }\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/frames.h\n> b/src/libcamera/pipeline/ipu3/frames.h\n> > index 6e3cb915..8fcb8a14 100644\n> > --- a/src/libcamera/pipeline/ipu3/frames.h\n> > +++ b/src/libcamera/pipeline/ipu3/frames.h\n> > @@ -36,16 +36,20 @@ public:\n> >               FrameBuffer *paramBuffer;\n> >               FrameBuffer *statBuffer;\n> >\n> > +             FrameBuffer *captureParamBuffer;\n> > +\n> >               ControlList effectiveSensorControls;\n> >\n> >               bool paramDequeued;\n> > +             bool captureParamDequeued;\n> >               bool metadataProcessed;\n> >       };\n> >\n> >       IPU3Frames();\n> >\n> >       void init(const std::vector<std::unique_ptr<FrameBuffer>>\n> &paramBuffers,\n> > -               const std::vector<std::unique_ptr<FrameBuffer>>\n> &statBuffers);\n> > +               const std::vector<std::unique_ptr<FrameBuffer>>\n> &statBuffers,\n> > +               const std::vector<std::unique_ptr<FrameBuffer>>\n> &captureParamBuffers);\n> >       void clear();\n> >\n> >       Info *create(Request *request);\n> > @@ -61,6 +65,8 @@ private:\n> >       std::queue<FrameBuffer *> availableParamBuffers_;\n> >       std::queue<FrameBuffer *> availableStatBuffers_;\n> >\n> > +     std::queue<FrameBuffer *> availableCaptureParamBuffers_;\n> > +\n> >       std::map<unsigned int, std::unique_ptr<Info>> frameInfo_;\n> >   };\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index e219f704..a201c5ca 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -70,6 +70,7 @@ public:\n> >       Stream outStream_;\n> >       Stream vfStream_;\n> >       Stream rawStream_;\n> > +     Stream outCaptureStream_;\n>\n>\n> If there is a outCaptureStream_ now, should we be dropping outStream_\n> and associated buffer allocations for outStream_ ?\n>\n> My guess is yes, let's see what happens in subsequent patches.\n>\n> For this one,\n>\n>\nSo |outStream_| is still in use with |outCaptureStream_| available, as it's\nstill the main output stream of ImgU0 (ipu3.cpp: line 380).\nUnless you're suggesting that when a JPEG stream is requested, there's at\nmost one video stream requested (which is likely used as the Preview), and\nit should be supported by ImgU0's viewfinder node.\nHowever, The Android LIMIT-level guaranteed configuration already has a\nuse-case that supports two YUV/video streams and a JPEG stream [1], not to\nmention the FULL-level.\n\n[1]:\nhttps://developer.android.com/reference/android/hardware/camera2/CameraDevice#regular-capture\n\n\n> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n>\n> >\n> >       Rectangle cropRegion_;\n> >       bool supportsFlips_;\n> > @@ -696,6 +697,8 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera\n> *camera, Stream *stream,\n> >               return imgu0_.viewfinder_->exportBuffers(count, buffers);\n> >       else if (stream == &data->rawStream_)\n> >               return data->cio2_.exportBuffers(count, buffers);\n> > +     else if (stream == &data->outCaptureStream_)\n> > +             return imgu1_.output_->exportBuffers(count, buffers);\n> >\n> >       return -EINVAL;\n> >   }\n> > @@ -718,11 +721,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera\n> *camera)\n> >               data->outStream_.configuration().bufferCount,\n> >               data->vfStream_.configuration().bufferCount,\n> >               data->rawStream_.configuration().bufferCount,\n> > +             data->outCaptureStream_.configuration().bufferCount,\n> >       });\n> >\n> >       ret = imgu0_.allocateBuffers(bufferCount);\n> >       if (ret < 0)\n> >               return ret;\n> > +     ret = imgu1_.allocateBuffers(bufferCount);\n> > +     if (ret < 0) {\n> > +             imgu0_.freeBuffers();\n> > +             return ret;\n> > +     }\n> >\n> >       /* Map buffers to the IPA. */\n> >       unsigned int ipaBufferId = 1;\n> > @@ -737,9 +746,14 @@ int PipelineHandlerIPU3::allocateBuffers(Camera\n> *camera)\n> >               ipaBuffers_.emplace_back(buffer->cookie(),\n> buffer->planes());\n> >       }\n> >\n> > +     for (const std::unique_ptr<FrameBuffer> &buffer :\n> imgu1_.paramBuffers_) {\n> > +             buffer->setCookie(ipaBufferId++);\n> > +             ipaBuffers_.emplace_back(buffer->cookie(),\n> buffer->planes());\n> > +     }\n> > +\n> >       data->ipa_->mapBuffers(ipaBuffers_);\n> >\n> > -     data->frameInfos_.init(imgu0_.paramBuffers_, imgu0_.statBuffers_);\n> > +     data->frameInfos_.init(imgu0_.paramBuffers_, imgu0_.statBuffers_,\n> imgu1_.paramBuffers_);\n> >       data->frameInfos_.bufferAvailable.connect(\n> >               data, &IPU3CameraData::queuePendingRequests);\n> >\n> > @@ -760,6 +774,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n> >       ipaBuffers_.clear();\n> >\n> >       imgu0_.freeBuffers();\n> > +     imgu1_.freeBuffers();\n> >\n> >       return 0;\n> >   }\n> > @@ -1143,6 +1158,7 @@ int PipelineHandlerIPU3::registerCameras()\n> >                       &data->outStream_,\n> >                       &data->vfStream_,\n> >                       &data->rawStream_,\n> > +                     &data->outCaptureStream_,\n> >               };\n> >               CIO2Device *cio2 = &data->cio2_;\n> >\n> > @@ -1318,6 +1334,8 @@ void IPU3CameraData::paramsBufferReady(unsigned\n> int id)\n> >       imgu0_->param_->queueBuffer(info->paramBuffer);\n> >       imgu0_->stat_->queueBuffer(info->statBuffer);\n> >       imgu0_->input_->queueBuffer(info->rawBuffer);\n> > +\n> > +     info->captureParamDequeued = true;\n> >   }\n> >\n> >   void IPU3CameraData::metadataReady(unsigned int id, const ControlList\n> &metadata)\n>","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 413D5BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  2 Aug 2022 10:31:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0CA7E63312;\n\tTue,  2 Aug 2022 12:31:00 +0200 (CEST)","from mail-lf1-x12f.google.com (mail-lf1-x12f.google.com\n\t[IPv6:2a00:1450:4864:20::12f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B15816330E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Aug 2022 12:30:58 +0200 (CEST)","by mail-lf1-x12f.google.com with SMTP id x39so11723178lfu.7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 02 Aug 2022 03:30:58 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659436260;\n\tbh=w0AvwspfyvmiSY3EO24LKbm1HVbJ52m/70BxhrSZu0w=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=1S9cMQ7iBw9KD3Yr74CKgpkdvQNoiONsG3CAZh/Qc0y9KW3I1vv6l893nTvn9Os2B\n\ti/q/3u1a+L6+gZ/Mdx3GHc+jYi78mZy5aJrXuPhZjFlv6xp5ChGODmtXr1ljn3q0sE\n\tDsgmHg+tWHgvfV7XcI6GDINhtE9Pac9s14tkvQJ3xUWJRQPtVLzSf5/VNaaBkQA4O/\n\t4eMrlJ8W6cMWqybVLiL4q0Pf1O+0OP1gi65TTRFNqKP0Qmgy1pZdWGWEc0ZfzuAMfA\n\teToOjlt9hUaVCeVdQqFWoDEjQvTNVjkKV7rLEWEbmhym/P53buqCAEgRe2BhyfUJBM\n\tK9gdhd3oJHzEw==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=fU0kWlpL9TkqRsK6tsQCxn9H4ftAC8aIk6AJChkFMag=;\n\tb=hFKha3DH4hdF7EDU7LiP9Wck0G6bzcgDR5nNSBr+E8Dd7iI4yw2MAFERX7UuSVvFA5\n\tFO7R9u1yaNWyRZ/Q7R/IK0NoZpv9zbTeHOrcuktNRTgTzaJH+21A6Rgq7tjtsxYNgygy\n\tLM2eASpJ/qvn/Z+U92oT/NJZ55iFH6qFoirmw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"hFKha3DH\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=fU0kWlpL9TkqRsK6tsQCxn9H4ftAC8aIk6AJChkFMag=;\n\tb=LDn9iBmqw91icLdGH69SO024m4k6NHwwCBCidcd6+2eDdsA6gc3u+r0leVj4at5CRG\n\tMBAwmmlY2WUUKY55v8AAGXVwtFhUN7X3J+PpngXd6eqOCTwV7pUWPmht+BlyJVtauqkx\n\tAlbSkmzBgrV8vFRfDu4lyCN0mcyovtFTcNzC8VfMbsbKZ6j/7yAGIRnUNyjQ5+mi8Nc9\n\tj/e2IfsXvX7c3E1w0IrM57sgjJrLGiFWFhMb/NXeOLOF+NAI60tt/GFJcXZWAPFdUJ0P\n\tmwu5Hw1Yh7cpJ6/rOR1BvhEbVohWnBSykCChSQfsvmTFN3g/obQBfObxATAjVrvHOPy2\n\tJbUg==","X-Gm-Message-State":"ACgBeo1bF8Mb2ghNqpebmZaWSqvgOQnZKYPb+hOylicFPgj+5aSG6pZC\n\tqyzjW+j1jRhawNIrX0fPvw/MC6C2iBq6anCXpUY/OY6wOJY=","X-Google-Smtp-Source":"AA6agR6OwhQWwObwVlpvbiV33naBAvhxFDvpYuwypgSa7VE+jPIDLQUqasDBxj1HcuIALw8SdQl8FmKTRSaffaRTBQA=","X-Received":"by 2002:a05:6512:230f:b0:48a:f327:7042 with SMTP id\n\to15-20020a056512230f00b0048af3277042mr4497905lfu.75.1659436257974;\n\tTue, 02 Aug 2022 03:30:57 -0700 (PDT)","MIME-Version":"1.0","References":"<20220629103018.4025635-1-chenghaoyang@google.com>\n\t<20220629103018.4025635-5-chenghaoyang@google.com>\n\t<5aa5111e-68d7-6812-8419-d70717db1743@ideasonboard.com>","In-Reply-To":"<5aa5111e-68d7-6812-8419-d70717db1743@ideasonboard.com>","Date":"Tue, 2 Aug 2022 18:30:47 +0800","Message-ID":"<CAEB1ahtwOh5NPJLHeXyFxvVE=Xushe6TAuhAGMsMv+ehDSOmWA@mail.gmail.com>","To":"Umang Jain <umang.jain@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"000000000000af3bc005e53f9cee\"","Subject":"Re: [libcamera-devel] [PATCH v3 4/9] ipu3: Add StillCapture stream\n\tand imgu1 param buffers","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>","From":"Cheng-Hao Yang via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Cheng-Hao Yang <chenghaoyang@chromium.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]