[{"id":24139,"web_url":"https://patchwork.libcamera.org/comment/24139/","msgid":"<841f182f-1c0c-e060-833b-f61296a52acf@ideasonboard.com>","date":"2022-07-26T07:16:08","subject":"Re: [libcamera-devel] [PATCH v3 2/9] ipu3: Allow only one camera\n\tbeing started","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Harvey,\n\nThank you for the patch.\n\nOn 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:\n> From: Harvey Yang \n>\n> As we hardly have use cases/applications that need both cameras at the\n> same time, this patch adds a rule that only one camera can be started\n> one time. This also allows the following patches that use both imgus to\n\n\nBefore I go any further, I believe there are CTS tests that require \nmultiple streaming cameras at same point.\n\nFor e.g. I see android.hardware.cts.CameraTest#testMultipleCameras [1]\n\nIs my understanding correct that streaming multiple cameras at the same \nis a CTS requirement ? Doesn't this patch violate that?\n\nI also see a related patch on the list under `[PATCH] libcamera: Allow \nconcurrent use of cameras from same pipeline handler` subject. Would you \nlike to review/discuss or clarify the situation here please?\n\n[1] \nhttps://android.googlesource.com/platform/cts/+/refs/heads/master/tests/camera/src/android/hardware/camera2/cts/CameraManagerTest.java#337\n\n> process frames from one single camera.\n>\n> Signed-off-by: Harvey Yang \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 16 ++++++++++++++++\n> 1 file changed, 16 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index fd989e61..c943ee6a 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -166,6 +166,8 @@ private:\n> \tMediaDevice *cio2MediaDev_;\n> \tMediaDevice *imguMediaDev_;\n> \n> +\tCamera *inUseCamera_ = nullptr;\n> +\n> \tstd::vector ipaBuffers_;\n> };\n> \n> @@ -765,6 +767,14 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n> \n> int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlList *controls)\n> {\n> +\t/*\n> +\t * Enforce that only a single camera can be used at a time to use both\n> +\t * ImgUs on the camera, so that StillCapture stream can adopt another\n> +\t * set of configuration.\n> +\t */\n> +\tif (inUseCamera_ && inUseCamera_ != camera)\n> +\t\treturn -EBUSY;\n\n\nI think this is a wrong place to return -EBUSY.\n\nAfter Camera::acquire() success, we fail to start the camera reporting \n-EBUSY, is not a good behavior IMO\n\n> +\n> \tIPU3CameraData *data = cameraData(camera);\n> \tCIO2Device *cio2 = &data->cio2_;\n> \tImgUDevice *imgu = data->imgu_;\n> @@ -781,6 +791,8 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tinUseCamera_ = camera;\n> +\n> \tret = data->ipa_->start();\n> \tif (ret)\n> \t\tgoto error;\n> @@ -808,6 +820,8 @@ error:\n> \tfreeBuffers(camera);\n> \tLOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n> \n> +\tinUseCamera_ = nullptr;\n> +\n> \treturn ret;\n> }\n> \n> @@ -826,6 +840,8 @@ void PipelineHandlerIPU3::stopDevice(Camera *camera)\n> \t\tLOG(IPU3, Warning) << \"Failed to stop camera \" << camera->id();\n> \n> \tfreeBuffers(camera);\n> +\n> +\tinUseCamera_ = nullptr;\n> }\n> \n> void IPU3CameraData::cancelPendingRequests()","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 7CC98C3275\n\tfor ;\n\tTue, 26 Jul 2022 07:16:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D139563312;\n\tTue, 26 Jul 2022 09:16:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 940486330D\n\tfor ;\n\tTue, 26 Jul 2022 09:16:15 +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 0DAD1735;\n\tTue, 26 Jul 2022 09:16:13 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658819776;\n\tbh=wOzHBjLeUQ+jVjtWpaNBRvGwdQqfB3UFu8gs6eo3V+k=;\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=uTEE8jPfcxbe6Bip4oJv7m0aZ3RN6vSAZIqRZlPMnm0t0xa7HgqOi3D4nw/ZNfC6+\n\tTP1zxGO/P81mmA5q4t9Xc7bgxW2zFogzmc4DtXdlFUR7vTh2EFVlrr1tMkC+uiKwZR\n\tjNLedFz43AN3Od2oi4svZaHH1RVbRYaNboCxB3Gzttd6fbLW/sTGeJK7It/K3fd6tW\n\tEQsmJjv8zsaoHm/dhu7wF4hiRL5gDxZOokQ4n53h8h1RbaEuikJHXsG1zFY5gazaU/\n\ttTXQB+D9E8vMdtSj7eSdmo/7cI95HTokW3UfWcJ5oy3BofaIZooU0QlhDSm5phhSw1\n\txGyppA9921UKw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658819775;\n\tbh=wOzHBjLeUQ+jVjtWpaNBRvGwdQqfB3UFu8gs6eo3V+k=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=El3BqS1Pfv68MJvRjq1oXfMt0S0XeycOtxAzcnulSSsPxVvnOl9iNnDBRCtk8BVq/\n\t0Fv+M9ucRMK2/8Td6Vimgwk+QeQEGErcGK8yaYYpkcS2b6CzoowBjaFWxNjMgZDsst\n\tG4FheU93x+9jy9kdNRO2/Mbtnj7ra81HbjSh2lxg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"El3BqS1P\"; dkim-atps=neutral","Message-ID":"<841f182f-1c0c-e060-833b-f61296a52acf@ideasonboard.com>","Date":"Tue, 26 Jul 2022 12:46:08 +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 ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220629103018.4025635-1-chenghaoyang@google.com>\n\t<20220629103018.4025635-3-chenghaoyang@google.com>","In-Reply-To":"<20220629103018.4025635-3-chenghaoyang@google.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 2/9] ipu3: Allow only one camera\n\tbeing started","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":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24146,"web_url":"https://patchwork.libcamera.org/comment/24146/","msgid":"","date":"2022-07-26T12:20:37","subject":"Re: [libcamera-devel] [PATCH v3 2/9] ipu3: Allow only one camera\n\tbeing started","submitter":{"id":98,"url":"https://patchwork.libcamera.org/api/people/98/","name":"Hanlin Chen","email":"hanlinchen@chromium.org"},"content":"On Tue, Jul 26, 2022 at 3:16 PM Umang Jain via libcamera-devel\n wrote:\n>\n> Hi Harvey,\n>\n> Thank you for the patch.\n>\n> On 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:\n> > From: Harvey Yang \n> >\n> > As we hardly have use cases/applications that need both cameras at the\n> > same time, this patch adds a rule that only one camera can be started\n> > one time. This also allows the following patches that use both imgus to\n>\n>\n> Before I go any further, I believe there are CTS tests that require\n> multiple streaming cameras at same point.\n>\n> For e.g. I see android.hardware.cts.CameraTest#testMultipleCameras [1]\n>\n> Is my understanding correct that streaming multiple cameras at the same\n> is a CTS requirement ? Doesn't this patch violate that?\n>\n> I also see a related patch on the list under `[PATCH] libcamera: Allow\n> concurrent use of cameras from same pipeline handler` subject. Would you\n> like to review/discuss or clarify the situation here please?\n>\n> [1]\n> https://android.googlesource.com/platform/cts/+/refs/heads/master/tests/camera/src/android/hardware/camera2/cts/CameraManagerTest.java#337\n>\nHi Umang and Harvey,\n\nI think the test accept MAX_CAMERAS_IN_USE error as a pass situation,\nwhich means HAL rejects the open due to internal resource limit. The\nerror is set when CameraDevice::open() returns -EUSER:\n* -EUSERS: The maximal number of camera devices that can be opened\nconcurrently were opened already, either by this method or the\nopen_legacy method.\nThe problem here is that we didn't return -EUSERS on Camera::acquire()\nwhich is called by CameraDevice::open().\nInstead of checking this in PipelineHandlerIPU3::start(),\nCamera::acquire() could call into the pipeline handler to check the\nresource limit and return the -EUSER error directly, since only the\npipeline handler knows how to manage its resources.\nSee:\nhttps://android.googlesource.com/platform/cts/+/refs/heads/master/tests/camera/src/android/hardware/camera2/cts/CameraManagerTest.java#409\nhttps://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/camera_common.h#872\n\nI think \"[PATCH] libcamera: Allow concurrent use of cameras from same\npipeline handler\" is resolving a different problem. It allows opening\nmulti cameras if resources are adequate. Our case is to report the\nerror accordingly when resources are inadequate.\n\n> > process frames from one single camera.\n> >\n> > Signed-off-by: Harvey Yang \n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 16 ++++++++++++++++\n> > 1 file changed, 16 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index fd989e61..c943ee6a 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -166,6 +166,8 @@ private:\n> > MediaDevice *cio2MediaDev_;\n> > MediaDevice *imguMediaDev_;\n> >\n> > + Camera *inUseCamera_ = nullptr;\n> > +\n> > std::vector ipaBuffers_;\n> > };\n> >\n> > @@ -765,6 +767,14 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n> >\n> > int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlList *controls)\n> > {\n> > + /*\n> > + * Enforce that only a single camera can be used at a time to use both\n> > + * ImgUs on the camera, so that StillCapture stream can adopt another\n> > + * set of configuration.\n> > + */\n> > + if (inUseCamera_ && inUseCamera_ != camera)\n> > + return -EBUSY;\n>\n>\n> I think this is a wrong place to return -EBUSY.\n>\n> After Camera::acquire() success, we fail to start the camera reporting\n> -EBUSY, is not a good behavior IMO\n>\n> > +\n> > IPU3CameraData *data = cameraData(camera);\n> > CIO2Device *cio2 = &data->cio2_;\n> > ImgUDevice *imgu = data->imgu_;\n> > @@ -781,6 +791,8 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis\n> > if (ret)\n> > return ret;\n> >\n> > + inUseCamera_ = camera;\n> > +\n> > ret = data->ipa_->start();\n> > if (ret)\n> > goto error;\n> > @@ -808,6 +820,8 @@ error:\n> > freeBuffers(camera);\n> > LOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n> >\n> > + inUseCamera_ = nullptr;\n> > +\n> > return ret;\n> > }\n> >\n> > @@ -826,6 +840,8 @@ void PipelineHandlerIPU3::stopDevice(Camera *camera)\n> > LOG(IPU3, Warning) << \"Failed to stop camera \" << camera->id();\n> >\n> > freeBuffers(camera);\n> > +\n> > + inUseCamera_ = nullptr;\n> > }\n> >\n> > void IPU3CameraData::cancelPendingRequests()","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 0A584BE173\n\tfor ;\n\tTue, 26 Jul 2022 12:20:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5A3CA63312;\n\tTue, 26 Jul 2022 14:20:52 +0200 (CEST)","from mail-oi1-x22c.google.com (mail-oi1-x22c.google.com\n\t[IPv6:2607:f8b0:4864:20::22c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 10A1460487\n\tfor ;\n\tTue, 26 Jul 2022 14:20:50 +0200 (CEST)","by mail-oi1-x22c.google.com with SMTP id w204so16825550oie.7\n\tfor ;\n\tTue, 26 Jul 2022 05:20:49 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658838052;\n\tbh=coq8ZdyabthezH6V4Hl5H18mRnKUTgx1hX6ZBl1gPi0=;\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=fGFfhV1iGJ/igZpu1NW3MYEs/F0hc7ri5imPB8IZ7d3KR8CGMKG86JR675Fcy9+tN\n\th2NcQsEGYYbbR+7VoGEH++wmM8Yjl4NgGMCJ5YL7o9x2DhKWOLKk+HGRhR5KBJDIYb\n\tVgoAH23yJog+7qSQEm+1mZuBG74PCvjTwWY5lq+RWgDSlkWXBsUPYsVz/CUu5NAXL9\n\t0jeS+GGnmyk3CrtXcwbCfQCD3NtPTCtwu+bNC4NFjPlidl8U9QO/0NDnETLToQXVPW\n\tnOS8QHbmW1DvLAgGHZwZ3rbsJZ6/rfmWdg7cBjtcOEtnctGHCmRFAxar+XdWDYD6qr\n\tJbOdnRSmDGxcw==","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=V4Kw9gYOTwLIThAJPkHpYxZw5s3fN0Aqx0bGuoHGuEQ=;\n\tb=aVgXsMhFmPh+o1Jj71oY5MD9wWoMj1YykXBlUQQZs5ezvd3r8JgI0MtKuMa5VMZr8R\n\trWahr1bf6a4t4qS2pSoywpH9WnfcDkmoGzt7zVGGMr4kEyRCkwvl1LkeYWrQo7SiQIUb\n\tYdQe0NJWqY/pjbdng3pufVKafISlFt8G48tug="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"aVgXsMhF\"; \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=V4Kw9gYOTwLIThAJPkHpYxZw5s3fN0Aqx0bGuoHGuEQ=;\n\tb=wxD1RcxLpJkjzNGV1NTULsYpTO9L5FO2oWFFf1+69jm5+gSopG+jtDLvJUcvJXnJBm\n\tjxW3juufloGLGQlB0R0VEoN+6ncfsM1MaTOkrqXAbTQoe/nFDM2/bloaceqdFy5ciIXE\n\tJmgQSDYsKBJLopObB2p19V5q/6BfDpH0rpTQkknE3SnucfJeIrffOA2VXtGeaYasnI+9\n\tsYY/BPWObRJaa/Vswz2+vS+uXqCsbDu3jkLwTzbkSxabSItkhFQjnblg/oakZGZmU9GZ\n\tjSAkGu8yFvxSN9q+MfN6zn2tnjVIvQRHSaQ49vaTM+el7ngw2of9hU3I3lhflvep/m35\n\thDRg==","X-Gm-Message-State":"AJIora8Wd/TdFZCWAejicl6Jx2/jUjfCs0hlHNHjyXJI+BN/aRIdVXHp\n\tAV/BoKxRvvPFL79yDaKpX+3cYk3qSDmqTsOHRAFWeDrSqODT+g==","X-Google-Smtp-Source":"AGRyM1vajsFO741gV0kfXmuuMLJ9AR+eaGkmcHKkxF1sjFhOsStU/xENvndyYJs0qTdjiITVVKfKTU04ikC5Dlq1uac=","X-Received":"by 2002:a05:6808:20a9:b0:33a:91a9:4846 with SMTP id\n\ts41-20020a05680820a900b0033a91a94846mr14335981oiw.65.1658838048673;\n\tTue, 26 Jul 2022 05:20:48 -0700 (PDT)","MIME-Version":"1.0","References":"<20220629103018.4025635-1-chenghaoyang@google.com>\n\t<20220629103018.4025635-3-chenghaoyang@google.com>\n\t<841f182f-1c0c-e060-833b-f61296a52acf@ideasonboard.com>","In-Reply-To":"<841f182f-1c0c-e060-833b-f61296a52acf@ideasonboard.com>","Date":"Tue, 26 Jul 2022 20:20:37 +0800","Message-ID":"","To":"Umang Jain ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 2/9] ipu3: Allow only one camera\n\tbeing started","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":"Hanlin Chen via libcamera-devel ","Reply-To":"Hanlin Chen ","Cc":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24151,"web_url":"https://patchwork.libcamera.org/comment/24151/","msgid":"<7d6ce545-32a8-9152-c681-bc00f2e2d810@ideasonboard.com>","date":"2022-07-26T15:47:59","subject":"Re: [libcamera-devel] [PATCH v3 2/9] ipu3: Allow only one camera\n\tbeing started","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Hanlin,\n\nOn 7/26/22 17:50, Hanlin Chen wrote:\n> On Tue, Jul 26, 2022 at 3:16 PM Umang Jain via libcamera-devel\n> wrote:\n>> Hi Harvey,\n>>\n>> Thank you for the patch.\n>>\n>> On 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:\n>>> From: Harvey Yang \n>>>\n>>> As we hardly have use cases/applications that need both cameras at the\n>>> same time, this patch adds a rule that only one camera can be started\n>>> one time. This also allows the following patches that use both imgus to\n>>\n>> Before I go any further, I believe there are CTS tests that require\n>> multiple streaming cameras at same point.\n>>\n>> For e.g. I see android.hardware.cts.CameraTest#testMultipleCameras [1]\n>>\n>> Is my understanding correct that streaming multiple cameras at the same\n>> is a CTS requirement ? Doesn't this patch violate that?\n>>\n>> I also see a related patch on the list under `[PATCH] libcamera: Allow\n>> concurrent use of cameras from same pipeline handler` subject. Would you\n>> like to review/discuss or clarify the situation here please?\n>>\n>> [1]\n>> https://android.googlesource.com/platform/cts/+/refs/heads/master/tests/camera/src/android/hardware/camera2/cts/CameraManagerTest.java#337\n>>\n> Hi Umang and Harvey,\n>\n> I think the test accept MAX_CAMERAS_IN_USE error as a pass situation,\n> which means HAL rejects the open due to internal resource limit. The\n> error is set when CameraDevice::open() returns -EUSER:\n> * -EUSERS: The maximal number of camera devices that can be opened\n> concurrently were opened already, either by this method or the\n> open_legacy method.\n> The problem here is that we didn't return -EUSERS on Camera::acquire()\n> which is called by CameraDevice::open().\n> Instead of checking this in PipelineHandlerIPU3::start(),\n> Camera::acquire() could call into the pipeline handler to check the\n> resource limit and return the -EUSER error directly, since only the\n> pipeline handler knows how to manage its resources.\n\n\nI agree with the premise here, but currently I think pipeline-handlers \ndon't have a concept of \"resource limiting\" being implemented; based on \nwhich we decide whether a camera can be acquired or not.\n\nThe patch in discussion below, implements a acquire() per \npipeline-handler so it might be good to extend that to satisfy this case \nfor IPU3 platform..\n\n\n> See:\n> https://android.googlesource.com/platform/cts/+/refs/heads/master/tests/camera/src/android/hardware/camera2/cts/CameraManagerTest.java#409\n> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/camera_common.h#872\n>\n> I think \"[PATCH] libcamera: Allow concurrent use of cameras from same\n> pipeline handler\" is resolving a different problem. It allows opening\n> multi cameras if resources are adequate. Our case is to report the\n> error accordingly when resources are inadequate.\n>\n>>> process frames from one single camera.\n>>>\n>>> Signed-off-by: Harvey Yang \n>>> ---\n>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 16 ++++++++++++++++\n>>> 1 file changed, 16 insertions(+)\n>>>\n>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> index fd989e61..c943ee6a 100644\n>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> @@ -166,6 +166,8 @@ private:\n>>> MediaDevice *cio2MediaDev_;\n>>> MediaDevice *imguMediaDev_;\n>>>\n>>> + Camera *inUseCamera_ = nullptr;\n>>> +\n>>> std::vector ipaBuffers_;\n>>> };\n>>>\n>>> @@ -765,6 +767,14 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n>>>\n>>> int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlList *controls)\n>>> {\n>>> + /*\n>>> + * Enforce that only a single camera can be used at a time to use both\n>>> + * ImgUs on the camera, so that StillCapture stream can adopt another\n>>> + * set of configuration.\n>>> + */\n>>> + if (inUseCamera_ && inUseCamera_ != camera)\n>>> + return -EBUSY;\n>>\n>> I think this is a wrong place to return -EBUSY.\n>>\n>> After Camera::acquire() success, we fail to start the camera reporting\n>> -EBUSY, is not a good behavior IMO\n>>\n>>> +\n>>> IPU3CameraData *data = cameraData(camera);\n>>> CIO2Device *cio2 = &data->cio2_;\n>>> ImgUDevice *imgu = data->imgu_;\n>>> @@ -781,6 +791,8 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis\n>>> if (ret)\n>>> return ret;\n>>>\n>>> + inUseCamera_ = camera;\n>>> +\n>>> ret = data->ipa_->start();\n>>> if (ret)\n>>> goto error;\n>>> @@ -808,6 +820,8 @@ error:\n>>> freeBuffers(camera);\n>>> LOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n>>>\n>>> + inUseCamera_ = nullptr;\n>>> +\n>>> return ret;\n>>> }\n>>>\n>>> @@ -826,6 +840,8 @@ void PipelineHandlerIPU3::stopDevice(Camera *camera)\n>>> LOG(IPU3, Warning) << \"Failed to stop camera \" << camera->id();\n>>>\n>>> freeBuffers(camera);\n>>> +\n>>> + inUseCamera_ = nullptr;\n>>> }\n>>>\n>>> void IPU3CameraData::cancelPendingRequests()","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 C566BBE173\n\tfor ;\n\tTue, 26 Jul 2022 15:48:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 79B9363312;\n\tTue, 26 Jul 2022 17:48:06 +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 AE58D60487\n\tfor ;\n\tTue, 26 Jul 2022 17:48:04 +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 ADF97735;\n\tTue, 26 Jul 2022 17:48:03 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658850486;\n\tbh=CNKZj6xIRnaJU+mvGqZW4OZWEEhWGgR5QQA8nT4Qg8M=;\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=PvMQGK9YDkNs0+aQKuQhszAXFB85sUI7TPSeaqK/aS4q5oXum/XcEC1KEkmCUExEf\n\tQ9I6m9YZ9e57/2fmY4ys2WQuEKclwFkIynsY9eeALQ5AO0hxTeHk3ozXUOL/laPAfA\n\t/BL7g5FofydaT/LU6JxWouOxpSTPnTAuNFKQ7o5xBw2k+chM4/G9Il+oGDN82ppEmc\n\tTXU8vlPJgLb5W2rCyMk7+pi8rTmTpdkb4xhz6fRe+YjK8jYO8QwPyFWear4G/IYIEy\n\t9SVGw0Y8UD2ubsCDssTS96jh9ruBrU9jVpIxzJhRnWX/QRFDBs9kPyY75Stc3Y6WD1\n\tzt5Zec/o34aWg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658850484;\n\tbh=CNKZj6xIRnaJU+mvGqZW4OZWEEhWGgR5QQA8nT4Qg8M=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=UcwKzyE7HH/DusHd1CI2NtyTcthyysqrB9td7BrCAHtNJ+5Rje7/A0kCqYSkW9j3h\n\tKhymqJlyMdfhkdBNp+GMSDoXseEQiZjb7HwL/COlBh1c5/tpAzHOfc2GjrKUEcFBJr\n\ttyxGXszNM/G8Y7uXyfFYBqsyqFoh3gjYzjbvbdsA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"UcwKzyE7\"; dkim-atps=neutral","Message-ID":"<7d6ce545-32a8-9152-c681-bc00f2e2d810@ideasonboard.com>","Date":"Tue, 26 Jul 2022 21:17:59 +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":"Hanlin Chen ","References":"<20220629103018.4025635-1-chenghaoyang@google.com>\n\t<20220629103018.4025635-3-chenghaoyang@google.com>\n\t<841f182f-1c0c-e060-833b-f61296a52acf@ideasonboard.com>\n\t","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 2/9] ipu3: Allow only one camera\n\tbeing started","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":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Cc":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24295,"web_url":"https://patchwork.libcamera.org/comment/24295/","msgid":"","date":"2022-08-02T10:30:19","subject":"Re: [libcamera-devel] [PATCH v3 2/9] ipu3: Allow only one camera\n\tbeing started","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Thanks Umang and Han-lin,\n\nAn interesting fact is that the current patch (v3) actually passes\nandroid.hardware.cts.CameraTest#testMultipleCameras. Maybe it only opens\nthe cameras without actually starting them and getting frames?\n\nNonetheless, I updated Camera::acquire() and Camera::release() to check\nwith the pipeline handler, and still makes PipelineHandlerIPU3 support only\none camera being acquired at a time. Please check.\n\n> > int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const\nControlList *controls)\n> > {\n> > + /*\n> > + * Enforce that only a single camera can be used at a time to use\nboth\n> > + * ImgUs on the camera, so that StillCapture stream can adopt\nanother\n> > + * set of configuration.\n> > + */\n> > + if (inUseCamera_ && inUseCamera_ != camera)\n> > + return -EBUSY;\n\n> I think this is a wrong place to return -EBUSY.\n\n> After Camera::acquire() success, we fail to start the camera reporting\n> -EBUSY, is not a good behavior IMO\n\nRight, how about returning -EUSERS as well? With v4, it actually should\nnever happen.\n\nOn Tue, Jul 26, 2022 at 11:48 PM Umang Jain \nwrote:\n\n> Hi Hanlin,\n>\n> On 7/26/22 17:50, Hanlin Chen wrote:\n> > On Tue, Jul 26, 2022 at 3:16 PM Umang Jain via libcamera-devel\n> > wrote:\n> >> Hi Harvey,\n> >>\n> >> Thank you for the patch.\n> >>\n> >> On 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:\n> >>> From: Harvey Yang \n> >>>\n> >>> As we hardly have use cases/applications that need both cameras at the\n> >>> same time, this patch adds a rule that only one camera can be started\n> >>> one time. This also allows the following patches that use both imgus to\n> >>\n> >> Before I go any further, I believe there are CTS tests that require\n> >> multiple streaming cameras at same point.\n> >>\n> >> For e.g. I see android.hardware.cts.CameraTest#testMultipleCameras [1]\n> >>\n> >> Is my understanding correct that streaming multiple cameras at the same\n> >> is a CTS requirement ? Doesn't this patch violate that?\n> >>\n> >> I also see a related patch on the list under `[PATCH] libcamera: Allow\n> >> concurrent use of cameras from same pipeline handler` subject. Would you\n> >> like to review/discuss or clarify the situation here please?\n> >>\n> >> [1]\n> >>\n> https://android.googlesource.com/platform/cts/+/refs/heads/master/tests/camera/src/android/hardware/camera2/cts/CameraManagerTest.java#337\n> >>\n> > Hi Umang and Harvey,\n> >\n> > I think the test accept MAX_CAMERAS_IN_USE error as a pass situation,\n> > which means HAL rejects the open due to internal resource limit. The\n> > error is set when CameraDevice::open() returns -EUSER:\n> > * -EUSERS: The maximal number of camera devices that can be opened\n> > concurrently were opened already, either by this method or the\n> > open_legacy method.\n> > The problem here is that we didn't return -EUSERS on Camera::acquire()\n> > which is called by CameraDevice::open().\n> > Instead of checking this in PipelineHandlerIPU3::start(),\n> > Camera::acquire() could call into the pipeline handler to check the\n> > resource limit and return the -EUSER error directly, since only the\n> > pipeline handler knows how to manage its resources.\n>\n>\n> I agree with the premise here, but currently I think pipeline-handlers\n> don't have a concept of \"resource limiting\" being implemented; based on\n> which we decide whether a camera can be acquired or not.\n>\n> The patch in discussion below, implements a acquire() per\n> pipeline-handler so it might be good to extend that to satisfy this case\n> for IPU3 platform..\n>\n>\n> > See:\n> >\n> https://android.googlesource.com/platform/cts/+/refs/heads/master/tests/camera/src/android/hardware/camera2/cts/CameraManagerTest.java#409\n> >\n> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/camera_common.h#872\n> >\n> > I think \"[PATCH] libcamera: Allow concurrent use of cameras from same\n> > pipeline handler\" is resolving a different problem. It allows opening\n> > multi cameras if resources are adequate. Our case is to report the\n> > error accordingly when resources are inadequate.\n> >\n> >>> process frames from one single camera.\n> >>>\n> >>> Signed-off-by: Harvey Yang \n> >>> ---\n> >>> src/libcamera/pipeline/ipu3/ipu3.cpp | 16 ++++++++++++++++\n> >>> 1 file changed, 16 insertions(+)\n> >>>\n> >>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >>> index fd989e61..c943ee6a 100644\n> >>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >>> @@ -166,6 +166,8 @@ private:\n> >>> MediaDevice *cio2MediaDev_;\n> >>> MediaDevice *imguMediaDev_;\n> >>>\n> >>> + Camera *inUseCamera_ = nullptr;\n> >>> +\n> >>> std::vector ipaBuffers_;\n> >>> };\n> >>>\n> >>> @@ -765,6 +767,14 @@ int PipelineHandlerIPU3::freeBuffers(Camera\n> *camera)\n> >>>\n> >>> int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]]\n> const ControlList *controls)\n> >>> {\n> >>> + /*\n> >>> + * Enforce that only a single camera can be used at a time to\n> use both\n> >>> + * ImgUs on the camera, so that StillCapture stream can adopt\n> another\n> >>> + * set of configuration.\n> >>> + */\n> >>> + if (inUseCamera_ && inUseCamera_ != camera)\n> >>> + return -EBUSY;\n> >>\n> >> I think this is a wrong place to return -EBUSY.\n> >>\n> >> After Camera::acquire() success, we fail to start the camera reporting\n> >> -EBUSY, is not a good behavior IMO\n> >>\n> >>> +\n> >>> IPU3CameraData *data = cameraData(camera);\n> >>> CIO2Device *cio2 = &data->cio2_;\n> >>> ImgUDevice *imgu = data->imgu_;\n> >>> @@ -781,6 +791,8 @@ int PipelineHandlerIPU3::start(Camera *camera,\n> [[maybe_unused]] const ControlLis\n> >>> if (ret)\n> >>> return ret;\n> >>>\n> >>> + inUseCamera_ = camera;\n> >>> +\n> >>> ret = data->ipa_->start();\n> >>> if (ret)\n> >>> goto error;\n> >>> @@ -808,6 +820,8 @@ error:\n> >>> freeBuffers(camera);\n> >>> LOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n> >>>\n> >>> + inUseCamera_ = nullptr;\n> >>> +\n> >>> return ret;\n> >>> }\n> >>>\n> >>> @@ -826,6 +840,8 @@ void PipelineHandlerIPU3::stopDevice(Camera\n> *camera)\n> >>> LOG(IPU3, Warning) << \"Failed to stop camera \" <<\n> camera->id();\n> >>>\n> >>> freeBuffers(camera);\n> >>> +\n> >>> + inUseCamera_ = nullptr;\n> >>> }\n> >>>\n> >>> void IPU3CameraData::cancelPendingRequests()\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 35609C3275\n\tfor ;\n\tTue, 2 Aug 2022 10:30:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE46B63315;\n\tTue, 2 Aug 2022 12:30:32 +0200 (CEST)","from mail-lj1-x233.google.com (mail-lj1-x233.google.com\n\t[IPv6:2a00:1450:4864:20::233])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 72FA0603E7\n\tfor ;\n\tTue, 2 Aug 2022 12:30:31 +0200 (CEST)","by mail-lj1-x233.google.com with SMTP id w15so6034656ljw.1\n\tfor ;\n\tTue, 02 Aug 2022 03:30:31 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659436233;\n\tbh=jeinm5lNAO32Zkv4gRkXxBzfjOOpC06mAqCCjXu3wL4=;\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=zzaZ77Cl/7uRZrk3ZzX2IpQrDFKPIQl4DtevYCM1BwSpLEix6UOBHgFCAODAJUuZE\n\thvgzmCAVDm5yEeSyE9hN8W5l7RYVr5X16/NVRveIUh+/NTzTCIKOlE7VMAstmpLwYV\n\tmBABUvBPyroPq7nmfsdT4FAPsrfGX19QtStRjSQVvo/QaejWh6I81VN3ikWnK2yOZW\n\tiiD0iYwFEYA0IYFaFfym26vJRv1IR3+nNthHcais7BdJ0XP9sfp4OTw+B4aGUGlMC4\n\t/yyS0yZjkSKDNxBXTgdnQ5w5Tp5fETtN+Wf0UnLeYA8/n58tCgWtbeQ/8aP6o2v58L\n\tLQuOjdwRkON7A==","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=mfqVOfiWo2k9/eJkV8cVrla19tMmvbo4qi1IyyR0Ymk=;\n\tb=dVDjuQB8HgMw31d3RbnNhVeYlZKZ1n9nejKRUw5EANCCG+0p1gGw7Zsv2MXw6dMOd4\n\tQA9fjC0griUT7VLvUOhDxi1qSYCUXF5WyJHOoezJfdD81oM+mEHOdF1XXjWiLZHg8/rN\n\t1yFyy9U8fsnxIVs20sd4g2cuNCCKSgs5T3VkE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"dVDjuQB8\"; \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=mfqVOfiWo2k9/eJkV8cVrla19tMmvbo4qi1IyyR0Ymk=;\n\tb=F6qcy8yGpu3Ek7xaTuYJsoL/qsN/5CYVpr0N2OklO84fwQI8Ju608AV1uuDQEdgAHl\n\t+YiJd4tyKRAIzm09OQqGQZsJddtMEQcMOk808hjmG+KGD6P3gTMZCDz0AS9NfCggQA8i\n\tHF7cUbSaQDOB+mYaIQRMg48PqCIPSqhcHVT97Jv4cJcm6DBnnlrFLDWrSiOG4I3L6dQN\n\tKnvRf9Urtg1fHsf+1iOKYHJ/MdDQj8pB90WZYEXKm72e9otMa/VeogV1TeN53LMZvBmU\n\tK9TWrakRXsFTqiE4UT7o596zGiV+EoTjmMb3xS4Nr+au0SS9+Qg0j8/2WtuDrTzkwlLt\n\t2O3A==","X-Gm-Message-State":"ACgBeo328ce0e9LQSpoinRHkamjcOffbKiAEdDIGGu5BvGI8A20+kYkx\n\t2pWQoGeQ5cztSP1p7UyyigIHRxskxc57DEu0H5T+NA==","X-Google-Smtp-Source":"AA6agR6/0A22Uaf7B8+HeVzOn96PaxB36etAepHIIV/rLkEqR88KuEAzHnbPiIUI/+KfliJb5UddTp82Z+WJDf2Vi1o=","X-Received":"by 2002:a2e:9e88:0:b0:25e:6099:8ece with SMTP id\n\tf8-20020a2e9e88000000b0025e60998ecemr363341ljk.330.1659436230818;\n\tTue, 02 Aug 2022 03:30:30 -0700 (PDT)","MIME-Version":"1.0","References":"<20220629103018.4025635-1-chenghaoyang@google.com>\n\t<20220629103018.4025635-3-chenghaoyang@google.com>\n\t<841f182f-1c0c-e060-833b-f61296a52acf@ideasonboard.com>\n\t\n\t<7d6ce545-32a8-9152-c681-bc00f2e2d810@ideasonboard.com>","In-Reply-To":"<7d6ce545-32a8-9152-c681-bc00f2e2d810@ideasonboard.com>","Date":"Tue, 2 Aug 2022 18:30:19 +0800","Message-ID":"","To":"Umang Jain ","Content-Type":"multipart/alternative; boundary=\"00000000000010cfbd05e53f9bc0\"","Subject":"Re: [libcamera-devel] [PATCH v3 2/9] ipu3: Allow only one camera\n\tbeing started","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\" "}}]