[{"id":12197,"web_url":"https://patchwork.libcamera.org/comment/12197/","msgid":"<573b5b06-6492-091f-00a1-1a393d931f83@ideasonboard.com>","date":"2020-08-28T15:01:54","subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi David,\n\nOn 28/08/2020 15:41, David Plowman wrote:\n> We set the sensor orientation (h and v flips) during validate as this\n> will in general affect the Bayer order output by the sensor. Doing it\n> here means that the correct raw format gets advertised in any raw\n> streams that the application requested.\n\nEeep - I'm not sure if we could do this in validate().\n\nValidation should not actually make any change to the hardware, but it\nshould check that the configuration can be applied correctly, and make\nany changes that would be necessary to support a correct (and 'valid')\nconfiguration to be applied through the ->configure()\n\n\n> Signed-off-by: David Plowman \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 18 +++++++++++-------\n> 1 file changed, 11 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 42c9caa..7aace71 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -400,6 +400,17 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> \tif (config_.empty())\n> \t\treturn Invalid;\n> \n> +\t/*\n> +\t * Configure the H/V flip controls based on the sensor rotation. We do\n> +\t * this here so that the sensor has the correct Bayer format that will\n> +\t * get advertised in the configuration of any raw streams.\n> +\t */\n> +\tControlList ctrls(data_->unicam_[Unicam::Image].dev()->controls());\n> +\tint32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n> +\tctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> +\tctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> +\tdata_->unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> +\n> \tunsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;\n> \tstd::pair outSize[2];\n> \tSize maxSize;\n> @@ -1164,13 +1175,6 @@ int RPiCameraData::configureIPA()\n> \t\t\t\t\t { V4L2_CID_EXPOSURE, result.data[1] } });\n> \t\t\tsensorMetadata_ = result.data[2];\n> \t\t}\n> -\n> -\t\t/* Configure the H/V flip controls based on the sensor rotation. */\n> -\t\tControlList ctrls(unicam_[Unicam::Image].dev()->controls());\n> -\t\tint32_t rotation = sensor_->properties().get(properties::Rotation);\n> -\t\tctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> -\t\tctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> -\t\tunicam_[Unicam::Image].dev()->setControls(&ctrls);\n> \t}\n> \n> \tif (result.operation & RPI_IPA_CONFIG_SENSOR) {\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 82853BF019\n\tfor ;\n\tFri, 28 Aug 2020 15:01:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0E37A628F3;\n\tFri, 28 Aug 2020 17:01:59 +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 141036037B\n\tfor ;\n\tFri, 28 Aug 2020 17:01:58 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9003A303;\n\tFri, 28 Aug 2020 17:01:57 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"oklTrFG9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598626917;\n\tbh=Zc6vO3jsi/YYUGr+NKeQOWg8XImKNPzjiYt4yOfamLw=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=oklTrFG95E2qzRHxRtWLD6B6TsMh4EQzf/B1yUqscCWO2eeD2QPGZN5e+7BoU0VDw\n\t02A9DJh2DUKkAxpZqdCMPJtOAuvR083gSPkowBGY5+NK3PzCjYIQDv8oa6gsSesM2V\n\td2F/nYK5AF5YsIjkOdWLUdi4jzPUVvU8Q/agabSQ=","To":"David Plowman ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200828144110.17303-1-david.plowman@raspberrypi.com>\n\t<20200828144110.17303-3-david.plowman@raspberrypi.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<573b5b06-6492-091f-00a1-1a393d931f83@ideasonboard.com>","Date":"Fri, 28 Aug 2020 16:01:54 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200828144110.17303-3-david.plowman@raspberrypi.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12198,"web_url":"https://patchwork.libcamera.org/comment/12198/","msgid":"","date":"2020-08-28T15:14:47","subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Kieran\n\nOn Fri, 28 Aug 2020 at 16:02, Kieran Bingham\n wrote:\n>\n> Hi David,\n>\n> On 28/08/2020 15:41, David Plowman wrote:\n> > We set the sensor orientation (h and v flips) during validate as this\n> > will in general affect the Bayer order output by the sensor. Doing it\n> > here means that the correct raw format gets advertised in any raw\n> > streams that the application requested.\n>\n> Eeep - I'm not sure if we could do this in validate().\n>\n> Validation should not actually make any change to the hardware, but it\n> should check that the configuration can be applied correctly, and make\n> any changes that would be necessary to support a correct (and 'valid')\n> configuration to be applied through the ->configure()\n>\n\nYes, that's an interesting one. The reason for doing it here is so\nthat the Bayer format comes out correctly for any raw streams that\nwere requested, and we're relying on the camera driver to give us the\ntrue Bayer order.\n\nOf course, the camera driver doesn't *have* to change the Bayer order\nwhen you transform it (it might do 1-pixel shits to maintain the\noriginal Bayer order), so the puzzle then is... how would you know?\n\nAnother solution I toyed with - and indeed implemented first - was to\ndo it in the configure() method, but then I had to dig around and find\nany raw stream configurations and update the pixelFormat post facto.\nThis involves changing stream formats after validate(), which seemed\nbad to me too... but do we prefer it?\n\nDavid\n\n>\n> > Signed-off-by: David Plowman \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 18 +++++++++++-------\n> > 1 file changed, 11 insertions(+), 7 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 42c9caa..7aace71 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -400,6 +400,17 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > if (config_.empty())\n> > return Invalid;\n> >\n> > + /*\n> > + * Configure the H/V flip controls based on the sensor rotation. We do\n> > + * this here so that the sensor has the correct Bayer format that will\n> > + * get advertised in the configuration of any raw streams.\n> > + */\n> > + ControlList ctrls(data_->unicam_[Unicam::Image].dev()->controls());\n> > + int32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n> > + ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> > + ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> > + data_->unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> > +\n> > unsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;\n> > std::pair outSize[2];\n> > Size maxSize;\n> > @@ -1164,13 +1175,6 @@ int RPiCameraData::configureIPA()\n> > { V4L2_CID_EXPOSURE, result.data[1] } });\n> > sensorMetadata_ = result.data[2];\n> > }\n> > -\n> > - /* Configure the H/V flip controls based on the sensor rotation. */\n> > - ControlList ctrls(unicam_[Unicam::Image].dev()->controls());\n> > - int32_t rotation = sensor_->properties().get(properties::Rotation);\n> > - ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> > - ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> > - unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> > }\n> >\n> > if (result.operation & RPI_IPA_CONFIG_SENSOR) {\n> >\n>\n> --\n> Regards\n> --\n> Kieran","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 BBBBABDC71\n\tfor ;\n\tFri, 28 Aug 2020 15:15:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4E6456037B;\n\tFri, 28 Aug 2020 17:15:00 +0200 (CEST)","from mail-ot1-x341.google.com (mail-ot1-x341.google.com\n\t[IPv6:2607:f8b0:4864:20::341])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 707E86037B\n\tfor ;\n\tFri, 28 Aug 2020 17:14:59 +0200 (CEST)","by mail-ot1-x341.google.com with SMTP id k20so1101117otr.1\n\tfor ;\n\tFri, 28 Aug 2020 08:14:59 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"XJ7+JlE7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=BvHHmV/NWp9uJmjisKlZxVvaKmcozW3A9UXU7Xcf7HA=;\n\tb=XJ7+JlE77mSBNQQFSLzmOtl3q3HgxHrDDRizy36xV1HK6jzrZXb5yL1sxKOOT//kwb\n\tyrHt3651WtSA0s1gnyz26L9kVaybt5N+YZ3tgvP5uVtMF1fio88rShwioslSIX/ZKtA7\n\tj3qod+LTMyOfoaV3mTb1E7xeFiwxmV0hM7qPQvIxPHg1ofs8mlPm/+pNu47Syj07l9K4\n\t9Q/kUU6GwLbtMCjId1r1jr+0mRsHNFr4gZDLauNnh7zea3JK41o2WOivHrtD1vo/d6wH\n\tL0s6bAz6O8VbOPT/NJZKR8zjb5q0d+E/j2X0WKN0ULMx59hn5zlv8AvRS968XGuvjjz6\n\t2lyA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=BvHHmV/NWp9uJmjisKlZxVvaKmcozW3A9UXU7Xcf7HA=;\n\tb=cpEaXOm0w3lUIwAzBBvru1IZ/ZUNIqaEg2+SeZ3qQqG1vs4PMDVuQHWrWEJmRU1aiT\n\th2AsPTThg+tRfJtiWvTO33x8mGStTLNqVOOJOS7xtUOSNXhSOErspPVwD/RruA9PZkvh\n\ttA/9f/LLKj2gT/wl5AF1vTQuM1xVaMyFH41ZoqDoyc0g17ehfMxjmXZHD0HbQ5yJGblH\n\tbk7dqo374ygBNaOrHd4I/Ys8ddyXXoAPV6OJUHIrGDZgQzgFOhMoIM2+KYWJYENIOHL9\n\tGoajrgdnnjoA1AB5/vvPoeiZhqgpSmcFHWnzkB5Akj/KoHO5rPd4UYNPuzsvHiz8X+01\n\tFR/g==","X-Gm-Message-State":"AOAM5306Zm/BPKsCoh/QToV//LLvkv69lRaeY9VRF843IMfBf27zBSVk\n\t1905PrYzU90UtNSGkDF5K+m0eRAfohPVfl6vBhIUvepeY7I=","X-Google-Smtp-Source":"ABdhPJwwBQ2zlSH28Ws2TBgdxqvM5cEcSUPlAOT0/S8cSN9/xsgO88O9KBenqeMsbvtA6VBM59EhhcNHfIG9famj0T8=","X-Received":"by 2002:a9d:6e8c:: with SMTP id\n\ta12mr1460252otr.166.1598627697473; \n\tFri, 28 Aug 2020 08:14:57 -0700 (PDT)","MIME-Version":"1.0","References":"<20200828144110.17303-1-david.plowman@raspberrypi.com>\n\t<20200828144110.17303-3-david.plowman@raspberrypi.com>\n\t<573b5b06-6492-091f-00a1-1a393d931f83@ideasonboard.com>","In-Reply-To":"<573b5b06-6492-091f-00a1-1a393d931f83@ideasonboard.com>","From":"David Plowman ","Date":"Fri, 28 Aug 2020 16:14:47 +0100","Message-ID":"","To":"Kieran Bingham ","Subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12199,"web_url":"https://patchwork.libcamera.org/comment/12199/","msgid":"","date":"2020-08-28T15:15:54","subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Oops.\n\nOn Fri, 28 Aug 2020 at 16:14, David Plowman\n wrote:\n>\n> Hi Kieran\n>\n> On Fri, 28 Aug 2020 at 16:02, Kieran Bingham\n> wrote:\n> >\n> > Hi David,\n> >\n> > On 28/08/2020 15:41, David Plowman wrote:\n> > > We set the sensor orientation (h and v flips) during validate as this\n> > > will in general affect the Bayer order output by the sensor. Doing it\n> > > here means that the correct raw format gets advertised in any raw\n> > > streams that the application requested.\n> >\n> > Eeep - I'm not sure if we could do this in validate().\n> >\n> > Validation should not actually make any change to the hardware, but it\n> > should check that the configuration can be applied correctly, and make\n> > any changes that would be necessary to support a correct (and 'valid')\n> > configuration to be applied through the ->configure()\n> >\n>\n> Yes, that's an interesting one. The reason for doing it here is so\n> that the Bayer format comes out correctly for any raw streams that\n> were requested, and we're relying on the camera driver to give us the\n> true Bayer order.\n>\n> Of course, the camera driver doesn't *have* to change the Bayer order\n> when you transform it (it might do 1-pixel shits to maintain the\n\nshifts. oh dear... :(\n\n> original Bayer order), so the puzzle then is... how would you know?\n>\n> Another solution I toyed with - and indeed implemented first - was to\n> do it in the configure() method, but then I had to dig around and find\n> any raw stream configurations and update the pixelFormat post facto.\n> This involves changing stream formats after validate(), which seemed\n> bad to me too... but do we prefer it?\n>\n> David\n>\n> >\n> > > Signed-off-by: David Plowman \n> > > ---\n> > > .../pipeline/raspberrypi/raspberrypi.cpp | 18 +++++++++++-------\n> > > 1 file changed, 11 insertions(+), 7 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 42c9caa..7aace71 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -400,6 +400,17 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > > if (config_.empty())\n> > > return Invalid;\n> > >\n> > > + /*\n> > > + * Configure the H/V flip controls based on the sensor rotation. We do\n> > > + * this here so that the sensor has the correct Bayer format that will\n> > > + * get advertised in the configuration of any raw streams.\n> > > + */\n> > > + ControlList ctrls(data_->unicam_[Unicam::Image].dev()->controls());\n> > > + int32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n> > > + ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> > > + ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> > > + data_->unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> > > +\n> > > unsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;\n> > > std::pair outSize[2];\n> > > Size maxSize;\n> > > @@ -1164,13 +1175,6 @@ int RPiCameraData::configureIPA()\n> > > { V4L2_CID_EXPOSURE, result.data[1] } });\n> > > sensorMetadata_ = result.data[2];\n> > > }\n> > > -\n> > > - /* Configure the H/V flip controls based on the sensor rotation. */\n> > > - ControlList ctrls(unicam_[Unicam::Image].dev()->controls());\n> > > - int32_t rotation = sensor_->properties().get(properties::Rotation);\n> > > - ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> > > - ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> > > - unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> > > }\n> > >\n> > > if (result.operation & RPI_IPA_CONFIG_SENSOR) {\n> > >\n> >\n> > --\n> > Regards\n> > --\n> > Kieran","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 E8BF2BDC71\n\tfor ;\n\tFri, 28 Aug 2020 15:16:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B7068628F3;\n\tFri, 28 Aug 2020 17:16:07 +0200 (CEST)","from mail-ot1-x343.google.com (mail-ot1-x343.google.com\n\t[IPv6:2607:f8b0:4864:20::343])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4E59F6037B\n\tfor ;\n\tFri, 28 Aug 2020 17:16:06 +0200 (CEST)","by mail-ot1-x343.google.com with SMTP id t7so1112253otp.0\n\tfor ;\n\tFri, 28 Aug 2020 08:16:06 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"Qw8G3oby\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=k6XMk/TuzLGZRxJbj/PnKr/ZINpeBU1jexc3mgeJzUY=;\n\tb=Qw8G3obyyXaSj/BcODS5mKjIcxJpNxB5TfY02e4t5vUAbFdXRxHj94CPvb4v28yofH\n\t1N3O/yJT0eUMXmdNzU9RZEFyf7A0b6k9er/KUhMnqJs0fe07/jw6AGMTNYUyu8UxedFo\n\tx2XIkQmnEDJSYx5j2k9awUk2SDi8XFELEhbWeSVjxJNSr2e1D0u/nNnsC4+Z2xhS6hMb\n\t/I6Gtdk8QXgfE7PcRXvHo0uHH4YWShyb4YrfPdee75PEIXEbMDAXNcXmjZ8UE5Ic3rZV\n\tCTXvqxG8Y4n4wOGCDw0W7+oJNv7RlSndjfxy5HMBNiutFYRXYefWrMqwPCUs0AojtpvW\n\t0MkQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=k6XMk/TuzLGZRxJbj/PnKr/ZINpeBU1jexc3mgeJzUY=;\n\tb=MNyJJZJC2h57n/5wR+JGN+7dq2JyBU2ZNXP3bSJRecJ5kOXicYf44cdnnAbpB/PDF7\n\t8jCfTx6lv1HvT/6G0Jun3T1+5kDV4ZNRQiqPtQC2PxR+TG6S+sAFxYaXHWLNnKkEXTw+\n\t5Ob9zVVwltlYbr93pQRRk0FZNolW4mbByA5ms4hKoAmtSDHQ3VSj//sDXiH+jX0/X9WR\n\tzg4G4WL79JUrcgLH9kWTYRSZnmoBDkTZqTilrw+4RYCWdPm9UYzHzVIkxqtzFbZD3JSR\n\tcGfbiIKxqn1dDBr1O16vjqdQkMhzmRqtz0DLRtjo/UNDTXhrfjfuUWfuWtbjbLhhOOpl\n\tYQUg==","X-Gm-Message-State":"AOAM532pKci0l3W16wf20f2+sINbi3qLlpmOGl1clAoyMRc2Hrg7rszH\n\t/4fxkDsCWGhVESqy0xHh6ok0u/zTFBdWxcC/Zr3YZQ==","X-Google-Smtp-Source":"ABdhPJwejj4JHlivaPne2mev2QJ93kCSgszDnupLE5ln+B1Xr5O5jPOXpJwXD+uu6odZ/ZwYfsHjhNe3EDq/62x8s3Q=","X-Received":"by 2002:a9d:923:: with SMTP id 32mr1533532otp.317.1598627765091; \n\tFri, 28 Aug 2020 08:16:05 -0700 (PDT)","MIME-Version":"1.0","References":"<20200828144110.17303-1-david.plowman@raspberrypi.com>\n\t<20200828144110.17303-3-david.plowman@raspberrypi.com>\n\t<573b5b06-6492-091f-00a1-1a393d931f83@ideasonboard.com>\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Fri, 28 Aug 2020 16:15:54 +0100","Message-ID":"","To":"Kieran Bingham ","Subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12201,"web_url":"https://patchwork.libcamera.org/comment/12201/","msgid":"<29854291-97f3-3f67-e7a4-d21029cc2963@ideasonboard.com>","date":"2020-08-28T15:26:05","subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi David,\n\nOn 28/08/2020 16:14, David Plowman wrote:\n> Hi Kieran\n> \n> On Fri, 28 Aug 2020 at 16:02, Kieran Bingham\n> wrote:\n>>\n>> Hi David,\n>>\n>> On 28/08/2020 15:41, David Plowman wrote:\n>>> We set the sensor orientation (h and v flips) during validate as this\n>>> will in general affect the Bayer order output by the sensor. Doing it\n>>> here means that the correct raw format gets advertised in any raw\n>>> streams that the application requested.\n>>\n>> Eeep - I'm not sure if we could do this in validate().\n>>\n>> Validation should not actually make any change to the hardware, but it\n>> should check that the configuration can be applied correctly, and make\n>> any changes that would be necessary to support a correct (and 'valid')\n>> configuration to be applied through the ->configure()\n>>\n> \n> Yes, that's an interesting one. The reason for doing it here is so\n> that the Bayer format comes out correctly for any raw streams that\n> were requested, and we're relying on the camera driver to give us the\n> true Bayer order.\n> \n> Of course, the camera driver doesn't *have* to change the Bayer order\n> when you transform it (it might do 1-pixel shits to maintain the\n\nhaha, I hadn't noticed the typo until your follow up post ;-)\n\n\n> original Bayer order), so the puzzle then is... how would you know?\n> \n> Another solution I toyed with - and indeed implemented first - was to\n> do it in the configure() method, but then I had to dig around and find\n> any raw stream configurations and update the pixelFormat post facto.\n> This involves changing stream formats after validate(), which seemed\n> bad to me too... but do we prefer it?\n\nAyeee - no, we can't change a format after validate either.\nOh dear have we deadlocked...\n\n\nSo - is the issue that the rotation/transform affects the pixel format?\n\nAre we intending to support those transforms on a per-frame basis? or\njust a per-stream basis.\n\nAs long as it's per-stream - Can we just store the state of the expected\nrotation in the RPiCameraConfiguration, so that we can use it at\nconfigure time appropriately? Or is it that we will need to determine\nwhat adjustment will be made to the pixelformat based on the transform.\n\nAnd if that's the case, I assume it's the kernel driver which is going\nto tell us what the new format will be. I fear we might have to\nduplicate the determination of the format up here, and report the\nadjustment.\n\nI guess what we're lacking is the ability to do a full atomic 'try' of\nthe state of the device at the driver level ...\n\n--\nKieran\n\n\n\n> \n> David\n> \n>>\n>>> Signed-off-by: David Plowman \n>>> ---\n>>> .../pipeline/raspberrypi/raspberrypi.cpp | 18 +++++++++++-------\n>>> 1 file changed, 11 insertions(+), 7 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>>> index 42c9caa..7aace71 100644\n>>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>>> @@ -400,6 +400,17 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n>>> if (config_.empty())\n>>> return Invalid;\n>>>\n>>> + /*\n>>> + * Configure the H/V flip controls based on the sensor rotation. We do\n>>> + * this here so that the sensor has the correct Bayer format that will\n>>> + * get advertised in the configuration of any raw streams.\n>>> + */\n>>> + ControlList ctrls(data_->unicam_[Unicam::Image].dev()->controls());\n>>> + int32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n>>> + ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n>>> + ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n>>> + data_->unicam_[Unicam::Image].dev()->setControls(&ctrls);\n>>> +\n>>> unsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;\n>>> std::pair outSize[2];\n>>> Size maxSize;\n>>> @@ -1164,13 +1175,6 @@ int RPiCameraData::configureIPA()\n>>> { V4L2_CID_EXPOSURE, result.data[1] } });\n>>> sensorMetadata_ = result.data[2];\n>>> }\n>>> -\n>>> - /* Configure the H/V flip controls based on the sensor rotation. */\n>>> - ControlList ctrls(unicam_[Unicam::Image].dev()->controls());\n>>> - int32_t rotation = sensor_->properties().get(properties::Rotation);\n>>> - ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n>>> - ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n>>> - unicam_[Unicam::Image].dev()->setControls(&ctrls);\n>>> }\n>>>\n>>> if (result.operation & RPI_IPA_CONFIG_SENSOR) {\n>>>\n>>\n>> --\n>> Regards\n>> --\n>> Kieran","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 C42F4BDC71\n\tfor ;\n\tFri, 28 Aug 2020 15:26:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4A6B0628F3;\n\tFri, 28 Aug 2020 17:26:10 +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 30F616037B\n\tfor ;\n\tFri, 28 Aug 2020 17:26:09 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2992F303;\n\tFri, 28 Aug 2020 17:26:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"AqETsw9o\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598628368;\n\tbh=mAZHreCApiwLJ4LlMqeX4qs/NuASxq1uR+F7vf299EA=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=AqETsw9o9jBfzpey1gZlP9XI/Ra1KZ7T5NR3ytxh6z8nu4yfHONLk+GG4HLb3wHfu\n\thuaxcFwvWoBgQFPf4TBOZlKs0h+lvkwNH96+GW6u83EOY7rHTWYdB9fM5Bjk+3sCfz\n\te/55jmEpIDTWsyrUXsDevsjJ6ITpyihEE8wDfDQQ=","To":"David Plowman ","References":"<20200828144110.17303-1-david.plowman@raspberrypi.com>\n\t<20200828144110.17303-3-david.plowman@raspberrypi.com>\n\t<573b5b06-6492-091f-00a1-1a393d931f83@ideasonboard.com>\n\t","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<29854291-97f3-3f67-e7a4-d21029cc2963@ideasonboard.com>","Date":"Fri, 28 Aug 2020 16:26:05 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12204,"web_url":"https://patchwork.libcamera.org/comment/12204/","msgid":"<20200828153537.GD10034@pendragon.ideasonboard.com>","date":"2020-08-28T15:35:37","subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Fri, Aug 28, 2020 at 04:26:05PM +0100, Kieran Bingham wrote:\n> On 28/08/2020 16:14, David Plowman wrote:\n> > On Fri, 28 Aug 2020 at 16:02, Kieran Bingham wrote:\n> >> On 28/08/2020 15:41, David Plowman wrote:\n> >>> We set the sensor orientation (h and v flips) during validate as this\n> >>> will in general affect the Bayer order output by the sensor. Doing it\n> >>> here means that the correct raw format gets advertised in any raw\n> >>> streams that the application requested.\n> >>\n> >> Eeep - I'm not sure if we could do this in validate().\n> >>\n> >> Validation should not actually make any change to the hardware, but it\n> >> should check that the configuration can be applied correctly, and make\n> >> any changes that would be necessary to support a correct (and 'valid')\n> >> configuration to be applied through the ->configure()\n> > \n> > Yes, that's an interesting one. The reason for doing it here is so\n> > that the Bayer format comes out correctly for any raw streams that\n> > were requested, and we're relying on the camera driver to give us the\n> > true Bayer order.\n> > \n> > Of course, the camera driver doesn't *have* to change the Bayer order\n> > when you transform it (it might do 1-pixel shits to maintain the\n> \n> haha, I hadn't noticed the typo until your follow up post ;-)\n> \n> > original Bayer order), so the puzzle then is... how would you know?\n> > \n> > Another solution I toyed with - and indeed implemented first - was to\n> > do it in the configure() method, but then I had to dig around and find\n> > any raw stream configurations and update the pixelFormat post facto.\n> > This involves changing stream formats after validate(), which seemed\n> > bad to me too... but do we prefer it?\n> \n> Ayeee - no, we can't change a format after validate either.\n> Oh dear have we deadlocked...\n> \n> So - is the issue that the rotation/transform affects the pixel format?\n> \n> Are we intending to support those transforms on a per-frame basis? or\n> just a per-stream basis.\n> \n> As long as it's per-stream - Can we just store the state of the expected\n> rotation in the RPiCameraConfiguration, so that we can use it at\n> configure time appropriately? Or is it that we will need to determine\n> what adjustment will be made to the pixelformat based on the transform.\n\nThe latter. We need to report to the user which raw format will be used\nbased on the transformation.\n\n> And if that's the case, I assume it's the kernel driver which is going\n> to tell us what the new format will be. I fear we might have to\n> duplicate the determination of the format up here, and report the\n> adjustment.\n> \n> I guess what we're lacking is the ability to do a full atomic 'try' of\n> the state of the device at the driver level ...\n\nCorrect, and I doubt V4L2 will ever give us that.\n\nI think we need to adjust the format in the pipeline handler, instead of\nquerying the device. We however need to know if h/v flip will affect the\nbayer pattern, or if the sensor will compensate by shifting the crop\nrectangle.\n\nOne option is to query the sensor driver at match time, to see if the\nmedia bus format changes when the h/v flip controls are modified. A bit\nof a hassle, but shouldn't be too complicated. Another option would be\nto ignore all this and consider that a proper sensor driver will never\nshift the crop rectangle behind the scene. We have the option to\nestablish a set of rules for a sensor driver to be supported by\nlibcamera.\n\n> >>> Signed-off-by: David Plowman \n> >>> ---\n> >>> .../pipeline/raspberrypi/raspberrypi.cpp | 18 +++++++++++-------\n> >>> 1 file changed, 11 insertions(+), 7 deletions(-)\n> >>>\n> >>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> index 42c9caa..7aace71 100644\n> >>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> @@ -400,6 +400,17 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> >>> if (config_.empty())\n> >>> return Invalid;\n> >>>\n> >>> + /*\n> >>> + * Configure the H/V flip controls based on the sensor rotation. We do\n> >>> + * this here so that the sensor has the correct Bayer format that will\n> >>> + * get advertised in the configuration of any raw streams.\n> >>> + */\n> >>> + ControlList ctrls(data_->unicam_[Unicam::Image].dev()->controls());\n> >>> + int32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n> >>> + ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> >>> + ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> >>> + data_->unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> >>> +\n> >>> unsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;\n> >>> std::pair outSize[2];\n> >>> Size maxSize;\n> >>> @@ -1164,13 +1175,6 @@ int RPiCameraData::configureIPA()\n> >>> { V4L2_CID_EXPOSURE, result.data[1] } });\n> >>> sensorMetadata_ = result.data[2];\n> >>> }\n> >>> -\n> >>> - /* Configure the H/V flip controls based on the sensor rotation. */\n> >>> - ControlList ctrls(unicam_[Unicam::Image].dev()->controls());\n> >>> - int32_t rotation = sensor_->properties().get(properties::Rotation);\n> >>> - ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> >>> - ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> >>> - unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> >>> }\n> >>>\n> >>> if (result.operation & RPI_IPA_CONFIG_SENSOR) {\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 E69BABDC71\n\tfor ;\n\tFri, 28 Aug 2020 15:36:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7AB986037B;\n\tFri, 28 Aug 2020 17:36:00 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 701246037B\n\tfor ;\n\tFri, 28 Aug 2020 17:35:58 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E839C303;\n\tFri, 28 Aug 2020 17:35:57 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"iAzFLuTA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598628958;\n\tbh=MzdMGIx/Bsmi+E1PdsKUygB1LUcpa3KZVLJQCP10Rjw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iAzFLuTAzmrsVTtFkf58rAKThmO10H1ALgebBBT8uxVaLnYRcaWkXndZ3EaRRulrQ\n\tZ/ysWzX0SCpcrIFG2vNTAfi0EYKfELRaf/b1rZvlxWe75jkYsYFjKrdBFa7cXQTkO7\n\tplV/ToucuJLSjs1jwzLzggjXxdyTTMX5VZbnYxeg=","Date":"Fri, 28 Aug 2020 18:35:37 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20200828153537.GD10034@pendragon.ideasonboard.com>","References":"<20200828144110.17303-1-david.plowman@raspberrypi.com>\n\t<20200828144110.17303-3-david.plowman@raspberrypi.com>\n\t<573b5b06-6492-091f-00a1-1a393d931f83@ideasonboard.com>\n\t\n\t<29854291-97f3-3f67-e7a4-d21029cc2963@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<29854291-97f3-3f67-e7a4-d21029cc2963@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12205,"web_url":"https://patchwork.libcamera.org/comment/12205/","msgid":"","date":"2020-08-28T15:36:02","subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"HI Kieran\n\nOn Fri, 28 Aug 2020 at 16:26, Kieran Bingham\n wrote:\n>\n> Hi David,\n>\n> On 28/08/2020 16:14, David Plowman wrote:\n> > Hi Kieran\n> >\n> > On Fri, 28 Aug 2020 at 16:02, Kieran Bingham\n> > wrote:\n> >>\n> >> Hi David,\n> >>\n> >> On 28/08/2020 15:41, David Plowman wrote:\n> >>> We set the sensor orientation (h and v flips) during validate as this\n> >>> will in general affect the Bayer order output by the sensor. Doing it\n> >>> here means that the correct raw format gets advertised in any raw\n> >>> streams that the application requested.\n> >>\n> >> Eeep - I'm not sure if we could do this in validate().\n> >>\n> >> Validation should not actually make any change to the hardware, but it\n> >> should check that the configuration can be applied correctly, and make\n> >> any changes that would be necessary to support a correct (and 'valid')\n> >> configuration to be applied through the ->configure()\n> >>\n> >\n> > Yes, that's an interesting one. The reason for doing it here is so\n> > that the Bayer format comes out correctly for any raw streams that\n> > were requested, and we're relying on the camera driver to give us the\n> > true Bayer order.\n> >\n> > Of course, the camera driver doesn't *have* to change the Bayer order\n> > when you transform it (it might do 1-pixel shits to maintain the\n>\n> haha, I hadn't noticed the typo until your follow up post ;-)\n>\n>\n> > original Bayer order), so the puzzle then is... how would you know?\n> >\n> > Another solution I toyed with - and indeed implemented first - was to\n> > do it in the configure() method, but then I had to dig around and find\n> > any raw stream configurations and update the pixelFormat post facto.\n> > This involves changing stream formats after validate(), which seemed\n> > bad to me too... but do we prefer it?\n>\n> Ayeee - no, we can't change a format after validate either.\n> Oh dear have we deadlocked...\n>\n>\n> So - is the issue that the rotation/transform affects the pixel format?\n\nCorrect. If you have an RGGB sensor and ask it to do h and v flips you\nend up with BGGR.\n\n>\n> Are we intending to support those transforms on a per-frame basis? or\n> just a per-stream basis.\n\nPer-stream. Actually, they're per-camera because that's where the\ntransformations are being applied. There's no possibility to change\nany of this per frame.\n\n>\n> As long as it's per-stream - Can we just store the state of the expected\n> rotation in the RPiCameraConfiguration, so that we can use it at\n> configure time appropriately? Or is it that we will need to determine\n> what adjustment will be made to the pixelformat based on the transform.\n>\n> And if that's the case, I assume it's the kernel driver which is going\n> to tell us what the new format will be. I fear we might have to\n> duplicate the determination of the format up here, and report the\n> adjustment.\n\nI think the problem with this is that we don't know what the camera\nwill do when we ask for a transform. I think all the drivers we've\never been involved with will change the Bayer order, but others might\nnot. They might fiddle with some 1-pixel wide cropping to keep the\nBayer order the same.\n\n>\n> I guess what we're lacking is the ability to do a full atomic 'try' of\n> the state of the device at the driver level ...\n\nYes, I think this is the crux of the matter.\n\nDavid\n\n>\n> --\n> Kieran\n>\n>\n>\n> >\n> > David\n> >\n> >>\n> >>> Signed-off-by: David Plowman \n> >>> ---\n> >>> .../pipeline/raspberrypi/raspberrypi.cpp | 18 +++++++++++-------\n> >>> 1 file changed, 11 insertions(+), 7 deletions(-)\n> >>>\n> >>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> index 42c9caa..7aace71 100644\n> >>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> @@ -400,6 +400,17 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> >>> if (config_.empty())\n> >>> return Invalid;\n> >>>\n> >>> + /*\n> >>> + * Configure the H/V flip controls based on the sensor rotation. We do\n> >>> + * this here so that the sensor has the correct Bayer format that will\n> >>> + * get advertised in the configuration of any raw streams.\n> >>> + */\n> >>> + ControlList ctrls(data_->unicam_[Unicam::Image].dev()->controls());\n> >>> + int32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n> >>> + ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> >>> + ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> >>> + data_->unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> >>> +\n> >>> unsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;\n> >>> std::pair outSize[2];\n> >>> Size maxSize;\n> >>> @@ -1164,13 +1175,6 @@ int RPiCameraData::configureIPA()\n> >>> { V4L2_CID_EXPOSURE, result.data[1] } });\n> >>> sensorMetadata_ = result.data[2];\n> >>> }\n> >>> -\n> >>> - /* Configure the H/V flip controls based on the sensor rotation. */\n> >>> - ControlList ctrls(unicam_[Unicam::Image].dev()->controls());\n> >>> - int32_t rotation = sensor_->properties().get(properties::Rotation);\n> >>> - ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> >>> - ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> >>> - unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> >>> }\n> >>>\n> >>> if (result.operation & RPI_IPA_CONFIG_SENSOR) {\n> >>>\n> >>\n> >> --\n> >> Regards\n> >> --\n> >> Kieran\n>\n> --\n> Regards\n> --\n> Kieran","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 15C45BDC71\n\tfor ;\n\tFri, 28 Aug 2020 15:36:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D693B628F3;\n\tFri, 28 Aug 2020 17:36:14 +0200 (CEST)","from mail-ot1-x341.google.com (mail-ot1-x341.google.com\n\t[IPv6:2607:f8b0:4864:20::341])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3DBB46037B\n\tfor ;\n\tFri, 28 Aug 2020 17:36:14 +0200 (CEST)","by mail-ot1-x341.google.com with SMTP id n23so1164104otq.11\n\tfor ;\n\tFri, 28 Aug 2020 08:36:14 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"FASoH3Ry\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=NtMVm2tUlClm5Gf8grq5qJNZZ8NfTGCQvTAVfYHZPdE=;\n\tb=FASoH3RyFvtUx2BDoiyeaCcfDucZJ5/3k3F7R3ZFnNE30k0ESrfP3xWqTtYCwcBHsI\n\tkBEcgipbaiDR5vU25dP5ShGjcFm91022gLRan4awsdoai4Vlvolqj/CFNDF6pcfQMWZh\n\tPENLxtxAr0nonC9/Y9mGLOibGIcKxicel2Wx7UXzVQN6IbUSKak4T/iqAOscFtMVnd0o\n\t/fhs7DQyyXuMU3LXDAPqSdCNYdh3vJd1n8TJRgo78rcObD9hJGFL6aWyQP/JYcokI3ko\n\tQAjKSl1Koxy71DY2WYINpRwQzig52MtdBMGcqSvrqVz0A7gyYOp9t4flSAjVqkiokxtu\n\tZWOA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=NtMVm2tUlClm5Gf8grq5qJNZZ8NfTGCQvTAVfYHZPdE=;\n\tb=cnC04HALIMiCpHDDZrdUdZpdzkA930/54zkGOXY/3b07NxLuxxTOybqXKwhzbancAt\n\tJwAwLU34yOiCevOxgEAJwk+riXkiOLCmB2Ex5iVxIT00sTkm4aSszUpN6+/IFrt9DaPF\n\tl/8Zb8wk9psFw1dOHN0LYLuYsffe9yvqaVPI+iBDYRONFEIJyVIVlzq7osnypNebpIW+\n\tAGsSgcv98Mb3/btyE1kltPcFDcTnHvOVZncLopoq6b1T0MSMAFpcAD8+GEkQBzCc5BYH\n\tKsDbR+VIj3ypXSa0tmddJ8pLVw4IRspRgG8LMy7s0mq92pv13CYYBjfNhTE6xhuSsXmU\n\tVSKA==","X-Gm-Message-State":"AOAM532qw0DGzVO+Piy56oJwPvCoVaIqgoownIJf6Ea5RJqM8K9Iht8R\n\tBd2OXWe4cLlrPv52mYzVKH/V7vBvL5tDQwWew8sVZmCKMFE=","X-Google-Smtp-Source":"ABdhPJzi6Iw6S5EoNwfcpVwjkipSFotU6F1tAr/mMg38pBztXRtguZ85y2EQ61LdzCBHgwjN3A9GtAoBUX/KJq2WOh4=","X-Received":"by 2002:a9d:923:: with SMTP id 32mr1635938otp.317.1598628972922; \n\tFri, 28 Aug 2020 08:36:12 -0700 (PDT)","MIME-Version":"1.0","References":"<20200828144110.17303-1-david.plowman@raspberrypi.com>\n\t<20200828144110.17303-3-david.plowman@raspberrypi.com>\n\t<573b5b06-6492-091f-00a1-1a393d931f83@ideasonboard.com>\n\t\n\t<29854291-97f3-3f67-e7a4-d21029cc2963@ideasonboard.com>","In-Reply-To":"<29854291-97f3-3f67-e7a4-d21029cc2963@ideasonboard.com>","From":"David Plowman ","Date":"Fri, 28 Aug 2020 16:36:02 +0100","Message-ID":"","To":"Kieran Bingham ","Subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12207,"web_url":"https://patchwork.libcamera.org/comment/12207/","msgid":"","date":"2020-08-28T15:55:14","subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","submitter":{"id":27,"url":"https://patchwork.libcamera.org/api/people/27/","name":"Dave Stevenson","email":"dave.stevenson@raspberrypi.com"},"content":"Hi Guys\n\nOn Fri, 28 Aug 2020 at 16:36, Laurent Pinchart\n wrote:\n>\n> Hello,\n>\n> On Fri, Aug 28, 2020 at 04:26:05PM +0100, Kieran Bingham wrote:\n> > On 28/08/2020 16:14, David Plowman wrote:\n> > > On Fri, 28 Aug 2020 at 16:02, Kieran Bingham wrote:\n> > >> On 28/08/2020 15:41, David Plowman wrote:\n> > >>> We set the sensor orientation (h and v flips) during validate as this\n> > >>> will in general affect the Bayer order output by the sensor. Doing it\n> > >>> here means that the correct raw format gets advertised in any raw\n> > >>> streams that the application requested.\n> > >>\n> > >> Eeep - I'm not sure if we could do this in validate().\n> > >>\n> > >> Validation should not actually make any change to the hardware, but it\n> > >> should check that the configuration can be applied correctly, and make\n> > >> any changes that would be necessary to support a correct (and 'valid')\n> > >> configuration to be applied through the ->configure()\n> > >\n> > > Yes, that's an interesting one. The reason for doing it here is so\n> > > that the Bayer format comes out correctly for any raw streams that\n> > > were requested, and we're relying on the camera driver to give us the\n> > > true Bayer order.\n> > >\n> > > Of course, the camera driver doesn't *have* to change the Bayer order\n> > > when you transform it (it might do 1-pixel shits to maintain the\n> >\n> > haha, I hadn't noticed the typo until your follow up post ;-)\n> >\n> > > original Bayer order), so the puzzle then is... how would you know?\n> > >\n> > > Another solution I toyed with - and indeed implemented first - was to\n> > > do it in the configure() method, but then I had to dig around and find\n> > > any raw stream configurations and update the pixelFormat post facto.\n> > > This involves changing stream formats after validate(), which seemed\n> > > bad to me too... but do we prefer it?\n> >\n> > Ayeee - no, we can't change a format after validate either.\n> > Oh dear have we deadlocked...\n> >\n> > So - is the issue that the rotation/transform affects the pixel format?\n> >\n> > Are we intending to support those transforms on a per-frame basis? or\n> > just a per-stream basis.\n> >\n> > As long as it's per-stream - Can we just store the state of the expected\n> > rotation in the RPiCameraConfiguration, so that we can use it at\n> > configure time appropriately? Or is it that we will need to determine\n> > what adjustment will be made to the pixelformat based on the transform.\n>\n> The latter. We need to report to the user which raw format will be used\n> based on the transformation.\n>\n> > And if that's the case, I assume it's the kernel driver which is going\n> > to tell us what the new format will be. I fear we might have to\n> > duplicate the determination of the format up here, and report the\n> > adjustment.\n> >\n> > I guess what we're lacking is the ability to do a full atomic 'try' of\n> > the state of the device at the driver level ...\n>\n> Correct, and I doubt V4L2 will ever give us that.\n>\n> I think we need to adjust the format in the pipeline handler, instead of\n> querying the device. We however need to know if h/v flip will affect the\n> bayer pattern, or if the sensor will compensate by shifting the crop\n> rectangle.\n>\n> One option is to query the sensor driver at match time, to see if the\n> media bus format changes when the h/v flip controls are modified. A bit\n> of a hassle, but shouldn't be too complicated. Another option would be\n> to ignore all this and consider that a proper sensor driver will never\n> shift the crop rectangle behind the scene. We have the option to\n> establish a set of rules for a sensor driver to be supported by\n> libcamera.\n\nYou have a very good indicator of that from the\nV4L2_CTRL_FLAG_MODIFY_LAYOUT flag when querying controls. It would be\nfair to deduce from that flag on flips that it is going to change the\nbayer order. I've yet to see a Bayer sensor that offers transpose, but\nthere may be some out there.\n\nIMX327 and IMX290 are two that don't alter the Bayer order when using\nthe on-sensor flip controls. Reading the datasheet for IMX327 (page\n58), it explicitly states for the full frame mode the active lines are\n\"Line No during normal operation\" 21 to 1100\n\"Line No during inverted operation\" 1101 to 22\nand\n\"Horizontal pixel output image normal operation\" 13 to1932\n\"Horizontal pixel output image inverted operation\" 1933 to 14.\n\nI'm also aware of one further sensor (can't say what) that I know for\ndefinite doesn't change the Bayer order. Exactly how it achieves that\none can only assume is by shifting the readout by one pixel.\n\nShould the crop reported by G_SELECTION be shifted in response to the\nflips then?\n\n Dave\n\n> > >>> Signed-off-by: David Plowman \n> > >>> ---\n> > >>> .../pipeline/raspberrypi/raspberrypi.cpp | 18 +++++++++++-------\n> > >>> 1 file changed, 11 insertions(+), 7 deletions(-)\n> > >>>\n> > >>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > >>> index 42c9caa..7aace71 100644\n> > >>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > >>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > >>> @@ -400,6 +400,17 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > >>> if (config_.empty())\n> > >>> return Invalid;\n> > >>>\n> > >>> + /*\n> > >>> + * Configure the H/V flip controls based on the sensor rotation. We do\n> > >>> + * this here so that the sensor has the correct Bayer format that will\n> > >>> + * get advertised in the configuration of any raw streams.\n> > >>> + */\n> > >>> + ControlList ctrls(data_->unicam_[Unicam::Image].dev()->controls());\n> > >>> + int32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n> > >>> + ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> > >>> + ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> > >>> + data_->unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> > >>> +\n> > >>> unsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;\n> > >>> std::pair outSize[2];\n> > >>> Size maxSize;\n> > >>> @@ -1164,13 +1175,6 @@ int RPiCameraData::configureIPA()\n> > >>> { V4L2_CID_EXPOSURE, result.data[1] } });\n> > >>> sensorMetadata_ = result.data[2];\n> > >>> }\n> > >>> -\n> > >>> - /* Configure the H/V flip controls based on the sensor rotation. */\n> > >>> - ControlList ctrls(unicam_[Unicam::Image].dev()->controls());\n> > >>> - int32_t rotation = sensor_->properties().get(properties::Rotation);\n> > >>> - ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> > >>> - ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> > >>> - unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> > >>> }\n> > >>>\n> > >>> if (result.operation & RPI_IPA_CONFIG_SENSOR) {\n> > >>>\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 7F442BDC71\n\tfor ;\n\tFri, 28 Aug 2020 15:55:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EFE88628F3;\n\tFri, 28 Aug 2020 17:55:32 +0200 (CEST)","from mail-wr1-x441.google.com (mail-wr1-x441.google.com\n\t[IPv6:2a00:1450:4864:20::441])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B556A6037B\n\tfor ;\n\tFri, 28 Aug 2020 17:55:30 +0200 (CEST)","by mail-wr1-x441.google.com with SMTP id w5so959483wrp.8\n\tfor ;\n\tFri, 28 Aug 2020 08:55:30 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"bBd112Fl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=aDGycdg4qjSP68MIipnPxXZgUcP+YtWytW1FVzmspn0=;\n\tb=bBd112FlHw+lGxEFhOLvfgZB9SZO8RxrWGQfl3KQ0roHBTp/w5ZsFefVfMZGMVvA+u\n\tC5wQjQs/ZZcumeBL9TXXKB0qpj27umuPoQ4Ju2tE3aUfmMgmHmumq1CxfGC5kGzXv+Ei\n\tA7n+9sKskxVXYBbnH/1McyWlt6H52s+1ddU+ZFiP3qXuax7Gj9m1COgYE7HDFWn+pB5W\n\t6EFxB7JIK/djPW6LoA1VEcf/Sc2SnYb9VFrFPIgNIYEmXV66srIUxn+demZx3qQqwZ7C\n\tp2Ym7/2SneBe4HYBkdHPhxKb4+caeDhlagr9CD8drt/sAe68DVEargj9c5LHD4O/+cv8\n\tEiDw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=aDGycdg4qjSP68MIipnPxXZgUcP+YtWytW1FVzmspn0=;\n\tb=K7FysdPmx0pRhijqd16qJtJPdFVx//R7Hq9OweC6Kvbo7ASURtYuC/Q/zA4A0HSPWc\n\tFErgLZvNwJDT/abLiVcYDHkoMa2m3WmqtYPEx3dgFbrzoOb214BDWeSQXSlRhC6aSFux\n\t1qSL7eGQ5LA6f5FdJwgiL7Zp8pnQ+D97lx59uVgSPeyEwTOgdt/heBVEnTWjwnl78LCb\n\t9nT/KB0i+/qMicQZlgo3ojztCKaz6y2us96Ab8Syq3nJAz/HD3oSpzKiEjwjHdjy5G0p\n\t4mEKjQ2IkmkrHFXz3IQGjv2Hd0FLqZRIK5Mz1BYK8TVz2eaDrcrJ6zdLv14uOJNntXAU\n\ts5dA==","X-Gm-Message-State":"AOAM532czC69vdNMYI7DiJsojeVJMvkDHICzXr19VA6eEtKULqzImdrM\n\tksNkDbO1o45tzrcxmUfmUf/qyPGw1IRca+aQrybcIg==","X-Google-Smtp-Source":"ABdhPJzya48WUl3zaHTU8VD6uEbF+DoxL2XEu1sm/4EzdYcbtfRqPVtVpxikVQVAmxi7ff8esz5RofcbD5CQDc9eAik=","X-Received":"by 2002:a5d:6288:: with SMTP id k8mr2061288wru.273.1598630130288;\n\tFri, 28 Aug 2020 08:55:30 -0700 (PDT)","MIME-Version":"1.0","References":"<20200828144110.17303-1-david.plowman@raspberrypi.com>\n\t<20200828144110.17303-3-david.plowman@raspberrypi.com>\n\t<573b5b06-6492-091f-00a1-1a393d931f83@ideasonboard.com>\n\t\n\t<29854291-97f3-3f67-e7a4-d21029cc2963@ideasonboard.com>\n\t<20200828153537.GD10034@pendragon.ideasonboard.com>","In-Reply-To":"<20200828153537.GD10034@pendragon.ideasonboard.com>","From":"Dave Stevenson ","Date":"Fri, 28 Aug 2020 16:55:14 +0100","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12213,"web_url":"https://patchwork.libcamera.org/comment/12213/","msgid":"<20200828160258.GH10034@pendragon.ideasonboard.com>","date":"2020-08-28T16:02:58","subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Dave,\n\nOn Fri, Aug 28, 2020 at 04:55:14PM +0100, Dave Stevenson wrote:\n> On Fri, 28 Aug 2020 at 16:36, Laurent Pinchart wrote:\n> > On Fri, Aug 28, 2020 at 04:26:05PM +0100, Kieran Bingham wrote:\n> >> On 28/08/2020 16:14, David Plowman wrote:\n> >>> On Fri, 28 Aug 2020 at 16:02, Kieran Bingham wrote:\n> >>>> On 28/08/2020 15:41, David Plowman wrote:\n> >>>>> We set the sensor orientation (h and v flips) during validate as this\n> >>>>> will in general affect the Bayer order output by the sensor. Doing it\n> >>>>> here means that the correct raw format gets advertised in any raw\n> >>>>> streams that the application requested.\n> >>>>\n> >>>> Eeep - I'm not sure if we could do this in validate().\n> >>>>\n> >>>> Validation should not actually make any change to the hardware, but it\n> >>>> should check that the configuration can be applied correctly, and make\n> >>>> any changes that would be necessary to support a correct (and 'valid')\n> >>>> configuration to be applied through the ->configure()\n> >>>\n> >>> Yes, that's an interesting one. The reason for doing it here is so\n> >>> that the Bayer format comes out correctly for any raw streams that\n> >>> were requested, and we're relying on the camera driver to give us the\n> >>> true Bayer order.\n> >>>\n> >>> Of course, the camera driver doesn't *have* to change the Bayer order\n> >>> when you transform it (it might do 1-pixel shits to maintain the\n> >>\n> >> haha, I hadn't noticed the typo until your follow up post ;-)\n> >>\n> >>> original Bayer order), so the puzzle then is... how would you know?\n> >>>\n> >>> Another solution I toyed with - and indeed implemented first - was to\n> >>> do it in the configure() method, but then I had to dig around and find\n> >>> any raw stream configurations and update the pixelFormat post facto.\n> >>> This involves changing stream formats after validate(), which seemed\n> >>> bad to me too... but do we prefer it?\n> >>\n> >> Ayeee - no, we can't change a format after validate either.\n> >> Oh dear have we deadlocked...\n> >>\n> >> So - is the issue that the rotation/transform affects the pixel format?\n> >>\n> >> Are we intending to support those transforms on a per-frame basis? or\n> >> just a per-stream basis.\n> >>\n> >> As long as it's per-stream - Can we just store the state of the expected\n> >> rotation in the RPiCameraConfiguration, so that we can use it at\n> >> configure time appropriately? Or is it that we will need to determine\n> >> what adjustment will be made to the pixelformat based on the transform.\n> >\n> > The latter. We need to report to the user which raw format will be used\n> > based on the transformation.\n> >\n> >> And if that's the case, I assume it's the kernel driver which is going\n> >> to tell us what the new format will be. I fear we might have to\n> >> duplicate the determination of the format up here, and report the\n> >> adjustment.\n> >>\n> >> I guess what we're lacking is the ability to do a full atomic 'try' of\n> >> the state of the device at the driver level ...\n> >\n> > Correct, and I doubt V4L2 will ever give us that.\n> >\n> > I think we need to adjust the format in the pipeline handler, instead of\n> > querying the device. We however need to know if h/v flip will affect the\n> > bayer pattern, or if the sensor will compensate by shifting the crop\n> > rectangle.\n> >\n> > One option is to query the sensor driver at match time, to see if the\n> > media bus format changes when the h/v flip controls are modified. A bit\n> > of a hassle, but shouldn't be too complicated. Another option would be\n> > to ignore all this and consider that a proper sensor driver will never\n> > shift the crop rectangle behind the scene. We have the option to\n> > establish a set of rules for a sensor driver to be supported by\n> > libcamera.\n> \n> You have a very good indicator of that from the\n> V4L2_CTRL_FLAG_MODIFY_LAYOUT flag when querying controls. It would be\n> fair to deduce from that flag on flips that it is going to change the\n> bayer order. I've yet to see a Bayer sensor that offers transpose, but\n> there may be some out there.\n\nGood point, that's easier than changing the controls and checking the\nmedia bus format.\n\n> IMX327 and IMX290 are two that don't alter the Bayer order when using\n> the on-sensor flip controls. Reading the datasheet for IMX327 (page\n> 58), it explicitly states for the full frame mode the active lines are\n> \"Line No during normal operation\" 21 to 1100\n> \"Line No during inverted operation\" 1101 to 22\n> and\n> \"Horizontal pixel output image normal operation\" 13 to1932\n> \"Horizontal pixel output image inverted operation\" 1933 to 14.\n> \n> I'm also aware of one further sensor (can't say what) that I know for\n> definite doesn't change the Bayer order. Exactly how it achieves that\n> one can only assume is by shifting the readout by one pixel.\n> \n> Should the crop reported by G_SELECTION be shifted in response to the\n> flips then?\n\nI think it should, yes.\n\n> >>>>> Signed-off-by: David Plowman \n> >>>>> ---\n> >>>>> .../pipeline/raspberrypi/raspberrypi.cpp | 18 +++++++++++-------\n> >>>>> 1 file changed, 11 insertions(+), 7 deletions(-)\n> >>>>>\n> >>>>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>>>> index 42c9caa..7aace71 100644\n> >>>>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>>>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>>>> @@ -400,6 +400,17 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> >>>>> if (config_.empty())\n> >>>>> return Invalid;\n> >>>>>\n> >>>>> + /*\n> >>>>> + * Configure the H/V flip controls based on the sensor rotation. We do\n> >>>>> + * this here so that the sensor has the correct Bayer format that will\n> >>>>> + * get advertised in the configuration of any raw streams.\n> >>>>> + */\n> >>>>> + ControlList ctrls(data_->unicam_[Unicam::Image].dev()->controls());\n> >>>>> + int32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n> >>>>> + ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> >>>>> + ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> >>>>> + data_->unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> >>>>> +\n> >>>>> unsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;\n> >>>>> std::pair outSize[2];\n> >>>>> Size maxSize;\n> >>>>> @@ -1164,13 +1175,6 @@ int RPiCameraData::configureIPA()\n> >>>>> { V4L2_CID_EXPOSURE, result.data[1] } });\n> >>>>> sensorMetadata_ = result.data[2];\n> >>>>> }\n> >>>>> -\n> >>>>> - /* Configure the H/V flip controls based on the sensor rotation. */\n> >>>>> - ControlList ctrls(unicam_[Unicam::Image].dev()->controls());\n> >>>>> - int32_t rotation = sensor_->properties().get(properties::Rotation);\n> >>>>> - ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> >>>>> - ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> >>>>> - unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> >>>>> }\n> >>>>>\n> >>>>> if (result.operation & RPI_IPA_CONFIG_SENSOR) {\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 40672BDC71\n\tfor ;\n\tFri, 28 Aug 2020 16:03:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 070FF62919;\n\tFri, 28 Aug 2020 18:03:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D6373628F3\n\tfor ;\n\tFri, 28 Aug 2020 18:03:19 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 65A5B303;\n\tFri, 28 Aug 2020 18:03:19 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Z9X0bO7u\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598630599;\n\tbh=skMufgPSMSDO+9Yl0BrN3FjFuPTu/Pf4oeytMUa6fXo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Z9X0bO7uTWi9ppWEMXtTkbtqxFB9+5bp60DlsvMWXOWdjwXQ3OblJmaPAhQtlIPJF\n\tVdY7Sd6OvUQDCy+DhXrrzSk8uGo0JXEi/AD0+0P6DS4E39ws+A5ddV5LcttUgr1THq\n\tMQo2ct9dejVK00bw5e/9Obx5BjnqZTNj4JbXjjC0=","Date":"Fri, 28 Aug 2020 19:02:58 +0300","From":"Laurent Pinchart ","To":"Dave Stevenson ","Message-ID":"<20200828160258.GH10034@pendragon.ideasonboard.com>","References":"<20200828144110.17303-1-david.plowman@raspberrypi.com>\n\t<20200828144110.17303-3-david.plowman@raspberrypi.com>\n\t<573b5b06-6492-091f-00a1-1a393d931f83@ideasonboard.com>\n\t\n\t<29854291-97f3-3f67-e7a4-d21029cc2963@ideasonboard.com>\n\t<20200828153537.GD10034@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12218,"web_url":"https://patchwork.libcamera.org/comment/12218/","msgid":"","date":"2020-08-28T16:13:09","subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","submitter":{"id":27,"url":"https://patchwork.libcamera.org/api/people/27/","name":"Dave Stevenson","email":"dave.stevenson@raspberrypi.com"},"content":"On Fri, 28 Aug 2020 at 17:03, Laurent Pinchart\n wrote:\n>\n> Hi Dave,\n>\n> On Fri, Aug 28, 2020 at 04:55:14PM +0100, Dave Stevenson wrote:\n> > On Fri, 28 Aug 2020 at 16:36, Laurent Pinchart wrote:\n> > > On Fri, Aug 28, 2020 at 04:26:05PM +0100, Kieran Bingham wrote:\n> > >> On 28/08/2020 16:14, David Plowman wrote:\n> > >>> On Fri, 28 Aug 2020 at 16:02, Kieran Bingham wrote:\n> > >>>> On 28/08/2020 15:41, David Plowman wrote:\n> > >>>>> We set the sensor orientation (h and v flips) during validate as this\n> > >>>>> will in general affect the Bayer order output by the sensor. Doing it\n> > >>>>> here means that the correct raw format gets advertised in any raw\n> > >>>>> streams that the application requested.\n> > >>>>\n> > >>>> Eeep - I'm not sure if we could do this in validate().\n> > >>>>\n> > >>>> Validation should not actually make any change to the hardware, but it\n> > >>>> should check that the configuration can be applied correctly, and make\n> > >>>> any changes that would be necessary to support a correct (and 'valid')\n> > >>>> configuration to be applied through the ->configure()\n> > >>>\n> > >>> Yes, that's an interesting one. The reason for doing it here is so\n> > >>> that the Bayer format comes out correctly for any raw streams that\n> > >>> were requested, and we're relying on the camera driver to give us the\n> > >>> true Bayer order.\n> > >>>\n> > >>> Of course, the camera driver doesn't *have* to change the Bayer order\n> > >>> when you transform it (it might do 1-pixel shits to maintain the\n> > >>\n> > >> haha, I hadn't noticed the typo until your follow up post ;-)\n> > >>\n> > >>> original Bayer order), so the puzzle then is... how would you know?\n> > >>>\n> > >>> Another solution I toyed with - and indeed implemented first - was to\n> > >>> do it in the configure() method, but then I had to dig around and find\n> > >>> any raw stream configurations and update the pixelFormat post facto.\n> > >>> This involves changing stream formats after validate(), which seemed\n> > >>> bad to me too... but do we prefer it?\n> > >>\n> > >> Ayeee - no, we can't change a format after validate either.\n> > >> Oh dear have we deadlocked...\n> > >>\n> > >> So - is the issue that the rotation/transform affects the pixel format?\n> > >>\n> > >> Are we intending to support those transforms on a per-frame basis? or\n> > >> just a per-stream basis.\n> > >>\n> > >> As long as it's per-stream - Can we just store the state of the expected\n> > >> rotation in the RPiCameraConfiguration, so that we can use it at\n> > >> configure time appropriately? Or is it that we will need to determine\n> > >> what adjustment will be made to the pixelformat based on the transform.\n> > >\n> > > The latter. We need to report to the user which raw format will be used\n> > > based on the transformation.\n> > >\n> > >> And if that's the case, I assume it's the kernel driver which is going\n> > >> to tell us what the new format will be. I fear we might have to\n> > >> duplicate the determination of the format up here, and report the\n> > >> adjustment.\n> > >>\n> > >> I guess what we're lacking is the ability to do a full atomic 'try' of\n> > >> the state of the device at the driver level ...\n> > >\n> > > Correct, and I doubt V4L2 will ever give us that.\n> > >\n> > > I think we need to adjust the format in the pipeline handler, instead of\n> > > querying the device. We however need to know if h/v flip will affect the\n> > > bayer pattern, or if the sensor will compensate by shifting the crop\n> > > rectangle.\n> > >\n> > > One option is to query the sensor driver at match time, to see if the\n> > > media bus format changes when the h/v flip controls are modified. A bit\n> > > of a hassle, but shouldn't be too complicated. Another option would be\n> > > to ignore all this and consider that a proper sensor driver will never\n> > > shift the crop rectangle behind the scene. We have the option to\n> > > establish a set of rules for a sensor driver to be supported by\n> > > libcamera.\n> >\n> > You have a very good indicator of that from the\n> > V4L2_CTRL_FLAG_MODIFY_LAYOUT flag when querying controls. It would be\n> > fair to deduce from that flag on flips that it is going to change the\n> > bayer order. I've yet to see a Bayer sensor that offers transpose, but\n> > there may be some out there.\n>\n> Good point, that's easier than changing the controls and checking the\n> media bus format.\n>\n> > IMX327 and IMX290 are two that don't alter the Bayer order when using\n> > the on-sensor flip controls. Reading the datasheet for IMX327 (page\n> > 58), it explicitly states for the full frame mode the active lines are\n> > \"Line No during normal operation\" 21 to 1100\n> > \"Line No during inverted operation\" 1101 to 22\n> > and\n> > \"Horizontal pixel output image normal operation\" 13 to1932\n> > \"Horizontal pixel output image inverted operation\" 1933 to 14.\n> >\n> > I'm also aware of one further sensor (can't say what) that I know for\n> > definite doesn't change the Bayer order. Exactly how it achieves that\n> > one can only assume is by shifting the readout by one pixel.\n> >\n> > Should the crop reported by G_SELECTION be shifted in response to the\n> > flips then?\n>\n> I think it should, yes.\n\nYou make writing sensor drivers such fun! ;-)\nAt least with that one you don't have to fight the v4l2_ctrl framework.\n\n> > >>>>> Signed-off-by: David Plowman \n> > >>>>> ---\n> > >>>>> .../pipeline/raspberrypi/raspberrypi.cpp | 18 +++++++++++-------\n> > >>>>> 1 file changed, 11 insertions(+), 7 deletions(-)\n> > >>>>>\n> > >>>>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > >>>>> index 42c9caa..7aace71 100644\n> > >>>>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > >>>>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > >>>>> @@ -400,6 +400,17 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > >>>>> if (config_.empty())\n> > >>>>> return Invalid;\n> > >>>>>\n> > >>>>> + /*\n> > >>>>> + * Configure the H/V flip controls based on the sensor rotation. We do\n> > >>>>> + * this here so that the sensor has the correct Bayer format that will\n> > >>>>> + * get advertised in the configuration of any raw streams.\n> > >>>>> + */\n> > >>>>> + ControlList ctrls(data_->unicam_[Unicam::Image].dev()->controls());\n> > >>>>> + int32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n> > >>>>> + ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> > >>>>> + ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> > >>>>> + data_->unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> > >>>>> +\n> > >>>>> unsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;\n> > >>>>> std::pair outSize[2];\n> > >>>>> Size maxSize;\n> > >>>>> @@ -1164,13 +1175,6 @@ int RPiCameraData::configureIPA()\n> > >>>>> { V4L2_CID_EXPOSURE, result.data[1] } });\n> > >>>>> sensorMetadata_ = result.data[2];\n> > >>>>> }\n> > >>>>> -\n> > >>>>> - /* Configure the H/V flip controls based on the sensor rotation. */\n> > >>>>> - ControlList ctrls(unicam_[Unicam::Image].dev()->controls());\n> > >>>>> - int32_t rotation = sensor_->properties().get(properties::Rotation);\n> > >>>>> - ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> > >>>>> - ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> > >>>>> - unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> > >>>>> }\n> > >>>>>\n> > >>>>> if (result.operation & RPI_IPA_CONFIG_SENSOR) {\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 5E2EFBDC71\n\tfor ;\n\tFri, 28 Aug 2020 16:13:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 127EE628F3;\n\tFri, 28 Aug 2020 18:13:26 +0200 (CEST)","from mail-wm1-x343.google.com (mail-wm1-x343.google.com\n\t[IPv6:2a00:1450:4864:20::343])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7507C6037B\n\tfor ;\n\tFri, 28 Aug 2020 18:13:25 +0200 (CEST)","by mail-wm1-x343.google.com with SMTP id o21so1415909wmc.0\n\tfor ;\n\tFri, 28 Aug 2020 09:13:25 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"AUK/RpKa\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=7cmslg3hqxtqvheR7mvJuIJ0BYcoS9848G/89/SnCm0=;\n\tb=AUK/RpKa0zDO/ryV/62V++en4VzXR6nHijd41wseB+nRLzGD8cdZDwF6LfZf96c77g\n\tcUOV+O4u1DU6QaXXURr37rj8rY1YhsYDGkC7XVkjopuVnz0L+XRv3AprvaMi4eC+5VQx\n\tJz+Zs+Gbk+UvZiuP3Ro7skZ/CcMtgh4JfyP6fH5FxozBPNZB8TGhl84pRks5is9Ktz8X\n\t0gozICs+8+vbKl5k2g967Tly38CjbsnzC3Hq+ws/UHT9QEaW4isIsnWqa6xHoxGWf/iv\n\tsMCDuZbnphgrSqRIHzlJO5OMuKlz5L7jy4LHTe6HxSS+A62DSm2lsQ/BImgFzoGCBJJP\n\t1dsQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=7cmslg3hqxtqvheR7mvJuIJ0BYcoS9848G/89/SnCm0=;\n\tb=LjY0oWQhtYsxFzHsaEHCX7MUw3TV3P7XR9I2iFgRvT0QnPQS8XKb0Vau+of3CJUFJP\n\tLSX1Y0WJGVqLOcTmt8dkvuY49OU0sKsSlBTiWsQx1TmxLxSr2834BAZkg9Fy+xtY+ypd\n\tNvUuujfGUVuI5/yAxsfgqgJA9aTVn3j3sdVIcV0NCwVZWKF9DUArS0HNZ/SvFgCXk2og\n\trc4Sx8WFXK49LD8Q24HIuSeg77lKrc+vN1ni5nC4rK4G/7RuNUOcistGKDFgyeBVN5Ni\n\tNA4jWD2TAmftoxFsZzew4M2k5G84sG7jLlJjEhteMRA37+TlEkNFObYon6ier9nwWCAk\n\tCyDQ==","X-Gm-Message-State":"AOAM530wvQy0fV0AGlv6CyE0C6hbEQLiyfP5L6dM871hIx8F7Mi9jOGy\n\tazcXuooIS8C/JgrkgEVdeVp7yiMiAN9s3hvRUciz/0hlLC8=","X-Google-Smtp-Source":"ABdhPJzQYAfxwMz5JHlW4oPVNEFfCujHM0zG4IrHJj/dbVPwrYZycQ4brR06jevxchWOvb7pyIAYXQqocl/jfOBOzHs=","X-Received":"by 2002:a1c:4303:: with SMTP id q3mr2283895wma.158.1598631204998;\n\tFri, 28 Aug 2020 09:13:24 -0700 (PDT)","MIME-Version":"1.0","References":"<20200828144110.17303-1-david.plowman@raspberrypi.com>\n\t<20200828144110.17303-3-david.plowman@raspberrypi.com>\n\t<573b5b06-6492-091f-00a1-1a393d931f83@ideasonboard.com>\n\t\n\t<29854291-97f3-3f67-e7a4-d21029cc2963@ideasonboard.com>\n\t<20200828153537.GD10034@pendragon.ideasonboard.com>\n\t\n\t<20200828160258.GH10034@pendragon.ideasonboard.com>","In-Reply-To":"<20200828160258.GH10034@pendragon.ideasonboard.com>","From":"Dave Stevenson ","Date":"Fri, 28 Aug 2020 17:13:09 +0100","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12219,"web_url":"https://patchwork.libcamera.org/comment/12219/","msgid":"<20200828161724.GL10034@pendragon.ideasonboard.com>","date":"2020-08-28T16:17:24","subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Aug 28, 2020 at 05:13:09PM +0100, Dave Stevenson wrote:\n> On Fri, 28 Aug 2020 at 17:03, Laurent Pinchart wrote:\n> > On Fri, Aug 28, 2020 at 04:55:14PM +0100, Dave Stevenson wrote:\n> >> On Fri, 28 Aug 2020 at 16:36, Laurent Pinchart wrote:\n> >>> On Fri, Aug 28, 2020 at 04:26:05PM +0100, Kieran Bingham wrote:\n> >>>> On 28/08/2020 16:14, David Plowman wrote:\n> >>>>> On Fri, 28 Aug 2020 at 16:02, Kieran Bingham wrote:\n> >>>>>> On 28/08/2020 15:41, David Plowman wrote:\n> >>>>>>> We set the sensor orientation (h and v flips) during validate as this\n> >>>>>>> will in general affect the Bayer order output by the sensor. Doing it\n> >>>>>>> here means that the correct raw format gets advertised in any raw\n> >>>>>>> streams that the application requested.\n> >>>>>>\n> >>>>>> Eeep - I'm not sure if we could do this in validate().\n> >>>>>>\n> >>>>>> Validation should not actually make any change to the hardware, but it\n> >>>>>> should check that the configuration can be applied correctly, and make\n> >>>>>> any changes that would be necessary to support a correct (and 'valid')\n> >>>>>> configuration to be applied through the ->configure()\n> >>>>>\n> >>>>> Yes, that's an interesting one. The reason for doing it here is so\n> >>>>> that the Bayer format comes out correctly for any raw streams that\n> >>>>> were requested, and we're relying on the camera driver to give us the\n> >>>>> true Bayer order.\n> >>>>>\n> >>>>> Of course, the camera driver doesn't *have* to change the Bayer order\n> >>>>> when you transform it (it might do 1-pixel shits to maintain the\n> >>>>\n> >>>> haha, I hadn't noticed the typo until your follow up post ;-)\n> >>>>\n> >>>>> original Bayer order), so the puzzle then is... how would you know?\n> >>>>>\n> >>>>> Another solution I toyed with - and indeed implemented first - was to\n> >>>>> do it in the configure() method, but then I had to dig around and find\n> >>>>> any raw stream configurations and update the pixelFormat post facto.\n> >>>>> This involves changing stream formats after validate(), which seemed\n> >>>>> bad to me too... but do we prefer it?\n> >>>>\n> >>>> Ayeee - no, we can't change a format after validate either.\n> >>>> Oh dear have we deadlocked...\n> >>>>\n> >>>> So - is the issue that the rotation/transform affects the pixel format?\n> >>>>\n> >>>> Are we intending to support those transforms on a per-frame basis? or\n> >>>> just a per-stream basis.\n> >>>>\n> >>>> As long as it's per-stream - Can we just store the state of the expected\n> >>>> rotation in the RPiCameraConfiguration, so that we can use it at\n> >>>> configure time appropriately? Or is it that we will need to determine\n> >>>> what adjustment will be made to the pixelformat based on the transform.\n> >>>\n> >>> The latter. We need to report to the user which raw format will be used\n> >>> based on the transformation.\n> >>>\n> >>>> And if that's the case, I assume it's the kernel driver which is going\n> >>>> to tell us what the new format will be. I fear we might have to\n> >>>> duplicate the determination of the format up here, and report the\n> >>>> adjustment.\n> >>>>\n> >>>> I guess what we're lacking is the ability to do a full atomic 'try' of\n> >>>> the state of the device at the driver level ...\n> >>>\n> >>> Correct, and I doubt V4L2 will ever give us that.\n> >>>\n> >>> I think we need to adjust the format in the pipeline handler, instead of\n> >>> querying the device. We however need to know if h/v flip will affect the\n> >>> bayer pattern, or if the sensor will compensate by shifting the crop\n> >>> rectangle.\n> >>>\n> >>> One option is to query the sensor driver at match time, to see if the\n> >>> media bus format changes when the h/v flip controls are modified. A bit\n> >>> of a hassle, but shouldn't be too complicated. Another option would be\n> >>> to ignore all this and consider that a proper sensor driver will never\n> >>> shift the crop rectangle behind the scene. We have the option to\n> >>> establish a set of rules for a sensor driver to be supported by\n> >>> libcamera.\n> >>\n> >> You have a very good indicator of that from the\n> >> V4L2_CTRL_FLAG_MODIFY_LAYOUT flag when querying controls. It would be\n> >> fair to deduce from that flag on flips that it is going to change the\n> >> bayer order. I've yet to see a Bayer sensor that offers transpose, but\n> >> there may be some out there.\n> >\n> > Good point, that's easier than changing the controls and checking the\n> > media bus format.\n> >\n> >> IMX327 and IMX290 are two that don't alter the Bayer order when using\n> >> the on-sensor flip controls. Reading the datasheet for IMX327 (page\n> >> 58), it explicitly states for the full frame mode the active lines are\n> >> \"Line No during normal operation\" 21 to 1100\n> >> \"Line No during inverted operation\" 1101 to 22\n> >> and\n> >> \"Horizontal pixel output image normal operation\" 13 to1932\n> >> \"Horizontal pixel output image inverted operation\" 1933 to 14.\n> >>\n> >> I'm also aware of one further sensor (can't say what) that I know for\n> >> definite doesn't change the Bayer order. Exactly how it achieves that\n> >> one can only assume is by shifting the readout by one pixel.\n> >>\n> >> Should the crop reported by G_SELECTION be shifted in response to the\n> >> flips then?\n> >\n> > I think it should, yes.\n> \n> You make writing sensor drivers such fun! ;-)\n> At least with that one you don't have to fight the v4l2_ctrl framework.\n\nDoes it show that v4l2_subdev hasn't been designed with a good enough\nunderstanding of camera sensors ? :-) Or, to be fairer, when previously\nunsupported features of sensors had to be supported, the API and how it\nwas used by drivers never went through a coordinated design phase.\n\n> >>>>>>> Signed-off-by: David Plowman \n> >>>>>>> ---\n> >>>>>>> .../pipeline/raspberrypi/raspberrypi.cpp | 18 +++++++++++-------\n> >>>>>>> 1 file changed, 11 insertions(+), 7 deletions(-)\n> >>>>>>>\n> >>>>>>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>>>>>> index 42c9caa..7aace71 100644\n> >>>>>>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>>>>>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>>>>>> @@ -400,6 +400,17 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> >>>>>>> if (config_.empty())\n> >>>>>>> return Invalid;\n> >>>>>>>\n> >>>>>>> + /*\n> >>>>>>> + * Configure the H/V flip controls based on the sensor rotation. We do\n> >>>>>>> + * this here so that the sensor has the correct Bayer format that will\n> >>>>>>> + * get advertised in the configuration of any raw streams.\n> >>>>>>> + */\n> >>>>>>> + ControlList ctrls(data_->unicam_[Unicam::Image].dev()->controls());\n> >>>>>>> + int32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n> >>>>>>> + ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> >>>>>>> + ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> >>>>>>> + data_->unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> >>>>>>> +\n> >>>>>>> unsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;\n> >>>>>>> std::pair outSize[2];\n> >>>>>>> Size maxSize;\n> >>>>>>> @@ -1164,13 +1175,6 @@ int RPiCameraData::configureIPA()\n> >>>>>>> { V4L2_CID_EXPOSURE, result.data[1] } });\n> >>>>>>> sensorMetadata_ = result.data[2];\n> >>>>>>> }\n> >>>>>>> -\n> >>>>>>> - /* Configure the H/V flip controls based on the sensor rotation. */\n> >>>>>>> - ControlList ctrls(unicam_[Unicam::Image].dev()->controls());\n> >>>>>>> - int32_t rotation = sensor_->properties().get(properties::Rotation);\n> >>>>>>> - ctrls.set(V4L2_CID_HFLIP, static_cast(!!rotation));\n> >>>>>>> - ctrls.set(V4L2_CID_VFLIP, static_cast(!!rotation));\n> >>>>>>> - unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> >>>>>>> }\n> >>>>>>>\n> >>>>>>> if (result.operation & RPI_IPA_CONFIG_SENSOR) {\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 4B98BBDC71\n\tfor ;\n\tFri, 28 Aug 2020 16:17:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D7A6D61EA0;\n\tFri, 28 Aug 2020 18:17:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B86356037B\n\tfor ;\n\tFri, 28 Aug 2020 18:17:45 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2D036303;\n\tFri, 28 Aug 2020 18:17:45 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"N2swVxTX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598631465;\n\tbh=4yf7GgP0kmRnKmKT8ioJttK2eZhkaQJne3TdDpUmhIw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=N2swVxTXy2yO+r9yK6K2/vMAZEKeMniyMuL49RpzkvhKUhZooPukg9xe43oPgsqMp\n\t09mt1cgVsYwnOGhjPG2cKrxkUgRXYuLNNZKOg4lJynGHDHvH5YW9xASrBScs9frIU6\n\tyBjz9fDXnaR2f/BNhHQ3qYMr+Wh9Y9MzsvXUEsuY=","Date":"Fri, 28 Aug 2020 19:17:24 +0300","From":"Laurent Pinchart ","To":"Dave Stevenson ","Message-ID":"<20200828161724.GL10034@pendragon.ideasonboard.com>","References":"<20200828144110.17303-1-david.plowman@raspberrypi.com>\n\t<20200828144110.17303-3-david.plowman@raspberrypi.com>\n\t<573b5b06-6492-091f-00a1-1a393d931f83@ideasonboard.com>\n\t\n\t<29854291-97f3-3f67-e7a4-d21029cc2963@ideasonboard.com>\n\t<20200828153537.GD10034@pendragon.ideasonboard.com>\n\t\n\t<20200828160258.GH10034@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 2/7] libcamera: pipeline:\n\traspberrypi: Set sensor orientation during validate","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]