[{"id":27766,"web_url":"https://patchwork.libcamera.org/comment/27766/","msgid":"","date":"2023-09-14T07:47:21","subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Harvey\n\nOn Wed, Sep 13, 2023 at 03:20:50PM +0000, Harvey Yang via libcamera-devel wrote:\n> From: Harvey Yang \n>\n> In CameraDevice::processCaptureRequest, we might need to add an internal\n> buffer for Mapped streams. This patch fixes a case that more than one\n> Mapped streams depend on a stream that is not requested in one capture\n> request.\n\nAh! you're right! I wonder how it went unoticed... maybe we never had\nto create two Mapped streams from a single buffer ? CTS has been run\nmultiple times but we never hit this\n\n>\n> Change-Id: I37a1bcc9c4c2db666a90d74c39883ff18ed11bd5\n\nThis shouldn't be here. We can remove it if you don't have to re-send\n\n> Signed-off-by: Harvey Yang \n> ---\n> src/android/camera_device.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 1f7ce440..25cedd44 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1077,7 +1077,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \t\tdescriptor->request_->addBuffer(sourceStream->stream(),\n> \t\t\t\t\t\tframeBuffer, nullptr);\n>\n> -\t\trequestedStreams.erase(sourceStream);\n> +\t\trequestedStreams.insert(sourceStream);\n\nSo, assuming two Mapped streams that map on the same cameraStream.\n\nThe first processed one won't find a sourceStream in requestedStream\n\n\tif (requestedStreams.find(sourceStream) != requestedStreams.end())\n\t\tcontinue;\n\nso we don't continue and we add create an internal buffer for it and\nadd the framebuffer for the sourceStream to the requet\n\n FrameBuffer *frameBuffer = cameraStream->getBuffer();\n buffer.internalBuffer = frameBuffer;\n\n\tdescriptor->request_->addBuffer(sourceStream->stream(),\n\t\t\t\t\tframeBuffer, nullptr);\n\nAnd this clearly was a nop because of the above if () statement\n\n\trequestedStreams.erase(sourceStream);\n\nHowever, since the second one is a mapped stream too, don't we need to allocate\nan internal buffer for it ?\n\n\t\tFrameBuffer *frameBuffer = cameraStream->getBuffer();\n\t\tbuffer.internalBuffer = frameBuffer;\n\nWith your patch applied I presume the second mapped stream will hit\n\n\tif (requestedStreams.find(sourceStream) != requestedStreams.end())\n\t\tcontinue;\n\nand continue, so no buffer will be allocated for it ?\n\nHave you got a test case for this to try ?\n\nThanks\n j\n\n> \t}\n>\n> \t/*\n> --\n> 2.42.0.283.g2d96d420d3-goog\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 D3ED1BD160\n\tfor ;\n\tThu, 14 Sep 2023 07:47:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1C4EF628FB;\n\tThu, 14 Sep 2023 09:47:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BC90B628DA\n\tfor ;\n\tThu, 14 Sep 2023 09:47:28 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1CE6ADE5;\n\tThu, 14 Sep 2023 09:45:55 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694677650;\n\tbh=d9dL3UhXBeMmDQceTxO5pTEuZyfn2e5IuZOBMUKgZV0=;\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:Cc:\n\tFrom;\n\tb=uwfzPYgeN9tPIfebsEpkPtttQC2ojEShmQP6TGjnGlpAJwmOhxrH9j01xQ5KEWNpQ\n\tt3EPVXXFsV2u5mrbzHQyy5QOfoJ+xhf5M/Wj0fDpgKKxZTuGsGiLQpKT/QirsVLzmY\n\t+1GQVUCBztthrg+JWRIALYp+6jetULH4ggwbu7aixTzE8bmvmxTLfrRozvflJcg6Vk\n\tMIIdr+43WYq24/OkYI+7o4McYP+u9870iRIjaXn8BcTa+NKDKgVTWJ/gBRa31h8zFG\n\tcUe/X5+P/ygCmDo905UTHfPALp8YjXs4acPPJKC1X9Tr1bWlKqPeTUdoIsE7VKY70y\n\tQXsLsUbikwxkg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1694677556;\n\tbh=d9dL3UhXBeMmDQceTxO5pTEuZyfn2e5IuZOBMUKgZV0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TvQxiTBY7Y6EO0BKZ2Mt9leyhejxD0QcFDf1LwCEpYB/gbdbvujXCqJ4+siZHgGue\n\tbNeVgQVQXvNcDOnL53J1NizGEnWtc165oLXTjahWIByszOd8jNAk52L0hq/gQAM0/J\n\tmyPsItws8ZsHA/NkIIO+lmOGYFxwhJHwzTuiMLLU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"TvQxiTBY\"; dkim-atps=neutral","Date":"Thu, 14 Sep 2023 09:47:21 +0200","To":"Harvey Yang ","Message-ID":"","References":"<20230913152146.636483-1-chenghaoyang@google.com>\n\t<20230913152146.636483-2-chenghaoyang@google.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230913152146.636483-2-chenghaoyang@google.com>","Subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27806,"web_url":"https://patchwork.libcamera.org/comment/27806/","msgid":"","date":"2023-09-18T04:18:34","subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Jacopo,\n\nOn Thu, Sep 14, 2023 at 3:47 PM Jacopo Mondi \nwrote:\n\n> Hi Harvey\n>\n> On Wed, Sep 13, 2023 at 03:20:50PM +0000, Harvey Yang via libcamera-devel\n> wrote:\n> > From: Harvey Yang \n> >\n> > In CameraDevice::processCaptureRequest, we might need to add an internal\n> > buffer for Mapped streams. This patch fixes a case that more than one\n> > Mapped streams depend on a stream that is not requested in one capture\n> > request.\n>\n> Ah! you're right! I wonder how it went unoticed... maybe we never had\n> to create two Mapped streams from a single buffer ? CTS has been run\n> multiple times but we never hit this\n>\n> >\n> > Change-Id: I37a1bcc9c4c2db666a90d74c39883ff18ed11bd5\n>\n> This shouldn't be here. We can remove it if you don't have to re-send\n>\n> > Signed-off-by: Harvey Yang \n> > ---\n> > src/android/camera_device.cpp | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/src/android/camera_device.cpp\n> b/src/android/camera_device.cpp\n> > index 1f7ce440..25cedd44 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1077,7 +1077,7 @@ int\n> CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> > descriptor->request_->addBuffer(sourceStream->stream(),\n> > frameBuffer, nullptr);\n> >\n> > - requestedStreams.erase(sourceStream);\n> > + requestedStreams.insert(sourceStream);\n>\n> So, assuming two Mapped streams that map on the same cameraStream.\n>\n> The first processed one won't find a sourceStream in requestedStream\n>\n> if (requestedStreams.find(sourceStream) != requestedStreams.end())\n> continue;\n>\n> so we don't continue and we add create an internal buffer for it and\n> add the framebuffer for the sourceStream to the requet\n>\n> FrameBuffer *frameBuffer = cameraStream->getBuffer();\n> buffer.internalBuffer = frameBuffer;\n>\n> descriptor->request_->addBuffer(sourceStream->stream(),\n> frameBuffer, nullptr);\n>\n> And this clearly was a nop because of the above if () statement\n>\n> requestedStreams.erase(sourceStream);\n>\n> However, since the second one is a mapped stream too, don't we need to\n> allocate\n> an internal buffer for it ?\n>\n> FrameBuffer *frameBuffer = cameraStream->getBuffer();\n> buffer.internalBuffer = frameBuffer;\n>\n>\nNot really. The second stream can use the same internal buffer allocated by\nthe first stream\nto continue the processing. If we allocate a new internal buffer, this will\nfail anyway:\n\n descriptor->request_->addBuffer(sourceStream->stream(),\n\t\t\t\t frameBuffer, nullptr);\n\n, as libcamera::Request already adds the internal buffer to the\nlibcamera::Stream.\n\nThe only purpose of setting the internal buffer is to return the allocated\nbuffer\nto the CameraStream which created the buffer. See:\n\nhttps://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1246\nhttps://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1370\n\nThere are no other usages of\n`Camera3RequesetDescriptor::StreamBuffer::internalBuffer`.\n\ndescriptor->pendingStreamsToProcess_.insert({ cameraStream, &buffer });\n\nThis will ensure the mapped stream to be processed in:\n\nhttps://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1236\n\n\n\n> With your patch applied I presume the second mapped stream will hit\n>\n> if (requestedStreams.find(sourceStream) != requestedStreams.end())\n> continue;\n>\n> and continue, so no buffer will be allocated for it ?\n>\n> Have you got a test case for this to try ?\n>\n>\nSorry, I've checked with Han-lin, and we don't have such a test with CrOS\nor CTS.\n\n\n> Thanks\n> j\n>\n> > }\n> >\n> > /*\n> > --\n> > 2.42.0.283.g2d96d420d3-goog\n> >\n>\n\nThanks for the review!\n\nHarvey","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 E679DBE080\n\tfor ;\n\tMon, 18 Sep 2023 04:18:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EBE9B6293A;\n\tMon, 18 Sep 2023 06:18:49 +0200 (CEST)","from mail-lj1-x236.google.com (mail-lj1-x236.google.com\n\t[IPv6:2a00:1450:4864:20::236])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7D868628E9\n\tfor ;\n\tMon, 18 Sep 2023 06:18:46 +0200 (CEST)","by mail-lj1-x236.google.com with SMTP id\n\t38308e7fff4ca-2c008d8fd07so10548411fa.1\n\tfor ;\n\tSun, 17 Sep 2023 21:18:46 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695010730;\n\tbh=PjaqSiggJROZxlQy4fZeLIVYflUuUxp1AlPYojSvgT0=;\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=DcZ2M5hks18EMoCtW1PgurQTg/l1lPnS9+akDwKASPfYckgOhzbgWhKBFZ3EzGBpe\n\tYmju50sF/h8c2PuUSnvqaSBHa5fQYzfqrEXDCygFq16un7k+o+1piilIXK4UTAmG6Z\n\ts2k13OxKvX9nAQIDgk+G67WbiszJK15RNvHC5zfKgEllkMiS6Z9U+QGUEzyISDh2D8\n\t1PdEntYytQM0Rw1pSkZYSsDkkRuDgbuPdYSNEbswdIgPCEXeO6oOHv0tOiPIa3xnhJ\n\tqTxdfXTxHR+v2ipGU13Gc5QfRbavgdR5khe0yefYP1teUiao8K3BlbdrNWXMTSg0Rj\n\tS4ZjVVvKOjC+w==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1695010725; x=1695615525;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=va7xIJN1px92/RIGDzymJV8UZx6jiS6uZyCpj92DzTM=;\n\tb=BS/+f5q5bgkdGC115/7TV0tkzKFlDqLcFtcFx7enBxR8lnEdx/KlqF8Zylza6q5+xc\n\t+tmRWfZUa00UvntKQl8CjIsUzFB0EPVC6TnqTvoWG5R9710Jm0feHV0fRf1GE4+XGsv4\n\tK6ojB/3iqI/RCZ1qyHyxtL8j/4tkg04P7ieMU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"BS/+f5q5\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1695010725; x=1695615525;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=va7xIJN1px92/RIGDzymJV8UZx6jiS6uZyCpj92DzTM=;\n\tb=hSD7+/movB8L41kUmDM+UHllvEH0oAzh8ePJ7OcbOZG3LNNZfjqoUHW6XUgjCzspeF\n\tEL/N4AXoLgJnwCpXxwc3+4VctCk/nRsJzyK2/1J4LCDXMzstUtJZdwP/vTaB24YMZkvV\n\t88j+gA4eOVgowoXxuPg1c/QSEDL0IBgdBz5RioCh2R3dKEVFi6t3Zey3kgYB6O+A8eMY\n\t5NxS56FLhbRUU0tcOkEjgxPMDD855eWfp61Ak1tEgcRpo0i/w1qk2/3dArEGRjum6ymY\n\tRiLYKLZY1xQljfjAajTblK2XkQ4booILmbC1CgcBfo5dPvAqP5sC3felw426xAXkGZ6r\n\tTJCA==","X-Gm-Message-State":"AOJu0Yx5sTmnt1rfH8Lnf5aTxD5zM41sllqxG62qZ0rwBGLPzVOY5GQV\n\tJ6/NM/pjzd+0WLLjt1lnlK7wR0yC+vMb1Fy1o26/Nf/QJxfUhmaaxP8=","X-Google-Smtp-Source":"AGHT+IHpbS0tWc98TnrPPvrR/zDukL28LH+ckRQ0Id/gtQgE7CjCAnlTKL0/mO+G5hgme43c8ckIfUjbISuPAIvgfFg=","X-Received":"by 2002:a05:651c:610:b0:2c0:18e0:708a with SMTP id\n\tk16-20020a05651c061000b002c018e0708amr194797lje.46.1695010725147;\n\tSun, 17 Sep 2023 21:18:45 -0700 (PDT)","MIME-Version":"1.0","References":"<20230913152146.636483-1-chenghaoyang@google.com>\n\t<20230913152146.636483-2-chenghaoyang@google.com>\n\t","In-Reply-To":"","Date":"Mon, 18 Sep 2023 12:18:34 +0800","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"00000000000029d34e06059a7068\"","Subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","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","From":"Cheng-Hao Yang via libcamera-devel\n\t","Reply-To":"Cheng-Hao Yang ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27816,"web_url":"https://patchwork.libcamera.org/comment/27816/","msgid":"","date":"2023-09-19T07:52:13","subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Harvey\n\nOn Mon, Sep 18, 2023 at 12:18:34PM +0800, Cheng-Hao Yang via libcamera-devel wrote:\n> Hi Jacopo,\n>\n> On Thu, Sep 14, 2023 at 3:47 PM Jacopo Mondi \n> wrote:\n>\n> > Hi Harvey\n> >\n> > On Wed, Sep 13, 2023 at 03:20:50PM +0000, Harvey Yang via libcamera-devel\n> > wrote:\n> > > From: Harvey Yang \n> > >\n> > > In CameraDevice::processCaptureRequest, we might need to add an internal\n> > > buffer for Mapped streams. This patch fixes a case that more than one\n> > > Mapped streams depend on a stream that is not requested in one capture\n> > > request.\n> >\n> > Ah! you're right! I wonder how it went unoticed... maybe we never had\n> > to create two Mapped streams from a single buffer ? CTS has been run\n> > multiple times but we never hit this\n> >\n> > >\n> > > Change-Id: I37a1bcc9c4c2db666a90d74c39883ff18ed11bd5\n> >\n> > This shouldn't be here. We can remove it if you don't have to re-send\n> >\n> > > Signed-off-by: Harvey Yang \n> > > ---\n> > > src/android/camera_device.cpp | 2 +-\n> > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/android/camera_device.cpp\n> > b/src/android/camera_device.cpp\n> > > index 1f7ce440..25cedd44 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -1077,7 +1077,7 @@ int\n> > CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> > > descriptor->request_->addBuffer(sourceStream->stream(),\n> > > frameBuffer, nullptr);\n> > >\n> > > - requestedStreams.erase(sourceStream);\n> > > + requestedStreams.insert(sourceStream);\n> >\n> > So, assuming two Mapped streams that map on the same cameraStream.\n> >\n> > The first processed one won't find a sourceStream in requestedStream\n> >\n> > if (requestedStreams.find(sourceStream) != requestedStreams.end())\n> > continue;\n> >\n> > so we don't continue and we add create an internal buffer for it and\n> > add the framebuffer for the sourceStream to the requet\n> >\n> > FrameBuffer *frameBuffer = cameraStream->getBuffer();\n> > buffer.internalBuffer = frameBuffer;\n> >\n> > descriptor->request_->addBuffer(sourceStream->stream(),\n> > frameBuffer, nullptr);\n> >\n> > And this clearly was a nop because of the above if () statement\n> >\n> > requestedStreams.erase(sourceStream);\n> >\n> > However, since the second one is a mapped stream too, don't we need to\n> > allocate\n> > an internal buffer for it ?\n> >\n> > FrameBuffer *frameBuffer = cameraStream->getBuffer();\n> > buffer.internalBuffer = frameBuffer;\n> >\n> >\n> Not really. The second stream can use the same internal buffer allocated by\n> the first stream\n> to continue the processing. If we allocate a new internal buffer, this will\n> fail anyway:\n>\n> descriptor->request_->addBuffer(sourceStream->stream(),\n> \t\t\t\t frameBuffer, nullptr);\n>\n> , as libcamera::Request already adds the internal buffer to the\n> libcamera::Stream.\n\nInded, you're very right! I clearly was confused as I thought an\n\"internal buffer\" had to be allocated, but as we're here handling\nmapped streams the destination buffer is provided by the framework.\n\nSorry for the noise.\n\n>\n> The only purpose of setting the internal buffer is to return the allocated\n> buffer\n> to the CameraStream which created the buffer. See:\n>\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1246\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1370\n>\n> There are no other usages of\n> `Camera3RequesetDescriptor::StreamBuffer::internalBuffer`.\n\nack\n\n>\n> descriptor->pendingStreamsToProcess_.insert({ cameraStream, &buffer });\n>\n> This will ensure the mapped stream to be processed in:\n>\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1236\n>\n\nI re-read the code, but I can't figure out if there are issue in\nprocessing 2 mapped streams created from the same internal buffer, as\nI fear that code path has never really been tested ?\n\n>\n>\n> > With your patch applied I presume the second mapped stream will hit\n> >\n> > if (requestedStreams.find(sourceStream) != requestedStreams.end())\n> > continue;\n> >\n> > and continue, so no buffer will be allocated for it ?\n> >\n> > Have you got a test case for this to try ?\n> >\n> >\n> Sorry, I've checked with Han-lin, and we don't have such a test with CrOS\n> or CTS.\n>\n\nExactly, would be nice to test, but in the meantime, your patch seems\nto be fixing a bug indeed.\n\nThanks!\n\nReviewed-by: Jacopo Mondi \n\n>\n> > Thanks\n> > j\n> >\n> > > }\n> > >\n> > > /*\n> > > --\n> > > 2.42.0.283.g2d96d420d3-goog\n> > >\n> >\n>\n> Thanks for the review!\n>\n> Harvey","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 A792FC326B\n\tfor ;\n\tTue, 19 Sep 2023 07:52:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EA40D6293F;\n\tTue, 19 Sep 2023 09:52:17 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 86A78628D8\n\tfor ;\n\tTue, 19 Sep 2023 09:52:16 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 63AD42CF;\n\tTue, 19 Sep 2023 09:50:40 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695109938;\n\tbh=R4utHlDcqDHr+b1AGmomUdVPmX46uL0SMfW6UA8G3a4=;\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:Cc:\n\tFrom;\n\tb=I2ATrMnQ7LaR0t8zstZLfGXj2Bxhk73NDYtEXv7Qizblly3GA6zTwSaVrGZu7acGb\n\txy97orI+y7XnT7Oxdu7Kq4GN1BdWRgN37qM2ZmWvc5e3h3+6wmp6WUN2NJRBSvsYoI\n\t85ic30Sym3RtT9wMALl3YHcLvLtAyLuGrDfSoHv6HxfFZoWBZ1aP7cqmdKTXtGQ5tM\n\tQXubU1WgShiHSO+J26uw0qzXaVwKpi/JLhQ8EbXhTTv1pDCVeflDExPxqGrILLg8tT\n\tPMIG8G0bJkfyGYGHfrN23lPvbcXzVAaGLPI7DsMdHf7Uzw09F2zTHBheDenoDIhGhM\n\tGOSrPW8fPxOUQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1695109840;\n\tbh=R4utHlDcqDHr+b1AGmomUdVPmX46uL0SMfW6UA8G3a4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vrw0oeeuy4UqPnUzWQOZPS2cuIeh4JFswGbqsAX5pRP8mFM+FI3Md66URK/prTcIj\n\tp3L8CqyshoBdbq7W+A1qaxZGATNVcQ8k1oOoF9Sutt6tuBBF9wg6ZlwvCreRcb7lj2\n\t/6jpSQXe9GF4uhlqBexMJR0CwGVexQ1XbPYrFqco="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"vrw0oeeu\"; dkim-atps=neutral","Date":"Tue, 19 Sep 2023 09:52:13 +0200","To":"Cheng-Hao Yang ","Message-ID":"","References":"<20230913152146.636483-1-chenghaoyang@google.com>\n\t<20230913152146.636483-2-chenghaoyang@google.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27820,"web_url":"https://patchwork.libcamera.org/comment/27820/","msgid":"","date":"2023-09-20T07:31:21","subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Jacopo,\n\nOn Tue, Sep 19, 2023 at 3:52 PM Jacopo Mondi \nwrote:\n\n> Hi Harvey\n>\n> On Mon, Sep 18, 2023 at 12:18:34PM +0800, Cheng-Hao Yang via\n> libcamera-devel wrote:\n> > Hi Jacopo,\n> >\n> > On Thu, Sep 14, 2023 at 3:47 PM Jacopo Mondi <\n> jacopo.mondi@ideasonboard.com>\n> > wrote:\n> >\n> > > Hi Harvey\n> > >\n> > > On Wed, Sep 13, 2023 at 03:20:50PM +0000, Harvey Yang via\n> libcamera-devel\n> > > wrote:\n> > > > From: Harvey Yang \n> > > >\n> > > > In CameraDevice::processCaptureRequest, we might need to add an\n> internal\n> > > > buffer for Mapped streams. This patch fixes a case that more than one\n> > > > Mapped streams depend on a stream that is not requested in one\n> capture\n> > > > request.\n> > >\n> > > Ah! you're right! I wonder how it went unoticed... maybe we never had\n> > > to create two Mapped streams from a single buffer ? CTS has been run\n> > > multiple times but we never hit this\n> > >\n> > > >\n> > > > Change-Id: I37a1bcc9c4c2db666a90d74c39883ff18ed11bd5\n> > >\n> > > This shouldn't be here. We can remove it if you don't have to re-send\n> > >\n> > > > Signed-off-by: Harvey Yang \n> > > > ---\n> > > > src/android/camera_device.cpp | 2 +-\n> > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/src/android/camera_device.cpp\n> > > b/src/android/camera_device.cpp\n> > > > index 1f7ce440..25cedd44 100644\n> > > > --- a/src/android/camera_device.cpp\n> > > > +++ b/src/android/camera_device.cpp\n> > > > @@ -1077,7 +1077,7 @@ int\n> > > CameraDevice::processCaptureRequest(camera3_capture_request_t\n> *camera3Reques\n> > > > descriptor->request_->addBuffer(sourceStream->stream(),\n> > > > frameBuffer, nullptr);\n> > > >\n> > > > - requestedStreams.erase(sourceStream);\n> > > > + requestedStreams.insert(sourceStream);\n> > >\n> > > So, assuming two Mapped streams that map on the same cameraStream.\n> > >\n> > > The first processed one won't find a sourceStream in requestedStream\n> > >\n> > > if (requestedStreams.find(sourceStream) !=\n> requestedStreams.end())\n> > > continue;\n> > >\n> > > so we don't continue and we add create an internal buffer for it and\n> > > add the framebuffer for the sourceStream to the requet\n> > >\n> > > FrameBuffer *frameBuffer = cameraStream->getBuffer();\n> > > buffer.internalBuffer = frameBuffer;\n> > >\n> > > descriptor->request_->addBuffer(sourceStream->stream(),\n> > > frameBuffer, nullptr);\n> > >\n> > > And this clearly was a nop because of the above if () statement\n> > >\n> > > requestedStreams.erase(sourceStream);\n> > >\n> > > However, since the second one is a mapped stream too, don't we need to\n> > > allocate\n> > > an internal buffer for it ?\n> > >\n> > > FrameBuffer *frameBuffer = cameraStream->getBuffer();\n> > > buffer.internalBuffer = frameBuffer;\n> > >\n> > >\n> > Not really. The second stream can use the same internal buffer allocated\n> by\n> > the first stream\n> > to continue the processing. If we allocate a new internal buffer, this\n> will\n> > fail anyway:\n> >\n> > descriptor->request_->addBuffer(sourceStream->stream(),\n> > frameBuffer, nullptr);\n> >\n> > , as libcamera::Request already adds the internal buffer to the\n> > libcamera::Stream.\n>\n> Inded, you're very right! I clearly was confused as I thought an\n> \"internal buffer\" had to be allocated, but as we're here handling\n> mapped streams the destination buffer is provided by the framework.\n>\n> Sorry for the noise.\n>\n> >\n> > The only purpose of setting the internal buffer is to return the\n> allocated\n> > buffer\n> > to the CameraStream which created the buffer. See:\n> >\n> >\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1246\n> >\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1370\n> >\n> > There are no other usages of\n> > `Camera3RequesetDescriptor::StreamBuffer::internalBuffer`.\n>\n> ack\n>\n> >\n> > descriptor->pendingStreamsToProcess_.insert({ cameraStream, &buffer });\n> >\n> > This will ensure the mapped stream to be processed in:\n> >\n> >\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1236\n> >\n>\n> I re-read the code, but I can't figure out if there are issue in\n> processing 2 mapped streams created from the same internal buffer, as\n> I fear that code path has never really been tested ?\n>\n>\nYeah I understand. Logically, libcamera::Request doesn't care how the\nbuffer is created, while true that we don't test it at all yet.\n\n\n> >\n> >\n> > > With your patch applied I presume the second mapped stream will hit\n> > >\n> > > if (requestedStreams.find(sourceStream) !=\n> requestedStreams.end())\n> > > continue;\n> > >\n> > > and continue, so no buffer will be allocated for it ?\n> > >\n> > > Have you got a test case for this to try ?\n> > >\n> > >\n> > Sorry, I've checked with Han-lin, and we don't have such a test with CrOS\n> > or CTS.\n> >\n>\n> Exactly, would be nice to test, but in the meantime, your patch seems\n> to be fixing a bug indeed.\n>\n>\nHope it's worth merging now :)\n\n\n> Thanks!\n>\n> Reviewed-by: Jacopo Mondi \n>\n> >\n> > > Thanks\n> > > j\n> > >\n> > > > }\n> > > >\n> > > > /*\n> > > > --\n> > > > 2.42.0.283.g2d96d420d3-goog\n> > > >\n> > >\n> >\n> > Thanks for the review!\n> >\n> > Harvey\n>\n\nBR,\nHarvey","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 739FEBE080\n\tfor ;\n\tWed, 20 Sep 2023 07:31:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5F7256293F;\n\tWed, 20 Sep 2023 09:31:35 +0200 (CEST)","from mail-lj1-x22c.google.com (mail-lj1-x22c.google.com\n\t[IPv6:2a00:1450:4864:20::22c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A48BD62936\n\tfor ;\n\tWed, 20 Sep 2023 09:31:33 +0200 (CEST)","by mail-lj1-x22c.google.com with SMTP id\n\t38308e7fff4ca-2c123eed8b2so2349181fa.0\n\tfor ;\n\tWed, 20 Sep 2023 00:31:33 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695195095;\n\tbh=UGsmCiEt+wSqdOszONkB9Tk8jVE8DTJUqDbBMObWip0=;\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=ev//c1JYZNOh8rZZYY3KQBgNNNWzayy/74FMBwTMf3ooKT72rhmVx9f+laeJgRk9F\n\tBaCMNcLK5U7Kic/nvff3OmcWr9afZVMzefHClpWy6ET96rudGHbWQmXsmSBIM7tgpn\n\thrNEZktVw9wPtguxLVRJWiLHNo5D4vDQZGg8MRp9zAKfDCS1V4FHuE58AOjUQs0BqF\n\t4hbtuZ1X5+5XckgGbr2s+Efi/y/cR2Y2kz8XNnoeNAtK5zcLvNrfBdP3B5kAg/hm8c\n\tKZhpp2tnUT82MePbjgP0uuVXDXDnCdpU72pTgZMtpfMazVlPX+R0LQdaj1W435/Ct1\n\tzSM2hTX8CqXaw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1695195093; x=1695799893;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=ERkfj8cxxPfELCXYkpX45+i+y8eEijGQUuj4vRJZPfc=;\n\tb=TtH1klLyZv6TsawrI/D6qki09YDk6ag55HS5oCyq3bYanBRcjfN4eHVzw5iIjQZANk\n\teD9LRnlyIWppJ42PMf0SEr/9PyzEd59k2lIeqSQJawkPkSmw1zNrCFSyy9GpmhA7tIaW\n\t/jzZCHscuvwB4OGiaO8euXLvjV9gTS+H0fqpY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"TtH1klLy\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1695195093; x=1695799893;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=ERkfj8cxxPfELCXYkpX45+i+y8eEijGQUuj4vRJZPfc=;\n\tb=O8Pwq4666TOrNqqwWfhetPviZfTiN4cVTPn4AZJk+24VG2Fb2Vm6yobcaXdM2EFyCd\n\tt8OJtTyK5Z8OwH5Wv8sfkmILn+H3XNJ3+B75LoiE8fGnr5B3+XCCOPepjBpkIVx5Rq8C\n\tV197XO65iKr1OK4IssQ4IksAzTq4LKqLj9NWGVpe5Shbz/Q7sKuzYpcf10wHlBbEbvqE\n\tUhljdPMJJp1S9zVLC4O5Pm439rr409wuQN44daHjCAZDYyVGx8rgyIEPxtKqtizMLwv7\n\tV2kt0qfxnj0hsYQiA3mZY6lbe7I9R1L8LKejEaYY8NRtDv7sAc7Yk4OD2oBnJ0dGd2Dk\n\tfPZg==","X-Gm-Message-State":"AOJu0Yx5mRHM/oWUX2Jx3WKsds4QM658B00owZyHb9xdLnzFzUFg7oK4\n\tPMbI4ka++QrIKQGwXjSXrMCvGb7IYQFAgKO3xzaCwg==","X-Google-Smtp-Source":"AGHT+IFXk0533Em/kNjBbz9WKi7YPcTk1t4dh7fMOzoNH4EEvrrRWvO1TKLL2XxXuhfew9yPZk8RvI/kYC6sGfHyhzg=","X-Received":"by 2002:a2e:b616:0:b0:2b9:36d5:729c with SMTP id\n\tr22-20020a2eb616000000b002b936d5729cmr1252690ljn.47.1695195092544;\n\tWed, 20 Sep 2023 00:31:32 -0700 (PDT)","MIME-Version":"1.0","References":"<20230913152146.636483-1-chenghaoyang@google.com>\n\t<20230913152146.636483-2-chenghaoyang@google.com>\n\t\n\t\n\t","In-Reply-To":"","Date":"Wed, 20 Sep 2023 15:31:21 +0800","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"0000000000005102a70605c55dfc\"","Subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","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","From":"Cheng-Hao Yang via libcamera-devel\n\t","Reply-To":"Cheng-Hao Yang ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27824,"web_url":"https://patchwork.libcamera.org/comment/27824/","msgid":"<20230921090857.GA26483@pendragon.ideasonboard.com>","date":"2023-09-21T09:08:57","subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nThe subject line should start with \"android: \".\n\nOn Tue, Sep 19, 2023 at 09:52:13AM +0200, Jacopo Mondi via libcamera-devel wrote:\n> On Mon, Sep 18, 2023 at 12:18:34PM +0800, Cheng-Hao Yang via libcamera-devel wrote:\n> > On Thu, Sep 14, 2023 at 3:47 PM Jacopo Mondi wrote:\n> > > On Wed, Sep 13, 2023 at 03:20:50PM +0000, Harvey Yang via libcamera-devel wrote:\n> > > > From: Harvey Yang \n> > > >\n> > > > In CameraDevice::processCaptureRequest, we might need to add an internal\n> > > > buffer for Mapped streams. This patch fixes a case that more than one\n> > > > Mapped streams depend on a stream that is not requested in one capture\n> > > > request.\n> > >\n> > > Ah! you're right! I wonder how it went unoticed... maybe we never had\n> > > to create two Mapped streams from a single buffer ? CTS has been run\n> > > multiple times but we never hit this\n> > >\n> > > > Change-Id: I37a1bcc9c4c2db666a90d74c39883ff18ed11bd5\n> > >\n> > > This shouldn't be here. We can remove it if you don't have to re-send\n\nOn the other hand, a Fixes: line would be nice. I think\n\nFixes: 7ea83eba0df6 (\"android: camera_device: Postpone mapped streams handling\")\n\nis the right one.\n\n> > > > Signed-off-by: Harvey Yang \n> > > > ---\n> > > > src/android/camera_device.cpp | 2 +-\n> > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > index 1f7ce440..25cedd44 100644\n> > > > --- a/src/android/camera_device.cpp\n> > > > +++ b/src/android/camera_device.cpp\n> > > > @@ -1077,7 +1077,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> > > > descriptor->request_->addBuffer(sourceStream->stream(),\n> > > > frameBuffer, nullptr);\n> > > >\n> > > > - requestedStreams.erase(sourceStream);\n> > > > + requestedStreams.insert(sourceStream);\n> > >\n> > > So, assuming two Mapped streams that map on the same cameraStream.\n> > >\n> > > The first processed one won't find a sourceStream in requestedStream\n> > >\n> > > if (requestedStreams.find(sourceStream) != requestedStreams.end())\n> > > continue;\n> > >\n> > > so we don't continue and we add create an internal buffer for it and\n> > > add the framebuffer for the sourceStream to the requet\n> > >\n> > > FrameBuffer *frameBuffer = cameraStream->getBuffer();\n> > > buffer.internalBuffer = frameBuffer;\n> > >\n> > > descriptor->request_->addBuffer(sourceStream->stream(),\n> > > frameBuffer, nullptr);\n> > >\n> > > And this clearly was a nop because of the above if () statement\n> > >\n> > > requestedStreams.erase(sourceStream);\n> > >\n> > > However, since the second one is a mapped stream too, don't we need to\n> > > allocate\n> > > an internal buffer for it ?\n> > >\n> > > FrameBuffer *frameBuffer = cameraStream->getBuffer();\n> > > buffer.internalBuffer = frameBuffer;\n> > >\n> > >\n> > Not really. The second stream can use the same internal buffer allocated by\n> > the first stream\n> > to continue the processing. If we allocate a new internal buffer, this will\n> > fail anyway:\n> >\n> > descriptor->request_->addBuffer(sourceStream->stream(),\n> > \t\t\t\t frameBuffer, nullptr);\n> >\n> > , as libcamera::Request already adds the internal buffer to the\n> > libcamera::Stream.\n\nIt could be nice to capture a bit more context in the commit message, I\nhad to read through the implementation to understand the issue fixed by\nthis patch.\n\nReviewed-by: Laurent Pinchart \n\n> Inded, you're very right! I clearly was confused as I thought an\n> \"internal buffer\" had to be allocated, but as we're here handling\n> mapped streams the destination buffer is provided by the framework.\n> \n> Sorry for the noise.\n> \n> > The only purpose of setting the internal buffer is to return the allocated\n> > buffer\n> > to the CameraStream which created the buffer. See:\n> >\n> > https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1246\n> > https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1370\n> >\n> > There are no other usages of\n> > `Camera3RequesetDescriptor::StreamBuffer::internalBuffer`.\n> \n> ack\n> \n> > descriptor->pendingStreamsToProcess_.insert({ cameraStream, &buffer });\n> >\n> > This will ensure the mapped stream to be processed in:\n> >\n> > https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1236\n> \n> I re-read the code, but I can't figure out if there are issue in\n> processing 2 mapped streams created from the same internal buffer, as\n> I fear that code path has never really been tested ?\n> \n> > > With your patch applied I presume the second mapped stream will hit\n> > >\n> > > if (requestedStreams.find(sourceStream) != requestedStreams.end())\n> > > continue;\n> > >\n> > > and continue, so no buffer will be allocated for it ?\n> > >\n> > > Have you got a test case for this to try ?\n> >\n> > Sorry, I've checked with Han-lin, and we don't have such a test with CrOS\n> > or CTS.\n> \n> Exactly, would be nice to test, but in the meantime, your patch seems\n> to be fixing a bug indeed.\n> \n> Thanks!\n> \n> Reviewed-by: Jacopo Mondi \n> \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 BFAB0BE080\n\tfor ;\n\tThu, 21 Sep 2023 09:08:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0E27262944;\n\tThu, 21 Sep 2023 11:08:47 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C23A861DE7\n\tfor ;\n\tThu, 21 Sep 2023 11:08:45 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 34F131102;\n\tThu, 21 Sep 2023 11:07:08 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695287327;\n\tbh=o5b0DYo6TeVMxK3JqqG7IiYQoMH/nA/nxlB7wGcS9SE=;\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:Cc:\n\tFrom;\n\tb=JyepAJE77liGHIQFwfsiGo5p2P1in90rVwHcJcArAZgVm7JNAuNryuIEnyZgFHwe6\n\tfFdy0cNR3hhFgMtAMMQZ+zhoBL7ZehiAFGxFIaS1D/CufGPOJpRSgFuPhW6oBnSCQz\n\tw4qZ0alE7xRtSv2jftTFKw2k6yKLC8dq/CybNfNZTo3qWNPk7lXbsWsWLkdjS4ofUb\n\tPFGgFq77hMAv3NGfZTviGLEqbUmnmZ9kDH/9MYV0CgsqjbD+FwmKQ2hhmFLPBKImXT\n\tH9uV0vSv6FKNv6hHIowj7n9rBOUg13S9buAXwyn6DLFkn9a2Ixjj2z9K68vzNru+YZ\n\t83brNX+zwurUw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1695287228;\n\tbh=o5b0DYo6TeVMxK3JqqG7IiYQoMH/nA/nxlB7wGcS9SE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Ws0a9mtyXlC7B99++kwMEuTkBm2ut61tpZ+X+aB+eUEp7DS9FmQ1Wy6XyI3LWqiVs\n\tejTixhJwu6II78k70ioISwBsPsjKn5CE+Jxakb+Crxw4/+ZshAcj97TSBmF3KhWLfQ\n\ti0ys+OB6l/mvYfsqZrIjioH/UNRlvt+3I6wIsblU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Ws0a9mty\"; dkim-atps=neutral","Date":"Thu, 21 Sep 2023 12:08:57 +0300","To":"Jacopo Mondi ","Message-ID":"<20230921090857.GA26483@pendragon.ideasonboard.com>","References":"<20230913152146.636483-1-chenghaoyang@google.com>\n\t<20230913152146.636483-2-chenghaoyang@google.com>\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"Cheng-Hao Yang ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27827,"web_url":"https://patchwork.libcamera.org/comment/27827/","msgid":"","date":"2023-09-21T10:50:19","subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Laurent\n\nOn Thu, Sep 21, 2023 at 12:08:57PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Hello,\n>\n> The subject line should start with \"android: \".\n>\n> On Tue, Sep 19, 2023 at 09:52:13AM +0200, Jacopo Mondi via libcamera-devel wrote:\n> > On Mon, Sep 18, 2023 at 12:18:34PM +0800, Cheng-Hao Yang via libcamera-devel wrote:\n> > > On Thu, Sep 14, 2023 at 3:47 PM Jacopo Mondi wrote:\n> > > > On Wed, Sep 13, 2023 at 03:20:50PM +0000, Harvey Yang via libcamera-devel wrote:\n> > > > > From: Harvey Yang \n> > > > >\n> > > > > In CameraDevice::processCaptureRequest, we might need to add an internal\n> > > > > buffer for Mapped streams. This patch fixes a case that more than one\n> > > > > Mapped streams depend on a stream that is not requested in one capture\n> > > > > request.\n> > > >\n> > > > Ah! you're right! I wonder how it went unoticed... maybe we never had\n> > > > to create two Mapped streams from a single buffer ? CTS has been run\n> > > > multiple times but we never hit this\n> > > >\n> > > > > Change-Id: I37a1bcc9c4c2db666a90d74c39883ff18ed11bd5\n> > > >\n> > > > This shouldn't be here. We can remove it if you don't have to re-send\n>\n> On the other hand, a Fixes: line would be nice. I think\n>\n> Fixes: 7ea83eba0df6 (\"android: camera_device: Postpone mapped streams handling\")\n>\n> is the right one.\n>\n> > > > > Signed-off-by: Harvey Yang \n> > > > > ---\n> > > > > src/android/camera_device.cpp | 2 +-\n> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > >\n> > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > > index 1f7ce440..25cedd44 100644\n> > > > > --- a/src/android/camera_device.cpp\n> > > > > +++ b/src/android/camera_device.cpp\n> > > > > @@ -1077,7 +1077,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> > > > > descriptor->request_->addBuffer(sourceStream->stream(),\n> > > > > frameBuffer, nullptr);\n> > > > >\n> > > > > - requestedStreams.erase(sourceStream);\n> > > > > + requestedStreams.insert(sourceStream);\n> > > >\n> > > > So, assuming two Mapped streams that map on the same cameraStream.\n> > > >\n> > > > The first processed one won't find a sourceStream in requestedStream\n> > > >\n> > > > if (requestedStreams.find(sourceStream) != requestedStreams.end())\n> > > > continue;\n> > > >\n> > > > so we don't continue and we add create an internal buffer for it and\n> > > > add the framebuffer for the sourceStream to the requet\n> > > >\n> > > > FrameBuffer *frameBuffer = cameraStream->getBuffer();\n> > > > buffer.internalBuffer = frameBuffer;\n> > > >\n> > > > descriptor->request_->addBuffer(sourceStream->stream(),\n> > > > frameBuffer, nullptr);\n> > > >\n> > > > And this clearly was a nop because of the above if () statement\n> > > >\n> > > > requestedStreams.erase(sourceStream);\n> > > >\n> > > > However, since the second one is a mapped stream too, don't we need to\n> > > > allocate\n> > > > an internal buffer for it ?\n> > > >\n> > > > FrameBuffer *frameBuffer = cameraStream->getBuffer();\n> > > > buffer.internalBuffer = frameBuffer;\n> > > >\n> > > >\n> > > Not really. The second stream can use the same internal buffer allocated by\n> > > the first stream\n> > > to continue the processing. If we allocate a new internal buffer, this will\n> > > fail anyway:\n> > >\n> > > descriptor->request_->addBuffer(sourceStream->stream(),\n> > > \t\t\t\t frameBuffer, nullptr);\n> > >\n> > > , as libcamera::Request already adds the internal buffer to the\n> > > libcamera::Stream.\n>\n> It could be nice to capture a bit more context in the commit message, I\n> had to read through the implementation to understand the issue fixed by\n> this patch.\n>\n> Reviewed-by: Laurent Pinchart \n>\n\nWith Harvey's ack I'll push with the following commit message\n\n-------------------------------------------------------------------------------\nandroid: camera_device: Fix requestedStream handling\n\nThe Android CameraDevice class adds a sourceStream for each Mapped\nstream requested by the framework.\n\nWhen mapping multiple framework streams to the same sourceStream, the\nimplementation of CameraDevice::processCaptureRequest wrongly erases the\njust added sourceStream from the list of streams to request to\nlibcamera.\n\nFix this by adding the stream instead of erasing it.\n-------------------------------------------------------------------------------\n\n> > Inded, you're very right! I clearly was confused as I thought an\n> > \"internal buffer\" had to be allocated, but as we're here handling\n> > mapped streams the destination buffer is provided by the framework.\n> >\n> > Sorry for the noise.\n> >\n> > > The only purpose of setting the internal buffer is to return the allocated\n> > > buffer\n> > > to the CameraStream which created the buffer. See:\n> > >\n> > > https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1246\n> > > https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1370\n> > >\n> > > There are no other usages of\n> > > `Camera3RequesetDescriptor::StreamBuffer::internalBuffer`.\n> >\n> > ack\n> >\n> > > descriptor->pendingStreamsToProcess_.insert({ cameraStream, &buffer });\n> > >\n> > > This will ensure the mapped stream to be processed in:\n> > >\n> > > https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1236\n> >\n> > I re-read the code, but I can't figure out if there are issue in\n> > processing 2 mapped streams created from the same internal buffer, as\n> > I fear that code path has never really been tested ?\n> >\n> > > > With your patch applied I presume the second mapped stream will hit\n> > > >\n> > > > if (requestedStreams.find(sourceStream) != requestedStreams.end())\n> > > > continue;\n> > > >\n> > > > and continue, so no buffer will be allocated for it ?\n> > > >\n> > > > Have you got a test case for this to try ?\n> > >\n> > > Sorry, I've checked with Han-lin, and we don't have such a test with CrOS\n> > > or CTS.\n> >\n> > Exactly, would be nice to test, but in the meantime, your patch seems\n> > to be fixing a bug indeed.\n> >\n> > Thanks!\n> >\n> > Reviewed-by: Jacopo Mondi \n> >\n> > > > > }\n> > > > >\n> > > > > /*\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 03E11BE080\n\tfor ;\n\tThu, 21 Sep 2023 10:50:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 440FD62945;\n\tThu, 21 Sep 2023 12:50:24 +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 200D862931\n\tfor ;\n\tThu, 21 Sep 2023 12:50:23 +0200 (CEST)","from ideasonboard.com (93-46-82-201.ip106.fastwebnet.it\n\t[93.46.82.201])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7DF31B75;\n\tThu, 21 Sep 2023 12:48:45 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695293424;\n\tbh=JHrl1/BX/KPKX0CMEIpKaxiBf7NJIl4rRmtUNgau4E4=;\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:Cc:\n\tFrom;\n\tb=Jr8272D5G3wqs+STJNSb1kStxmCl0S5W+YWFZy547U5XOIuslwTa0/AAWFh38adU6\n\tQl5NArt3YPhnaG6mHI+cilXKE3hVejB4qhGPz7GuvwthzPWWHxcmUKLWSTfqXw6Xnq\n\tgdfNB4+QRRIBjds/EqUnLGH7YXOj4m4uax+yoNyCYlkAQAXETYditoK+XQpHV4bwuI\n\t5vQuCUn0H190vt4LOljq2fB5BZm17Lb0LyWewqp8GuWBsLJEVwNzLeXA/kT30ukV4Y\n\tnTHwQK37mV0dk5mTjOkNNpfDOnaBpYHX06sBixGAJvp6wZqA14EkpXwxH2ffFbMPRl\n\tdBLxpKefx2ylw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1695293325;\n\tbh=JHrl1/BX/KPKX0CMEIpKaxiBf7NJIl4rRmtUNgau4E4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=O9VYNoVvIQWfwxUss+WRQLdXbEUDERgmH1dI6QJ672zs3scpoBet1nRgES/wG3fkU\n\t6QFH8LJu6Courl4R/1A7aBzK0aG1PzfKZJ1VOCZgFOukIArtlG6wfT41caN1ZNtu83\n\ts5iH7vN4E+ngAreCX1wZZIyJJkBwr7AZshMnfkJY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"O9VYNoVv\"; dkim-atps=neutral","Date":"Thu, 21 Sep 2023 12:50:19 +0200","To":"Laurent Pinchart ","Message-ID":"","References":"<20230913152146.636483-1-chenghaoyang@google.com>\n\t<20230913152146.636483-2-chenghaoyang@google.com>\n\t\n\t\n\t\n\t<20230921090857.GA26483@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20230921090857.GA26483@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tCheng-Hao Yang ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27829,"web_url":"https://patchwork.libcamera.org/comment/27829/","msgid":"","date":"2023-09-21T12:13:15","subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Thanks Jacopo! LGTM.\nAlso thanks Laurent for the suggestions.\n\nBR,\nHarvey\n\nOn Thu, Sep 21, 2023 at 6:50 PM Jacopo Mondi \nwrote:\n\n> Hi Laurent\n>\n> On Thu, Sep 21, 2023 at 12:08:57PM +0300, Laurent Pinchart via\n> libcamera-devel wrote:\n> > Hello,\n> >\n> > The subject line should start with \"android: \".\n> >\n> > On Tue, Sep 19, 2023 at 09:52:13AM +0200, Jacopo Mondi via\n> libcamera-devel wrote:\n> > > On Mon, Sep 18, 2023 at 12:18:34PM +0800, Cheng-Hao Yang via\n> libcamera-devel wrote:\n> > > > On Thu, Sep 14, 2023 at 3:47 PM Jacopo Mondi wrote:\n> > > > > On Wed, Sep 13, 2023 at 03:20:50PM +0000, Harvey Yang via\n> libcamera-devel wrote:\n> > > > > > From: Harvey Yang \n> > > > > >\n> > > > > > In CameraDevice::processCaptureRequest, we might need to add an\n> internal\n> > > > > > buffer for Mapped streams. This patch fixes a case that more\n> than one\n> > > > > > Mapped streams depend on a stream that is not requested in one\n> capture\n> > > > > > request.\n> > > > >\n> > > > > Ah! you're right! I wonder how it went unoticed... maybe we never\n> had\n> > > > > to create two Mapped streams from a single buffer ? CTS has been\n> run\n> > > > > multiple times but we never hit this\n> > > > >\n> > > > > > Change-Id: I37a1bcc9c4c2db666a90d74c39883ff18ed11bd5\n> > > > >\n> > > > > This shouldn't be here. We can remove it if you don't have to\n> re-send\n> >\n> > On the other hand, a Fixes: line would be nice. I think\n> >\n> > Fixes: 7ea83eba0df6 (\"android: camera_device: Postpone mapped streams\n> handling\")\n> >\n> > is the right one.\n> >\n> > > > > > Signed-off-by: Harvey Yang \n> > > > > > ---\n> > > > > > src/android/camera_device.cpp | 2 +-\n> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > > >\n> > > > > > diff --git a/src/android/camera_device.cpp\n> b/src/android/camera_device.cpp\n> > > > > > index 1f7ce440..25cedd44 100644\n> > > > > > --- a/src/android/camera_device.cpp\n> > > > > > +++ b/src/android/camera_device.cpp\n> > > > > > @@ -1077,7 +1077,7 @@ int\n> CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> > > > > >\n> descriptor->request_->addBuffer(sourceStream->stream(),\n> > > > > > frameBuffer,\n> nullptr);\n> > > > > >\n> > > > > > - requestedStreams.erase(sourceStream);\n> > > > > > + requestedStreams.insert(sourceStream);\n> > > > >\n> > > > > So, assuming two Mapped streams that map on the same cameraStream.\n> > > > >\n> > > > > The first processed one won't find a sourceStream in\n> requestedStream\n> > > > >\n> > > > > if (requestedStreams.find(sourceStream) !=\n> requestedStreams.end())\n> > > > > continue;\n> > > > >\n> > > > > so we don't continue and we add create an internal buffer for it\n> and\n> > > > > add the framebuffer for the sourceStream to the requet\n> > > > >\n> > > > > FrameBuffer *frameBuffer = cameraStream->getBuffer();\n> > > > > buffer.internalBuffer = frameBuffer;\n> > > > >\n> > > > > descriptor->request_->addBuffer(sourceStream->stream(),\n> > > > > frameBuffer, nullptr);\n> > > > >\n> > > > > And this clearly was a nop because of the above if () statement\n> > > > >\n> > > > > requestedStreams.erase(sourceStream);\n> > > > >\n> > > > > However, since the second one is a mapped stream too, don't we\n> need to\n> > > > > allocate\n> > > > > an internal buffer for it ?\n> > > > >\n> > > > > FrameBuffer *frameBuffer =\n> cameraStream->getBuffer();\n> > > > > buffer.internalBuffer = frameBuffer;\n> > > > >\n> > > > >\n> > > > Not really. The second stream can use the same internal buffer\n> allocated by\n> > > > the first stream\n> > > > to continue the processing. If we allocate a new internal buffer,\n> this will\n> > > > fail anyway:\n> > > >\n> > > > descriptor->request_->addBuffer(sourceStream->stream(),\n> > > > frameBuffer, nullptr);\n> > > >\n> > > > , as libcamera::Request already adds the internal buffer to the\n> > > > libcamera::Stream.\n> >\n> > It could be nice to capture a bit more context in the commit message, I\n> > had to read through the implementation to understand the issue fixed by\n> > this patch.\n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n>\n> With Harvey's ack I'll push with the following commit message\n>\n>\n> -------------------------------------------------------------------------------\n> android: camera_device: Fix requestedStream handling\n>\n> The Android CameraDevice class adds a sourceStream for each Mapped\n> stream requested by the framework.\n>\n> When mapping multiple framework streams to the same sourceStream, the\n> implementation of CameraDevice::processCaptureRequest wrongly erases the\n> just added sourceStream from the list of streams to request to\n> libcamera.\n>\n> Fix this by adding the stream instead of erasing it.\n>\n> -------------------------------------------------------------------------------\n>\n> > > Inded, you're very right! I clearly was confused as I thought an\n> > > \"internal buffer\" had to be allocated, but as we're here handling\n> > > mapped streams the destination buffer is provided by the framework.\n> > >\n> > > Sorry for the noise.\n> > >\n> > > > The only purpose of setting the internal buffer is to return the\n> allocated\n> > > > buffer\n> > > > to the CameraStream which created the buffer. See:\n> > > >\n> > > >\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1246\n> > > >\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1370\n> > > >\n> > > > There are no other usages of\n> > > > `Camera3RequesetDescriptor::StreamBuffer::internalBuffer`.\n> > >\n> > > ack\n> > >\n> > > > descriptor->pendingStreamsToProcess_.insert({ cameraStream, &buffer\n> });\n> > > >\n> > > > This will ensure the mapped stream to be processed in:\n> > > >\n> > > >\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_device.cpp#n1236\n> > >\n> > > I re-read the code, but I can't figure out if there are issue in\n> > > processing 2 mapped streams created from the same internal buffer, as\n> > > I fear that code path has never really been tested ?\n> > >\n> > > > > With your patch applied I presume the second mapped stream will hit\n> > > > >\n> > > > > if (requestedStreams.find(sourceStream) !=\n> requestedStreams.end())\n> > > > > continue;\n> > > > >\n> > > > > and continue, so no buffer will be allocated for it ?\n> > > > >\n> > > > > Have you got a test case for this to try ?\n> > > >\n> > > > Sorry, I've checked with Han-lin, and we don't have such a test with\n> CrOS\n> > > > or CTS.\n> > >\n> > > Exactly, would be nice to test, but in the meantime, your patch seems\n> > > to be fixing a bug indeed.\n> > >\n> > > Thanks!\n> > >\n> > > Reviewed-by: Jacopo Mondi \n> > >\n> > > > > > }\n> > > > > >\n> > > > > > /*\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\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 350B6BE080\n\tfor ;\n\tThu, 21 Sep 2023 12:13:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 614EC62944;\n\tThu, 21 Sep 2023 14:13:29 +0200 (CEST)","from mail-lj1-x229.google.com (mail-lj1-x229.google.com\n\t[IPv6:2a00:1450:4864:20::229])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AA603628D8\n\tfor ;\n\tThu, 21 Sep 2023 14:13:27 +0200 (CEST)","by mail-lj1-x229.google.com with SMTP id\n\t38308e7fff4ca-2bffc55af02so13997381fa.2\n\tfor ;\n\tThu, 21 Sep 2023 05:13:27 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695298409;\n\tbh=BssVetdsgEHhV8+3xuqO+8v0WKx6hfcpHt5aj13Q4AM=;\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=h2Iz6QZEUEWAmXB0DaoR5dxahK4crKeVfhv6ZF2EBJKXqkRYVO0RCYp6H2+bv0Gfq\n\tic648XI2WbQFsogOIHv/acWHCgaulI0pJZi2uvLlChyWDCAn8KjLLcQzb5m1AQT6GX\n\tMoYVAbfAyOzrMaLfg4SnQeSgYLJTeCadHCyQHoXl4H9l6NZKsHph/XHgJZy28F0aIQ\n\toxWbk6UIOjRC+9EbxJqi85A2lGCBT8RMrpqKWoZFnoHC6dnP7oIqVcwqcWXTOpUuMZ\n\td29Ee+mu5JfnLukoUxaLL3pci1rZkTE+WonNzQ6LlnIk+HpcH8nIVqd1CkG6rRCKWB\n\tRc+2h9PE4PLaw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1695298407; x=1695903207;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=5Oo/tC5QjS1qur4a0dfvh/Fr7QqetkC3bM2t6fT+PrE=;\n\tb=ByCzmjqtsQsI6al1cwZizX/Rzn5zTp+l80sUn913Uwsjw2CZqpMAyzyMCOElofoipQ\n\tSiAy9z8FF/z/gZBOlAEcmRDa8kT668psSQL3OCcn+Lsor60YqySXg3+fEUJ1lzwClqMO\n\tyrooz7C18ACtZrovuahFqiQ+4gVoRa6uSULO8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"ByCzmjqt\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1695298407; x=1695903207;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=5Oo/tC5QjS1qur4a0dfvh/Fr7QqetkC3bM2t6fT+PrE=;\n\tb=MFp0n6ZYuw1iMmvivuP1DfUQb3VvTDk8nt4LFio6Rfa6xUHH4fI1MdwPlBAyqjrN2K\n\tGdoZSfuw0eyetW7jeAuQpXxmcRkkYGKFrvZbfrEX4F4gKfD0txnw7u2IjR3mpkSrxTyL\n\tRfwf3p/9B2qPfl4dAjJ4ChC9qL+No+sVspEpyoz5h0sIM0Gxozg2qtAn+O5K7xu31/CB\n\trMYTs0HBJwgQavmZ0c76JP2Nt/1bvJEaPk+QaT3F3lg/gbI6yh5RlrUWEYneBBcyc8Hd\n\tLXUh56hmw4p7s8XwK+zszPD8z1jp8eCT1pLo6Qp1/ozqRZPu1iVwErSpy618X/XkH3ZP\n\t7ZFQ==","X-Gm-Message-State":"AOJu0Yz58lb1EBCh0jRu7hq9qz9UYgMOSFgmPTYLXGvZSOXA+hyQFjQ+\n\tv7azPGBurKQm07n+3XzbWckTCAjQ7GHzFboem95ZktcBGcHNoliR","X-Google-Smtp-Source":"AGHT+IGxuc3n7ugprJ35n/PwyN5MSCtlMx4FsIv8ibDqBnQSyvvkoL/ZqG3g9HsUs7wOxvBK7Al8V38l5u4Ord6xWoI=","X-Received":"by 2002:a2e:8683:0:b0:2c0:9bd:c6f with SMTP id\n\tl3-20020a2e8683000000b002c009bd0c6fmr4976828lji.41.1695298406461; \n\tThu, 21 Sep 2023 05:13:26 -0700 (PDT)","MIME-Version":"1.0","References":"<20230913152146.636483-1-chenghaoyang@google.com>\n\t<20230913152146.636483-2-chenghaoyang@google.com>\n\t\n\t\n\t\n\t<20230921090857.GA26483@pendragon.ideasonboard.com>\n\t","In-Reply-To":"","Date":"Thu, 21 Sep 2023 20:13:15 +0800","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"0000000000004e46b80605dd6bd5\"","Subject":"Re: [libcamera-devel] [PATCH v1 1/1] Android adapter: CameraDevice\n\tfixes shared internal buffer","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","From":"Cheng-Hao Yang via libcamera-devel\n\t","Reply-To":"Cheng-Hao Yang ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]