[{"id":31822,"web_url":"https://patchwork.libcamera.org/comment/31822/","msgid":"<172937805738.2485972.55745232204701329@ping.linuxembedded.co.uk>","date":"2024-10-19T22:47:37","subject":"Re: [PATCH 4/4] libcamera: Make Camera::Private::isAcquired()\n\tprotected","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Harvey Yang (2024-10-18 08:57:37)\n> As some pipeline handlers need to know if a CameraData is currently\n> acquired, this patch makes the function protected, instead of private.\n> \n> For example, the upcoming mtkisp7 needs the information to determine if\n> it should send camera disconnect signal.\n\nWhy? If a camera is disconnected - it should always send a disconnect\nsignal to any listener.\n\nIt means the application must no longer even remember the camera\nexisted!\n\n--\nKieran\n\n\n> \n> Signed-off-by: Harvey Yang \n> Co-developed-by: Yudhistira Erlandinata \n> Signed-off-by: Yudhistira Erlandinata \n> ---\n> include/libcamera/internal/camera.h | 4 +++-\n> src/libcamera/camera.cpp | 3 +++\n> 2 files changed, 6 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h\n> index 0bef0980e..fe47a49b7 100644\n> --- a/include/libcamera/internal/camera.h\n> +++ b/include/libcamera/internal/camera.h\n> @@ -43,6 +43,9 @@ public:\n> \n> const CameraControlValidator *validator() const { return validator_.get(); }\n> \n> +protected:\n> + bool isAcquired() const;\n> +\n> private:\n> enum State {\n> CameraAvailable,\n> @@ -52,7 +55,6 @@ private:\n> CameraRunning,\n> };\n> \n> - bool isAcquired() const;\n> bool isRunning() const;\n> int isAccessAllowed(State state, bool allowDisconnected = false,\n> const char *from = __builtin_FUNCTION()) const;\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index ef5a6725f..38aa4ad22 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -670,6 +670,9 @@ static const char *const camera_state_names[] = {\n> \"Running\",\n> };\n> \n> +/**\n> + * \\return True if the camera is acquired, false otherwise\n> + */\n> bool Camera::Private::isAcquired() const\n> {\n> return state_.load(std::memory_order_acquire) != CameraAvailable;\n> -- \n> 2.47.0.rc1.288.g06298d1525-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 EAE75BD1F1\n\tfor ;\n\tSat, 19 Oct 2024 22:47:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B8B576538E;\n\tSun, 20 Oct 2024 00:47:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EEC8465382\n\tfor ;\n\tSun, 20 Oct 2024 00:47:39 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 278FA502;\n\tSun, 20 Oct 2024 00:45:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"G2pIt//9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729377955;\n\tbh=ZqF7FCJpzJJDBTUvsfr8mesnqSeJemfbH0v3GPp+CPM=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=G2pIt//9BqN0MoJ8QASnbR+9UifhJ+B8O2DszeJ5RcYnEGIhbwWz0q+mPs0Dpz1PL\n\twsAC/1X0qsJdjEjn0hiYl+Xcw2gCmU9FqjUs97Ou1bgJ5O4Nnfqj91UwD5sPAV7cAL\n\tN32/cEUbEsrG9Rl7oMa2I64HMYHUABiGj2SJA5bE=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241018075942.1150378-5-chenghaoyang@chromium.org>","References":"<20241018075942.1150378-1-chenghaoyang@chromium.org>\n\t<20241018075942.1150378-5-chenghaoyang@chromium.org>","Subject":"Re: [PATCH 4/4] libcamera: Make Camera::Private::isAcquired()\n\tprotected","From":"Kieran Bingham ","Cc":"Harvey Yang ,\n\tYudhistira Erlandinata ","To":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Sat, 19 Oct 2024 23:47:37 +0100","Message-ID":"<172937805738.2485972.55745232204701329@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31823,"web_url":"https://patchwork.libcamera.org/comment/31823/","msgid":"","date":"2024-10-20T04:35:20","subject":"Re: [PATCH 4/4] libcamera: Make Camera::Private::isAcquired()\n\tprotected","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Kieran,\n\nOn Sun, Oct 20, 2024 at 6:47 AM Kieran Bingham\n wrote:\n>\n> Quoting Harvey Yang (2024-10-18 08:57:37)\n> > As some pipeline handlers need to know if a CameraData is currently\n> > acquired, this patch makes the function protected, instead of private.\n> >\n> > For example, the upcoming mtkisp7 needs the information to determine if\n> > it should send camera disconnect signal.\n>\n> Why? If a camera is disconnected - it should always send a disconnect\n> signal to any listener.\n>\n> It means the application must no longer even remember the camera\n> existed!\n\nThis is particularly needed for IPA disconnection:\nPipeline handler holds the IPA proxy, so it needs to decide if calling\na camera's\ndisconnected signal is proper.\n\nIf you think that IPA proxy disconnection is another thing, apart from\na camera's\ndisconnection, we can also consider adding another signal.\n\nBR,\nHarvey\n\n>\n> --\n> Kieran\n>\n>\n> >\n> > Signed-off-by: Harvey Yang \n> > Co-developed-by: Yudhistira Erlandinata \n> > Signed-off-by: Yudhistira Erlandinata \n> > ---\n> > include/libcamera/internal/camera.h | 4 +++-\n> > src/libcamera/camera.cpp | 3 +++\n> > 2 files changed, 6 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h\n> > index 0bef0980e..fe47a49b7 100644\n> > --- a/include/libcamera/internal/camera.h\n> > +++ b/include/libcamera/internal/camera.h\n> > @@ -43,6 +43,9 @@ public:\n> >\n> > const CameraControlValidator *validator() const { return validator_.get(); }\n> >\n> > +protected:\n> > + bool isAcquired() const;\n> > +\n> > private:\n> > enum State {\n> > CameraAvailable,\n> > @@ -52,7 +55,6 @@ private:\n> > CameraRunning,\n> > };\n> >\n> > - bool isAcquired() const;\n> > bool isRunning() const;\n> > int isAccessAllowed(State state, bool allowDisconnected = false,\n> > const char *from = __builtin_FUNCTION()) const;\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index ef5a6725f..38aa4ad22 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -670,6 +670,9 @@ static const char *const camera_state_names[] = {\n> > \"Running\",\n> > };\n> >\n> > +/**\n> > + * \\return True if the camera is acquired, false otherwise\n> > + */\n> > bool Camera::Private::isAcquired() const\n> > {\n> > return state_.load(std::memory_order_acquire) != CameraAvailable;\n> > --\n> > 2.47.0.rc1.288.g06298d1525-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 C4E7FC3304\n\tfor ;\n\tSun, 20 Oct 2024 04:35:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A0F696538C;\n\tSun, 20 Oct 2024 06:35:35 +0200 (CEST)","from mail-lj1-x232.google.com (mail-lj1-x232.google.com\n\t[IPv6:2a00:1450:4864:20::232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 307E2603ED\n\tfor ;\n\tSun, 20 Oct 2024 06:35:33 +0200 (CEST)","by mail-lj1-x232.google.com with SMTP id\n\t38308e7fff4ca-2fb57f97d75so30702701fa.2\n\tfor ;\n\tSat, 19 Oct 2024 21:35:33 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"T5nm0iiX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1729398932; x=1730003732;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=UKceLjPrmWqYB/aZEfJjZomFhjIHW28wuyfHs36sGfY=;\n\tb=T5nm0iiXUGyrcpmJRQ4qePap1lQa6hiBdiBCYFErbIAS66hNIqup6pYHvn9QzP098w\n\t1FckItDz7cQeEXs8E69ycCCOdNkjqzDNLz+Hu5bxq519TTXoZ3UVcgoIS8+1rqB3/rd5\n\t2oe2+5GPAu4tElvC9G5vNrdwAzCDgJcrS6hS4=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1729398932; x=1730003732;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=UKceLjPrmWqYB/aZEfJjZomFhjIHW28wuyfHs36sGfY=;\n\tb=Sh3bOElMkeTezyjW8aZ/nOkh2F1oQkVS9ssDnIxu8GMJ5uWbkgxtH7nYlmDtzDsgjW\n\toTb42aCqCUN8gZlM7l9PVHotL/GtYfpBUZdUDkMLUtbupjZmECkn2dMor8WzxNW8IkIB\n\tWSk2rN7EnoGgxsWx56xELY2+i6fW21Kop2pfhCdmIY526yEF6qfSSs7iKetNcI91gAcB\n\tCbUO59W10FIlL1sh0ReY6Hq5OadTPh4BOqpj7CfiK1BPqITbHd3RZGnVBHNgq37YMkRi\n\t+zhbmFssp5vUKX+IWFAZuJypgdV+z9eCdtDQyo/vFBBBsi3djtsO2CUbAl2h2fbicJXo\n\tMRyw==","X-Gm-Message-State":"AOJu0YzW89dmNi0IDiL3qRqC68eKJDsX0SQu6TAnmGzgYy/m66M2AxYZ\n\tCH2DxyZ6ghC1CCMmMU+44aAqeznnA3QnxCK0qTpUASCJK9nSkprQ7G7UjBVOUVo56gvf8gjDj2P\n\tSV4XPL4zFEP7dTg8M76O1ny+7iZpf6SyOtYd0","X-Google-Smtp-Source":"AGHT+IECaezY34UEeDchfRGDJeKjfCymBidsFwXEQxw+BwtFs5dNkDostaXQH35hWMR/IGtjfjKz8lNZ7V+KBtltNKI=","X-Received":"by 2002:a2e:a545:0:b0:2fb:6169:c42d with SMTP id\n\t38308e7fff4ca-2fb83281d34mr33629741fa.41.1729398931888;\n\tSat, 19 Oct 2024 21:35:31 -0700 (PDT)","MIME-Version":"1.0","References":"<20241018075942.1150378-1-chenghaoyang@chromium.org>\n\t<20241018075942.1150378-5-chenghaoyang@chromium.org>\n\t<172937805738.2485972.55745232204701329@ping.linuxembedded.co.uk>","In-Reply-To":"<172937805738.2485972.55745232204701329@ping.linuxembedded.co.uk>","From":"Cheng-Hao Yang ","Date":"Sun, 20 Oct 2024 12:35:20 +0800","Message-ID":"","Subject":"Re: [PATCH 4/4] libcamera: Make Camera::Private::isAcquired()\n\tprotected","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31826,"web_url":"https://patchwork.libcamera.org/comment/31826/","msgid":"<172942187980.31090.11067267546780904358@ping.linuxembedded.co.uk>","date":"2024-10-20T10:57:59","subject":"Re: [PATCH 4/4] libcamera: Make Camera::Private::isAcquired()\n\tprotected","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Cheng-Hao Yang (2024-10-20 05:35:20)\n> Hi Kieran,\n> \n> On Sun, Oct 20, 2024 at 6:47 AM Kieran Bingham\n> wrote:\n> >\n> > Quoting Harvey Yang (2024-10-18 08:57:37)\n> > > As some pipeline handlers need to know if a CameraData is currently\n> > > acquired, this patch makes the function protected, instead of private.\n> > >\n> > > For example, the upcoming mtkisp7 needs the information to determine if\n> > > it should send camera disconnect signal.\n> >\n> > Why? If a camera is disconnected - it should always send a disconnect\n> > signal to any listener.\n> >\n> > It means the application must no longer even remember the camera\n> > existed!\n> \n> This is particularly needed for IPA disconnection:\n> Pipeline handler holds the IPA proxy, so it needs to decide if calling\n> a camera's\n> disconnected signal is proper.\n\nThat's the part I didn't understand and this patch doesn't tell me.\n\n'isAcquired' isn't something I would expect to use as a gate for when to\nsend a disconnected signal or not. If the IPC is disconne\n\nIf an IPA crashes or gets disconnected because the socket is broken -\nthe camera is 'inoperable' right? So if we're using the equivalent calls\nof uvc as disconnected - then we should disconnect the whole camera -\nand trigger a camera removed? At least until we can correctly\nreconstruct a new camera ...\n\nOr are you trying to 'keep' the camera but report it to applications\nthat it can't be used ? This patch doesn't answer anything about how\nit's expected to be used in a pipeline handler.\n\nPerhaps - making sure something is generic - or adding the same\nfunctionality to the existing pipeline handlers would help demonstrate\nwhat you're trying to do?\n\nMy concern here (this patch is trivial) is that the implementation for\nhandling a disconnected IPA should be the same for /all/ pipeline\nhandlers. Not something specific to an mtkisp pipeline handler that we\nhaven't seen yet.\n\nI see PipelineHandler::disconnect() is ultimately the path that would\nthen remove the camera from the camera manager.\n\nInterestingly - that removes the whole pipeline handler, which could be\nmanaging multiple cameras - and presumably - here we are only trying to\n'remove' a single camera - the pipeline handler itself has not been\ndisconnected (and in theory something I presume could try to reload the\nIPA?)\n\n\n\n--\nKieran\n\n\n> \n> If you think that IPA proxy disconnection is another thing, apart from\n> a camera's\n> disconnection, we can also consider adding another signal.\n> \n> BR,\n> Harvey\n> \n> >\n> > --\n> > Kieran\n> >\n> >\n> > >\n> > > Signed-off-by: Harvey Yang \n> > > Co-developed-by: Yudhistira Erlandinata \n> > > Signed-off-by: Yudhistira Erlandinata \n> > > ---\n> > > include/libcamera/internal/camera.h | 4 +++-\n> > > src/libcamera/camera.cpp | 3 +++\n> > > 2 files changed, 6 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h\n> > > index 0bef0980e..fe47a49b7 100644\n> > > --- a/include/libcamera/internal/camera.h\n> > > +++ b/include/libcamera/internal/camera.h\n> > > @@ -43,6 +43,9 @@ public:\n> > >\n> > > const CameraControlValidator *validator() const { return validator_.get(); }\n> > >\n> > > +protected:\n> > > + bool isAcquired() const;\n> > > +\n> > > private:\n> > > enum State {\n> > > CameraAvailable,\n> > > @@ -52,7 +55,6 @@ private:\n> > > CameraRunning,\n> > > };\n> > >\n> > > - bool isAcquired() const;\n> > > bool isRunning() const;\n> > > int isAccessAllowed(State state, bool allowDisconnected = false,\n> > > const char *from = __builtin_FUNCTION()) const;\n> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > index ef5a6725f..38aa4ad22 100644\n> > > --- a/src/libcamera/camera.cpp\n> > > +++ b/src/libcamera/camera.cpp\n> > > @@ -670,6 +670,9 @@ static const char *const camera_state_names[] = {\n> > > \"Running\",\n> > > };\n> > >\n> > > +/**\n> > > + * \\return True if the camera is acquired, false otherwise\n> > > + */\n> > > bool Camera::Private::isAcquired() const\n> > > {\n> > > return state_.load(std::memory_order_acquire) != CameraAvailable;\n> > > --\n> > > 2.47.0.rc1.288.g06298d1525-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 9046BC3304\n\tfor ;\n\tSun, 20 Oct 2024 10:58:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 813F66538C;\n\tSun, 20 Oct 2024 12:58:04 +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 3C52765379\n\tfor ;\n\tSun, 20 Oct 2024 12:58:03 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E5236352;\n\tSun, 20 Oct 2024 12:56:17 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"vyJk/ILY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729421778;\n\tbh=cJ7yzUJXkCGBoQZWje9L2meM3OtznorDGQLJRNHFMBA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=vyJk/ILYq4HehhRs+KdxGXydGPQzhyRQPUT/0CsE7ZRX4Hn5UmkZUgUuGQ1eH4i4E\n\tWE3NfYY697I9BC07KqyjDX+y02AMP8mqHQ499+KJu/Cb8YPT3DAS8Sk9f5MS5aiKEq\n\tODHuIYsky847r/ub8vfJtjgzh/00HVoSFGcqp55I=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20241018075942.1150378-1-chenghaoyang@chromium.org>\n\t<20241018075942.1150378-5-chenghaoyang@chromium.org>\n\t<172937805738.2485972.55745232204701329@ping.linuxembedded.co.uk>\n\t","Subject":"Re: [PATCH 4/4] libcamera: Make Camera::Private::isAcquired()\n\tprotected","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tYudhistira Erlandinata ","To":"Cheng-Hao Yang ","Date":"Sun, 20 Oct 2024 11:57:59 +0100","Message-ID":"<172942187980.31090.11067267546780904358@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31844,"web_url":"https://patchwork.libcamera.org/comment/31844/","msgid":"","date":"2024-10-21T07:31:06","subject":"Re: [PATCH 4/4] libcamera: Make Camera::Private::isAcquired()\n\tprotected","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Kieran,\n\nOn Sun, Oct 20, 2024 at 6:58 PM Kieran Bingham\n wrote:\n>\n> Quoting Cheng-Hao Yang (2024-10-20 05:35:20)\n> > Hi Kieran,\n> >\n> > On Sun, Oct 20, 2024 at 6:47 AM Kieran Bingham\n> > wrote:\n> > >\n> > > Quoting Harvey Yang (2024-10-18 08:57:37)\n> > > > As some pipeline handlers need to know if a CameraData is currently\n> > > > acquired, this patch makes the function protected, instead of private.\n> > > >\n> > > > For example, the upcoming mtkisp7 needs the information to determine if\n> > > > it should send camera disconnect signal.\n> > >\n> > > Why? If a camera is disconnected - it should always send a disconnect\n> > > signal to any listener.\n> > >\n> > > It means the application must no longer even remember the camera\n> > > existed!\n> >\n> > This is particularly needed for IPA disconnection:\n> > Pipeline handler holds the IPA proxy, so it needs to decide if calling\n> > a camera's\n> > disconnected signal is proper.\n>\n> That's the part I didn't understand and this patch doesn't tell me.\n>\n> 'isAcquired' isn't something I would expect to use as a gate for when to\n> send a disconnected signal or not. If the IPC is disconne\n>\n> If an IPA crashes or gets disconnected because the socket is broken -\n> the camera is 'inoperable' right? So if we're using the equivalent calls\n> of uvc as disconnected - then we should disconnect the whole camera -\n> and trigger a camera removed? At least until we can correctly\n> reconstruct a new camera ...\n>\n> Or are you trying to 'keep' the camera but report it to applications\n> that it can't be used ? This patch doesn't answer anything about how\n> it's expected to be used in a pipeline handler.\n\nThis is a good question.\nIn CrOS, there's a test that expects to receive\nCAMERA3_MSG_ERROR_REQUEST when the IPA process crashes [1].\nIn CrOS apps, the signal itself is enough, and apps will decide to\nstop the camera afterwards, IIUC.\n\nI don't have a strong opinion if we need a mechanism in libcamera\ncore library to remove/keep the camera when the IPA disconnects.\n\nWe can also check the state of `isAcquired` in\n`Camera::Private::notifyDisconnection()`, which is added in the first\npatch.\n\n[1]: https://chromium-review.googlesource.com/c/chromiumos/third_party/libcamera/+/5467266\n\n>\n> Perhaps - making sure something is generic - or adding the same\n> functionality to the existing pipeline handlers would help demonstrate\n> what you're trying to do?\n>\n> My concern here (this patch is trivial) is that the implementation for\n> handling a disconnected IPA should be the same for /all/ pipeline\n> handlers. Not something specific to an mtkisp pipeline handler that we\n> haven't seen yet.\n\nI think this is controversial: I'm not sure if an IPA is designed to be bound\nto a camera or not. I don't think so in the current design. While in CrOS\nmtkisp7 at least, this is how it'll be used.\n\n>\n> I see PipelineHandler::disconnect() is ultimately the path that would\n> then remove the camera from the camera manager.\n>\n> Interestingly - that removes the whole pipeline handler, which could be\n> managing multiple cameras - and presumably - here we are only trying to\n> 'remove' a single camera - the pipeline handler itself has not been\n> disconnected (and in theory something I presume could try to reload the\n> IPA?)\n\nYes, that was my concern as well, so I ended up adding the first patch.\n\nBR,\nHarvey\n\n>\n>\n>\n> --\n> Kieran\n>\n>\n> >\n> > If you think that IPA proxy disconnection is another thing, apart from\n> > a camera's\n> > disconnection, we can also consider adding another signal.\n> >\n> > BR,\n> > Harvey\n> >\n> > >\n> > > --\n> > > Kieran\n> > >\n> > >\n> > > >\n> > > > Signed-off-by: Harvey Yang \n> > > > Co-developed-by: Yudhistira Erlandinata \n> > > > Signed-off-by: Yudhistira Erlandinata \n> > > > ---\n> > > > include/libcamera/internal/camera.h | 4 +++-\n> > > > src/libcamera/camera.cpp | 3 +++\n> > > > 2 files changed, 6 insertions(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h\n> > > > index 0bef0980e..fe47a49b7 100644\n> > > > --- a/include/libcamera/internal/camera.h\n> > > > +++ b/include/libcamera/internal/camera.h\n> > > > @@ -43,6 +43,9 @@ public:\n> > > >\n> > > > const CameraControlValidator *validator() const { return validator_.get(); }\n> > > >\n> > > > +protected:\n> > > > + bool isAcquired() const;\n> > > > +\n> > > > private:\n> > > > enum State {\n> > > > CameraAvailable,\n> > > > @@ -52,7 +55,6 @@ private:\n> > > > CameraRunning,\n> > > > };\n> > > >\n> > > > - bool isAcquired() const;\n> > > > bool isRunning() const;\n> > > > int isAccessAllowed(State state, bool allowDisconnected = false,\n> > > > const char *from = __builtin_FUNCTION()) const;\n> > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > > index ef5a6725f..38aa4ad22 100644\n> > > > --- a/src/libcamera/camera.cpp\n> > > > +++ b/src/libcamera/camera.cpp\n> > > > @@ -670,6 +670,9 @@ static const char *const camera_state_names[] = {\n> > > > \"Running\",\n> > > > };\n> > > >\n> > > > +/**\n> > > > + * \\return True if the camera is acquired, false otherwise\n> > > > + */\n> > > > bool Camera::Private::isAcquired() const\n> > > > {\n> > > > return state_.load(std::memory_order_acquire) != CameraAvailable;\n> > > > --\n> > > > 2.47.0.rc1.288.g06298d1525-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 D3057C32A3\n\tfor ;\n\tMon, 21 Oct 2024 07:31:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4C00E65391;\n\tMon, 21 Oct 2024 09:31:22 +0200 (CEST)","from mail-lj1-x22f.google.com (mail-lj1-x22f.google.com\n\t[IPv6:2a00:1450:4864:20::22f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EA0C76537E\n\tfor ;\n\tMon, 21 Oct 2024 09:31:19 +0200 (CEST)","by mail-lj1-x22f.google.com with SMTP id\n\t38308e7fff4ca-2fb5111747cso45237331fa.2\n\tfor ;\n\tMon, 21 Oct 2024 00:31:19 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"SA5dyxz7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1729495879; x=1730100679;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=rxdeg0cgg4ylorT6FQLUveg7Sdw6TcxNPOTbUalTRmg=;\n\tb=SA5dyxz7Sfv20QsmgXpmWrPrtbvqVRLuxZsv6tK3pVTg7LMoqAk4oGukz801gaIXjU\n\tU9fUiHpMEJdYdZcM8dwes3rL3Ix893MhtwcfTNzApRbHMD1r/pqE9rlhdn+DxJ6lVwH+\n\tTm/kFBAismVFaQZLD1c4kT8kYeYtjYaND6BZc=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1729495879; x=1730100679;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=rxdeg0cgg4ylorT6FQLUveg7Sdw6TcxNPOTbUalTRmg=;\n\tb=NdoyOEu2H9dYDmOFdWvKADozvgiMbNnfI/O2BK+IsaxqCDJgKL851UUwtU7Q/jif1i\n\tsN39v6ygOKTBkIYOq6FR1jrQQEjf3g6YTC29sviigbWld1DXeJWcwCq8iOair3S8a77/\n\tR7pQVm6swH2C1B2zQ58cfURE4eL6Wn/nPArpXy7BRQnmMQWIPPCziF9bgFVHw7n1nSuM\n\th9vDhtaUqUDIdZqzfO0afZDPJsJq5e/VRARjc4oybB1jF7ZAUN6XxJaSEYepOvxB5G9c\n\taWiUqwmigj+ycxQxfHRKRX5mPv09cZ1GuuuP0YHkDhzIH6sPcrevo7Ti+OU0Dfyw2U+a\n\t4U8Q==","X-Gm-Message-State":"AOJu0YzaU1W5Zk4rG/3GrnGZmijyF1PZ2waoh1r88CH5+U1FMhwvjLM9\n\tRVAFbmHasSYdu/gWxMczgpobnftY8I5fIQp6s69jq8uqfaAPrkhX5rVujfjicrNOVEfF3FzEDl4\n\ttk9/fDPpAx84NPkKzKNmUTD0f9SFOoTth+ndg","X-Google-Smtp-Source":"AGHT+IFQwGedc6YaXoFTsk8iw4nPlLop9GpT3J1rjLAo2Yh5RT7E+ZLgk8iR1TDHUCW1XV5E3ddfhkePSzjabQIfDFM=","X-Received":"by 2002:a2e:be9a:0:b0:2fb:8920:99c6 with SMTP id\n\t38308e7fff4ca-2fb89209a2cmr35561291fa.23.1729495878946;\n\tMon, 21 Oct 2024 00:31:18 -0700 (PDT)","MIME-Version":"1.0","References":"<20241018075942.1150378-1-chenghaoyang@chromium.org>\n\t<20241018075942.1150378-5-chenghaoyang@chromium.org>\n\t<172937805738.2485972.55745232204701329@ping.linuxembedded.co.uk>\n\t\n\t<172942187980.31090.11067267546780904358@ping.linuxembedded.co.uk>","In-Reply-To":"<172942187980.31090.11067267546780904358@ping.linuxembedded.co.uk>","From":"Cheng-Hao Yang ","Date":"Mon, 21 Oct 2024 15:31:06 +0800","Message-ID":"","Subject":"Re: [PATCH 4/4] libcamera: Make Camera::Private::isAcquired()\n\tprotected","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31845,"web_url":"https://patchwork.libcamera.org/comment/31845/","msgid":"","date":"2024-10-21T08:12:27","subject":"Re: [PATCH 4/4] libcamera: Make Camera::Private::isAcquired()\n\tprotected","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Kieran,\n\nOn Mon, Oct 21, 2024 at 3:31 PM Cheng-Hao Yang\n wrote:\n>\n> Hi Kieran,\n>\n> On Sun, Oct 20, 2024 at 6:58 PM Kieran Bingham\n> wrote:\n> >\n> > Quoting Cheng-Hao Yang (2024-10-20 05:35:20)\n> > > Hi Kieran,\n> > >\n> > > On Sun, Oct 20, 2024 at 6:47 AM Kieran Bingham\n> > > wrote:\n> > > >\n> > > > Quoting Harvey Yang (2024-10-18 08:57:37)\n> > > > > As some pipeline handlers need to know if a CameraData is currently\n> > > > > acquired, this patch makes the function protected, instead of private.\n> > > > >\n> > > > > For example, the upcoming mtkisp7 needs the information to determine if\n> > > > > it should send camera disconnect signal.\n> > > >\n> > > > Why? If a camera is disconnected - it should always send a disconnect\n> > > > signal to any listener.\n> > > >\n> > > > It means the application must no longer even remember the camera\n> > > > existed!\n> > >\n> > > This is particularly needed for IPA disconnection:\n> > > Pipeline handler holds the IPA proxy, so it needs to decide if calling\n> > > a camera's\n> > > disconnected signal is proper.\n> >\n> > That's the part I didn't understand and this patch doesn't tell me.\n> >\n> > 'isAcquired' isn't something I would expect to use as a gate for when to\n> > send a disconnected signal or not. If the IPC is disconne\n> >\n> > If an IPA crashes or gets disconnected because the socket is broken -\n> > the camera is 'inoperable' right? So if we're using the equivalent calls\n> > of uvc as disconnected - then we should disconnect the whole camera -\n> > and trigger a camera removed? At least until we can correctly\n> > reconstruct a new camera ...\n> >\n> > Or are you trying to 'keep' the camera but report it to applications\n> > that it can't be used ? This patch doesn't answer anything about how\n> > it's expected to be used in a pipeline handler.\n>\n> This is a good question.\n> In CrOS, there's a test that expects to receive\n> CAMERA3_MSG_ERROR_REQUEST when the IPA process crashes [1].\n> In CrOS apps, the signal itself is enough, and apps will decide to\n> stop the camera afterwards, IIUC.\n>\n> I don't have a strong opinion if we need a mechanism in libcamera\n> core library to remove/keep the camera when the IPA disconnects.\n\nSorry, CrOS doesn't expect the camera to be removed from the\npipeline handler (calling CameraManager::Private::removeCamera).\nCrOS expects the camera to work as expected when restarting the IPA\nprocess.\n\nI meant that we can either do nothing after the signal, or stop the\ncamera (calling stop() / release()).\n\nBR,\nHarvey\n\n>\n> We can also check the state of `isAcquired` in\n> `Camera::Private::notifyDisconnection()`, which is added in the first\n> patch.\n>\n> [1]: https://chromium-review.googlesource.com/c/chromiumos/third_party/libcamera/+/5467266\n>\n> >\n> > Perhaps - making sure something is generic - or adding the same\n> > functionality to the existing pipeline handlers would help demonstrate\n> > what you're trying to do?\n> >\n> > My concern here (this patch is trivial) is that the implementation for\n> > handling a disconnected IPA should be the same for /all/ pipeline\n> > handlers. Not something specific to an mtkisp pipeline handler that we\n> > haven't seen yet.\n>\n> I think this is controversial: I'm not sure if an IPA is designed to be bound\n> to a camera or not. I don't think so in the current design. While in CrOS\n> mtkisp7 at least, this is how it'll be used.\n>\n> >\n> > I see PipelineHandler::disconnect() is ultimately the path that would\n> > then remove the camera from the camera manager.\n> >\n> > Interestingly - that removes the whole pipeline handler, which could be\n> > managing multiple cameras - and presumably - here we are only trying to\n> > 'remove' a single camera - the pipeline handler itself has not been\n> > disconnected (and in theory something I presume could try to reload the\n> > IPA?)\n>\n> Yes, that was my concern as well, so I ended up adding the first patch.\n>\n> BR,\n> Harvey\n>\n> >\n> >\n> >\n> > --\n> > Kieran\n> >\n> >\n> > >\n> > > If you think that IPA proxy disconnection is another thing, apart from\n> > > a camera's\n> > > disconnection, we can also consider adding another signal.\n> > >\n> > > BR,\n> > > Harvey\n> > >\n> > > >\n> > > > --\n> > > > Kieran\n> > > >\n> > > >\n> > > > >\n> > > > > Signed-off-by: Harvey Yang \n> > > > > Co-developed-by: Yudhistira Erlandinata \n> > > > > Signed-off-by: Yudhistira Erlandinata \n> > > > > ---\n> > > > > include/libcamera/internal/camera.h | 4 +++-\n> > > > > src/libcamera/camera.cpp | 3 +++\n> > > > > 2 files changed, 6 insertions(+), 1 deletion(-)\n> > > > >\n> > > > > diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h\n> > > > > index 0bef0980e..fe47a49b7 100644\n> > > > > --- a/include/libcamera/internal/camera.h\n> > > > > +++ b/include/libcamera/internal/camera.h\n> > > > > @@ -43,6 +43,9 @@ public:\n> > > > >\n> > > > > const CameraControlValidator *validator() const { return validator_.get(); }\n> > > > >\n> > > > > +protected:\n> > > > > + bool isAcquired() const;\n> > > > > +\n> > > > > private:\n> > > > > enum State {\n> > > > > CameraAvailable,\n> > > > > @@ -52,7 +55,6 @@ private:\n> > > > > CameraRunning,\n> > > > > };\n> > > > >\n> > > > > - bool isAcquired() const;\n> > > > > bool isRunning() const;\n> > > > > int isAccessAllowed(State state, bool allowDisconnected = false,\n> > > > > const char *from = __builtin_FUNCTION()) const;\n> > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > > > index ef5a6725f..38aa4ad22 100644\n> > > > > --- a/src/libcamera/camera.cpp\n> > > > > +++ b/src/libcamera/camera.cpp\n> > > > > @@ -670,6 +670,9 @@ static const char *const camera_state_names[] = {\n> > > > > \"Running\",\n> > > > > };\n> > > > >\n> > > > > +/**\n> > > > > + * \\return True if the camera is acquired, false otherwise\n> > > > > + */\n> > > > > bool Camera::Private::isAcquired() const\n> > > > > {\n> > > > > return state_.load(std::memory_order_acquire) != CameraAvailable;\n> > > > > --\n> > > > > 2.47.0.rc1.288.g06298d1525-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 7B27CC3304\n\tfor ;\n\tMon, 21 Oct 2024 08:12:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7100565391;\n\tMon, 21 Oct 2024 10:12:40 +0200 (CEST)","from mail-lj1-x232.google.com (mail-lj1-x232.google.com\n\t[IPv6:2a00:1450:4864:20::232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 602566537E\n\tfor ;\n\tMon, 21 Oct 2024 10:12:39 +0200 (CEST)","by mail-lj1-x232.google.com with SMTP id\n\t38308e7fff4ca-2fb498a92f6so45625741fa.1\n\tfor ;\n\tMon, 21 Oct 2024 01:12:39 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"XzZpL6WZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1729498358; x=1730103158;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=aY6MWJFAXfoQzuufbtQq7admvGHnrAS6s5vqjLw0tM0=;\n\tb=XzZpL6WZU2QfNhIDrsSji3R1JfHWlBQ1bSY3zTttEyMwCLwoTDmkJKnwjxZ++7zxNT\n\thL+Ht7eNiM25QMkupdkizZ+FxApE8LNJ+FVmJkEzDeXXkAucEv2QCGwAAC4o4vM7X1g+\n\tnHhpeoS2/C2s6sHKmtP2/FQNUp9crdGCQjFf0=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1729498358; x=1730103158;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=aY6MWJFAXfoQzuufbtQq7admvGHnrAS6s5vqjLw0tM0=;\n\tb=jXKkbuWsA1WneA8aHD6eaKa/SrG9ahCPVDa4vmBcKl1g3k1fFQ1+7Fk+X4GqlmLcnJ\n\tJGDoQmr1hVdZSjdeYRi20UTSh09ZYikemH2BTOvbe6MItJYQnByY/8WzZ9pL5yhZnGH/\n\t22RqBolRE6pF/+36kEzexrbDZMpWZiFRRBHR4nYbZNQ9T1d4LP+ERAIgBnOLTnjjw6vf\n\tFNMk+awPC/xF8sRtSpRZ6qwLuKfYl2rRt1W14Gkq2fWDi025fioOnwK5c9Sdz7qBTs/l\n\t9A26h/zc7B6N30C7ilGKeG0scJ5vU+wKUNL74QFKzUHogkh2xKgLyv6t7h3n44URfXIO\n\t77tg==","X-Gm-Message-State":"AOJu0YzjunPEH1cZAtEQ9W9XahcMyjkuhZpJbLsUlxcvq8+mGN78JzS7\n\tR33z2TmF/p7F1HaVcbLxjObHA4jCtBOEF8Rxt1hDdj9n35IZg0oYE900HPdJmmv8WXmXNb5v6E2\n\t5h/6Y1j4HuiEU+Bq2AxFEctU4CGm4GSdKsSVL6M41Z2XOWNvIPA==","X-Google-Smtp-Source":"AGHT+IGv43WNT/to9vlttbioLuoDk9HrUgMB/eRR4GN/C+Utw0NuUfkmkANBM+qRMIdnUgtCB7z1BokDzfmXfsBFjhY=","X-Received":"by 2002:a05:651c:220d:b0:2fb:50ce:ce94 with SMTP id\n\t38308e7fff4ca-2fb82da5afamr41849391fa.0.1729498358294;\n\tMon, 21 Oct 2024 01:12:38 -0700 (PDT)","MIME-Version":"1.0","References":"<20241018075942.1150378-1-chenghaoyang@chromium.org>\n\t<20241018075942.1150378-5-chenghaoyang@chromium.org>\n\t<172937805738.2485972.55745232204701329@ping.linuxembedded.co.uk>\n\t\n\t<172942187980.31090.11067267546780904358@ping.linuxembedded.co.uk>\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Mon, 21 Oct 2024 16:12:27 +0800","Message-ID":"","Subject":"Re: [PATCH 4/4] libcamera: Make Camera::Private::isAcquired()\n\tprotected","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]