[{"id":26962,"web_url":"https://patchwork.libcamera.org/comment/26962/","msgid":"","date":"2023-04-27T11:36:21","subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Validate transform","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Robert,\n\nOn Thu, Apr 27, 2023 at 12:53:56PM +0200, Robert Mader via libcamera-devel wrote:\n> Just like we do for other pipeline handlers already.\n> This ensures we corretly pass on transforms that are not handled by the\n> sensor - e.g. rotations - back to the app via the config, which is\n> required on devices like the Pinephone.\n>\n> Signed-off-by: Robert Mader \n> Tested-by: Arnav Singh \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 21 ++++++++++++++-------\n> 1 file changed, 14 insertions(+), 7 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 2423ec10..abfb4c87 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -211,7 +211,8 @@ public:\n> \tint init();\n> \tint setupLinks();\n> \tint setupFormats(V4L2SubdeviceFormat *format,\n> -\t\t\t V4L2Subdevice::Whence whence);\n> +\t\t\t V4L2Subdevice::Whence whence,\n> +\t\t\t Transform transform = Transform::Identity);\n> \tvoid bufferReady(FrameBuffer *buffer);\n>\n> \tunsigned int streamIndex(const Stream *stream) const\n> @@ -292,6 +293,7 @@ public:\n> \t}\n>\n> \tbool needConversion() const { return needConversion_; }\n> +\tconst Transform &combinedTransform() { return combinedTransform_; }\n\n\tconst Transform &combinedTransform() const { return combinedTransform_; }\n\n>\n> private:\n> \t/*\n> @@ -304,6 +306,7 @@ private:\n>\n> \tconst SimpleCameraData::Configuration *pipeConfig_;\n> \tbool needConversion_;\n> +\tTransform combinedTransform_;\n> };\n>\n> class SimplePipelineHandler : public PipelineHandler\n> @@ -664,7 +667,8 @@ int SimpleCameraData::setupLinks()\n> }\n>\n> int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,\n> -\t\t\t\t V4L2Subdevice::Whence whence)\n> +\t\t\t\t V4L2Subdevice::Whence whence,\n> +\t\t\t\t Transform transform)\n> {\n> \tSimplePipelineHandler *pipe = SimpleCameraData::pipe();\n> \tint ret;\n> @@ -673,7 +677,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,\n> \t * Configure the format on the sensor output and propagate it through\n> \t * the pipeline.\n> \t */\n> -\tret = sensor_->setFormat(format);\n> +\tret = sensor_->setFormat(format, transform);\n> \tif (ret < 0)\n> \t\treturn ret;\n>\n> @@ -877,15 +881,16 @@ SimpleCameraConfiguration::SimpleCameraConfiguration(Camera *camera,\n>\n> CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> {\n> +\tconst CameraSensor *sensor = data_->sensor_.get();\n> \tStatus status = Valid;\n>\n> \tif (config_.empty())\n> \t\treturn Invalid;\n>\n> -\tif (transform != Transform::Identity) {\n> -\t\ttransform = Transform::Identity;\n> +\tTransform requestedTransform = transform;\n> +\tcombinedTransform_ = sensor->validateTransform(&transform);\n> +\tif (transform != requestedTransform)\n> \t\tstatus = Adjusted;\n> -\t}\n>\n> \t/* Cap the number of entries to the available streams. */\n> \tif (config_.size() > data_->streams_.size()) {\n> @@ -1116,7 +1121,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \tformat.mbus_code = pipeConfig->code;\n> \tformat.size = pipeConfig->sensorSize;\n>\n> -\tret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);\n> +\tret = data->setupFormats(&format,\n> +\t\t\t\t V4L2Subdevice::ActiveFormat,\n> +\t\t\t\t config->combinedTransform());\n\nProbably fits in 2 lines only\n\nMinor nits, can be fixed when applying\nReviewed-by: Jacopo Mondi \n\n> \tif (ret < 0)\n> \t\treturn ret;\n>\n> --\n> 2.40.0\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 54506BDCBD\n\tfor ;\n\tThu, 27 Apr 2023 11:36:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C985D627DF;\n\tThu, 27 Apr 2023 13:36:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 124F4627B7\n\tfor ;\n\tThu, 27 Apr 2023 13:36:25 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:1cf0:b3bc:c785:4625])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D280F9DE;\n\tThu, 27 Apr 2023 13:36:12 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1682595385;\n\tbh=E+jgVMZCPDObgs1usdzuSkXL4w/h8osqaDe+rkaOhCk=;\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=v8ge+bolAyTdCSWfvdGtMMjw173Mpc+DuPVOqwVznzE5jlFvCijHeH5A+aCy1jHqP\n\tmpooGWpbKNAmxAS8nz5Y0et1dczC9ixmULYDCO7KK4rqcuNcPeMBB6fvZEq6O9J7jI\n\t1Gggsh0rJqM/XokceVdN/OEinWvAdWuJH6LDsKaQdCLLhWdnrJBcmCQBZrqqBWKELW\n\tDIVvXKV9NeAU/U7jSXl8VtAlyPkt+zJdJvYzDD1VYuFKxukCuUSiT5T29Hd/m6tyVw\n\t1QMZyJlyyPDZjxWURi6XtzeYsE7qat95w0gLJitWK2cEFpVmK43k4IVkO/x+xTCXWK\n\tgIOJXjuKjbd5A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1682595373;\n\tbh=E+jgVMZCPDObgs1usdzuSkXL4w/h8osqaDe+rkaOhCk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NoymuFICA7xXMRaL85m5WkHII7oEMUmi5rhdTr/xvV+NUFQ+Hsf/9ZzQzc+WAROxq\n\tzXSqU98/3KlYfLM+GxgmN5NbyVrwim8DFPcaa+fmv/zrvwp/77+pAu6ODYo5TNJwQp\n\tQarKU65aHGaYtz88ayO6pSSWF8fNsLgcpomWQF/8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"NoymuFIC\"; dkim-atps=neutral","Date":"Thu, 27 Apr 2023 13:36:21 +0200","To":"Robert Mader ","Message-ID":"","References":"<20230427105356.16869-1-robert.mader@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230427105356.16869-1-robert.mader@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Validate transform","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, Arnav Singh ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27000,"web_url":"https://patchwork.libcamera.org/comment/27000/","msgid":"<168280298356.3875414.3377428591427997803@Monstersaurus>","date":"2023-04-29T21:16:23","subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Validate transform","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi via libcamera-devel (2023-04-27 12:36:21)\n> Hi Robert,\n> \n> On Thu, Apr 27, 2023 at 12:53:56PM +0200, Robert Mader via libcamera-devel wrote:\n> > Just like we do for other pipeline handlers already.\n> > This ensures we corretly pass on transforms that are not handled by the\n> > sensor - e.g. rotations - back to the app via the config, which is\n> > required on devices like the Pinephone.\n> >\n> > Signed-off-by: Robert Mader \n> > Tested-by: Arnav Singh \n> > ---\n> > src/libcamera/pipeline/simple/simple.cpp | 21 ++++++++++++++-------\n> > 1 file changed, 14 insertions(+), 7 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 2423ec10..abfb4c87 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -211,7 +211,8 @@ public:\n> > int init();\n> > int setupLinks();\n> > int setupFormats(V4L2SubdeviceFormat *format,\n> > - V4L2Subdevice::Whence whence);\n> > + V4L2Subdevice::Whence whence,\n> > + Transform transform = Transform::Identity);\n> > void bufferReady(FrameBuffer *buffer);\n> >\n> > unsigned int streamIndex(const Stream *stream) const\n> > @@ -292,6 +293,7 @@ public:\n> > }\n> >\n> > bool needConversion() const { return needConversion_; }\n> > + const Transform &combinedTransform() { return combinedTransform_; }\n> \n> const Transform &combinedTransform() const { return combinedTransform_; }\n> \n> >\n> > private:\n> > /*\n> > @@ -304,6 +306,7 @@ private:\n> >\n> > const SimpleCameraData::Configuration *pipeConfig_;\n> > bool needConversion_;\n> > + Transform combinedTransform_;\n> > };\n> >\n> > class SimplePipelineHandler : public PipelineHandler\n> > @@ -664,7 +667,8 @@ int SimpleCameraData::setupLinks()\n> > }\n> >\n> > int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,\n> > - V4L2Subdevice::Whence whence)\n> > + V4L2Subdevice::Whence whence,\n> > + Transform transform)\n> > {\n> > SimplePipelineHandler *pipe = SimpleCameraData::pipe();\n> > int ret;\n> > @@ -673,7 +677,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,\n> > * Configure the format on the sensor output and propagate it through\n> > * the pipeline.\n> > */\n> > - ret = sensor_->setFormat(format);\n> > + ret = sensor_->setFormat(format, transform);\n> > if (ret < 0)\n> > return ret;\n> >\n> > @@ -877,15 +881,16 @@ SimpleCameraConfiguration::SimpleCameraConfiguration(Camera *camera,\n> >\n> > CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> > {\n> > + const CameraSensor *sensor = data_->sensor_.get();\n> > Status status = Valid;\n> >\n> > if (config_.empty())\n> > return Invalid;\n> >\n> > - if (transform != Transform::Identity) {\n> > - transform = Transform::Identity;\n> > + Transform requestedTransform = transform;\n> > + combinedTransform_ = sensor->validateTransform(&transform);\n> > + if (transform != requestedTransform)\n> > status = Adjusted;\n> > - }\n> >\n> > /* Cap the number of entries to the available streams. */\n> > if (config_.size() > data_->streams_.size()) {\n> > @@ -1116,7 +1121,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > format.mbus_code = pipeConfig->code;\n> > format.size = pipeConfig->sensorSize;\n> >\n> > - ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);\n> > + ret = data->setupFormats(&format,\n> > + V4L2Subdevice::ActiveFormat,\n> > + config->combinedTransform());\n> \n> Probably fits in 2 lines only\n> \n> Minor nits, can be fixed when applying\n> Reviewed-by: Jacopo Mondi \n\n\nReviewed-by: Kieran Bingham \n\nMinors addressed and applied.\n--\nKieran\n\n> \n> > if (ret < 0)\n> > return ret;\n> >\n> > --\n> > 2.40.0\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 48F5FC0DA4\n\tfor ;\n\tSat, 29 Apr 2023 21:16:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A9EFB627DF;\n\tSat, 29 Apr 2023 23:16:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1BE1B627D1\n\tfor ;\n\tSat, 29 Apr 2023 23:16:28 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CFB257FC;\n\tSat, 29 Apr 2023 23:16:13 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1682802989;\n\tbh=wkqbcs8h+CACZ294p9eqUhVQ7WW2xt3g56DT5brPnBQ=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=FDqDSRGkQdbvgpXudgzelaq6JSJdBCvtnEOX/oE2GNFQ24xsOzoZUrdUtqr3rLgfL\n\t1BAOTKjE+gZ3kcnjr0y5QUjAUnkHuv8hYOc3cwQJ0hX83qTdReF6CLnciurn1fcQR4\n\tdriW7Q8plQl1z0ZMksvncv8X0CPGEPgZ9ULaxLBfYtn7VMfeg2HImD/4Qwpmngse/d\n\tOH7lYlp4+1I8/gNyAAiUH4c3RFv6yBeEHU1GQ39sKCjW3hff6UN5mGyhMe1MmNLtOb\n\twqByQ4GDVnnCttt4Woc3P688dKJjHeizCCjmjQWeVnt/fnqfTLIpkvEX32ROSiuSwe\n\tuvch18YEGVRBw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1682802974;\n\tbh=wkqbcs8h+CACZ294p9eqUhVQ7WW2xt3g56DT5brPnBQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=sBXxqG5IGywCPSPC6P1PA8aokc+UbFePHumImsnStZYqIW7OGSv+UUrVWdsQ+Oqgp\n\tSenxqFiYSg2ggunx0OsQPi+VYn/GyY8y2PwM8zGAEjSTEHnvaQJxdXqsnv2zZsMJPM\n\tmDAgfhCu5iBppf1xV/K2B3zHoGmF6POz7VX/rPmY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"sBXxqG5I\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20230427105356.16869-1-robert.mader@collabora.com>\n\t","To":"Jacopo Mondi ,\n\tJacopo Mondi via libcamera-devel , \n\tRobert Mader ","Date":"Sat, 29 Apr 2023 22:16:23 +0100","Message-ID":"<168280298356.3875414.3377428591427997803@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Validate transform","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, Arnav Singh ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]