[{"id":11898,"web_url":"https://patchwork.libcamera.org/comment/11898/","msgid":"<20200805211750.GG2712616@oden.dyn.berto.se>","date":"2020-08-05T21:17:50","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Match V4L2\n\tspecification","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your work.\n\nOn 2020-08-05 14:30:04 +0200, Jacopo Mondi wrote:\n> Adjust the top-left corner coordinates of the analogCrop rectangle\n> to match the libcamera and V4L2 specification.\n> \n> As per the V4L2 selection API specification, all rectangles accessed\n> using V4L2_SEL_TGT_* are defined relatively to the full pixel array,\n> which is retrieved using the V4L2_SEL_TGT_NATIVE target. Libcamera\n> defines the analogCrop rectangle to be defined relatively\n> to the active pixel matrix rectangle.\n> \n> To compensate the difference in the reference rectangle between the two\n> specification, subtract from the TGT_CROP rectangle top-left corner the\n> offset of the active pixel array previously obtained with\n> TGT_CROP_DEFAULT.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> \n> This patch depends on the inclusion of the following patch in the\n> Linux kernel:\n> [PATCH 4/4] media: i2c: imx219: Selection compliance fixes\n\nI think the patch looks fine, but this comment puzzles me. Do this \nchange depend on the for mentioned patch for correct behavior of the \ncontrols or is a regression introduced due to a missing fix for imx219 ?\n\nIn either case for this patch as long as it's not merged and introducing \na regression,\n\nReviewed-by: Niklas Söderlund \n\n> ---\n> src/libcamera/camera_sensor.cpp | 12 ++++++++++++\n> 1 file changed, 12 insertions(+)\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 350f49accad9..8f09206ff635 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -508,6 +508,18 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> \t\treturn ret;\n> \t}\n> \n> +\t/*\n> +\t * The top-left corner position of the selection rectangle returned by\n> +\t * the V4L2_SEL_TGT_CROP target is defined relatively to the full pixel\n> +\t * array, while libcamera's analogCrop is defined relatively to the\n> +\t * active portion of the pixel matrix.\n> +\t *\n> +\t * Subtract from the top-left coordinates the offsets of the\n> +\t * CROP_DEFAULT target rectangle to align the two.\n> +\t */\n> +\tinfo->analogCrop.x -= rect.x;\n> +\tinfo->analogCrop.y -= rect.y;\n> +\n> \t/* The bit depth and image size depend on the currently applied format. */\n> \tV4L2SubdeviceFormat format{};\n> \tret = subdev_->getFormat(pad_, &format);\n> --\n> 2.27.0\n> \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 0EE6FBD87A\n\tfor ;\n\tWed, 5 Aug 2020 21:17:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 89E156071B;\n\tWed, 5 Aug 2020 23:17:54 +0200 (CEST)","from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 97BF66039D\n\tfor ;\n\tWed, 5 Aug 2020 23:17:52 +0200 (CEST)","by mail-lf1-x142.google.com with SMTP id j22so19130695lfm.2\n\tfor ;\n\tWed, 05 Aug 2020 14:17:52 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\te14sm1369762ljl.96.2020.08.05.14.17.50\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 05 Aug 2020 14:17:50 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"OFw37cHi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=3serbQMZOQWygA7KJ94wMo1BkCKpboiaSq/m4h8vGSg=;\n\tb=OFw37cHiO5bjsa4I6WFXeT60toMrt4EZvLNaMioHEYdnUPqUIgJrIVnEPrriwgdE58\n\tSgas5ZF7koBCJprImqra403IkhnlS789qeowJ9/qJoxxhiTb6kRg+57HCedzzc9PB9GW\n\tH7oxAhbyRTJkuiMr5msMqUVwjkVsGU7oOW2MFPlpdupDW8sGk1tknMFD6jDU/kqk7Eaf\n\tCGQw6HMLBoLqpAZJW5Yfzo07uLEqZ+9njMTw9By49UmFvuX0F8aNFL4LObWsBKUWu9v+\n\tAZRnv1IYaRhfjQ87nyl/wQYUa9qnORAjChkrUpWJwgtEttU3Aau3eyWIeuektSBA1NLk\n\tdoGA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=3serbQMZOQWygA7KJ94wMo1BkCKpboiaSq/m4h8vGSg=;\n\tb=YprVvtGGHct1HmR3Mn4gr3cCj5qeUnfwMRrNViPAQCsQhXXBT4Y7kVYPh9QoYcO4/6\n\tmFbLiUNpCeax9+Y+jD/KpcNu3rkr8BUJDSdW/6hfEMVx+a4oolugVV/bQaiWd0OJab3k\n\tvRR42hkSxur/L0t0izJEmGlFybWGcocJyWcGsJiMiyV51efGZZKl2BLWtwhiDd40Gxm7\n\tVSd9ZR8zDr6KSNXeIcgNHruoW3mcVUOCMJwITpOQe+kzDWpn0oGYFKvB4zFjI1LeD8SK\n\tklASekGWJ678ckqJ7U7DOu9W8L/210/ffDnW7H912RrEF7/z5ZIuIRnb58agsu8ixZbP\n\tfEzQ==","X-Gm-Message-State":"AOAM53201YIRx5ynsjHKUT3D2k3kpcuyPxfJJCcbz7dAB7Wv6EY4hKaq\n\t5jkairvzd8lNgdujOxa57UHxNA==","X-Google-Smtp-Source":"ABdhPJxbupXGbg0IeXblhuWQddL+7eMJ/EXjB77G1O5GkF8+QjL5c18rDy8VsFiPCZkBYi2dVk2h/w==","X-Received":"by 2002:a19:8295:: with SMTP id\n\te143mr2352865lfd.95.1596662271745; \n\tWed, 05 Aug 2020 14:17:51 -0700 (PDT)","Date":"Wed, 5 Aug 2020 23:17:50 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"<20200805211750.GG2712616@oden.dyn.berto.se>","References":"<20200805123004.18794-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200805123004.18794-1-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Match V4L2\n\tspecification","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11903,"web_url":"https://patchwork.libcamera.org/comment/11903/","msgid":"<20200805214254.oyikeg345yyc7wpb@uno.localdomain>","date":"2020-08-05T21:42:54","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Match V4L2\n\tspecification","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Wed, Aug 05, 2020 at 11:17:50PM +0200, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your work.\n>\n> On 2020-08-05 14:30:04 +0200, Jacopo Mondi wrote:\n> > Adjust the top-left corner coordinates of the analogCrop rectangle\n> > to match the libcamera and V4L2 specification.\n> >\n> > As per the V4L2 selection API specification, all rectangles accessed\n> > using V4L2_SEL_TGT_* are defined relatively to the full pixel array,\n> > which is retrieved using the V4L2_SEL_TGT_NATIVE target. Libcamera\n> > defines the analogCrop rectangle to be defined relatively\n> > to the active pixel matrix rectangle.\n> >\n> > To compensate the difference in the reference rectangle between the two\n> > specification, subtract from the TGT_CROP rectangle top-left corner the\n> > offset of the active pixel array previously obtained with\n> > TGT_CROP_DEFAULT.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> >\n> > This patch depends on the inclusion of the following patch in the\n> > Linux kernel:\n> > [PATCH 4/4] media: i2c: imx219: Selection compliance fixes\n>\n> I think the patch looks fine, but this comment puzzles me. Do this\n> change depend on the for mentioned patch for correct behavior of the\n> controls or is a regression introduced due to a missing fix for imx219 ?\n>\n> In either case for this patch as long as it's not merged and introducing\n> a regression,\n\nLet's say someone (me) has implemented upstream support for\nG_SELECTION which does not match the intended behaviour. Partly\nbecause that part is not specified at all in V4L2 :) libcamera assumes\nthe current mainline behaviour, and the above mentioned kernel series\nmakes imx219 comply with what is expected. I sent it out yesterday,\nthe single patch itself is not controversial, the documentation part\nmight take longer, but the imx219 change could be broke out.\n\nThe moment that patch gets in, the libcamera side should be updated as\nwell, and this patch does so.\n\nSo yes, I sent it out, but it will have to stay out of tree for a\nwhile (5.10 or late fix for 5.9? I'll ping Hans tomorrow and ask).\n\n>\n> Reviewed-by: Niklas Söderlund \n>\n> > ---\n> > src/libcamera/camera_sensor.cpp | 12 ++++++++++++\n> > 1 file changed, 12 insertions(+)\n> >\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 350f49accad9..8f09206ff635 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -508,6 +508,18 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> > \t\treturn ret;\n> > \t}\n> >\n> > +\t/*\n> > +\t * The top-left corner position of the selection rectangle returned by\n> > +\t * the V4L2_SEL_TGT_CROP target is defined relatively to the full pixel\n> > +\t * array, while libcamera's analogCrop is defined relatively to the\n> > +\t * active portion of the pixel matrix.\n> > +\t *\n> > +\t * Subtract from the top-left coordinates the offsets of the\n> > +\t * CROP_DEFAULT target rectangle to align the two.\n> > +\t */\n> > +\tinfo->analogCrop.x -= rect.x;\n> > +\tinfo->analogCrop.y -= rect.y;\n> > +\n> > \t/* The bit depth and image size depend on the currently applied format. */\n> > \tV4L2SubdeviceFormat format{};\n> > \tret = subdev_->getFormat(pad_, &format);\n> > --\n> > 2.27.0\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards,\n> Niklas Söderlund","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 8F155BD86F\n\tfor ;\n\tWed, 5 Aug 2020 21:39:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 15B126071B;\n\tWed, 5 Aug 2020 23:39:17 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 82E2B6039D\n\tfor ;\n\tWed, 5 Aug 2020 23:39:15 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id CD687100006;\n\tWed, 5 Aug 2020 21:39:14 +0000 (UTC)"],"Date":"Wed, 5 Aug 2020 23:42:54 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200805214254.oyikeg345yyc7wpb@uno.localdomain>","References":"<20200805123004.18794-1-jacopo@jmondi.org>\n\t<20200805211750.GG2712616@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200805211750.GG2712616@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Match V4L2\n\tspecification","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11904,"web_url":"https://patchwork.libcamera.org/comment/11904/","msgid":"<20200805214107.GO6751@pendragon.ideasonboard.com>","date":"2020-08-05T21:41:07","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Match V4L2\n\tspecification","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Wed, Aug 05, 2020 at 02:30:04PM +0200, Jacopo Mondi wrote:\n> Adjust the top-left corner coordinates of the analogCrop rectangle\n> to match the libcamera and V4L2 specification.\n> \n> As per the V4L2 selection API specification, all rectangles accessed\n> using V4L2_SEL_TGT_* are defined relatively to the full pixel array,\n> which is retrieved using the V4L2_SEL_TGT_NATIVE target. Libcamera\n> defines the analogCrop rectangle to be defined relatively\n> to the active pixel matrix rectangle.\n> \n> To compensate the difference in the reference rectangle between the two\n> specification, subtract from the TGT_CROP rectangle top-left corner the\n> offset of the active pixel array previously obtained with\n> TGT_CROP_DEFAULT.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> \n> This patch depends on the inclusion of the following patch in the\n> Linux kernel:\n> [PATCH 4/4] media: i2c: imx219: Selection compliance fixes\n\nLet's postpone this until the review discussions related to the above\npatch are complete.\n\n> ---\n> src/libcamera/camera_sensor.cpp | 12 ++++++++++++\n> 1 file changed, 12 insertions(+)\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 350f49accad9..8f09206ff635 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -508,6 +508,18 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> \t\treturn ret;\n> \t}\n> \n> +\t/*\n> +\t * The top-left corner position of the selection rectangle returned by\n> +\t * the V4L2_SEL_TGT_CROP target is defined relatively to the full pixel\n> +\t * array, while libcamera's analogCrop is defined relatively to the\n> +\t * active portion of the pixel matrix.\n> +\t *\n> +\t * Subtract from the top-left coordinates the offsets of the\n> +\t * CROP_DEFAULT target rectangle to align the two.\n> +\t */\n> +\tinfo->analogCrop.x -= rect.x;\n> +\tinfo->analogCrop.y -= rect.y;\n> +\n> \t/* The bit depth and image size depend on the currently applied format. */\n> \tV4L2SubdeviceFormat format{};\n> \tret = subdev_->getFormat(pad_, &format);","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 F3E3CBD87A\n\tfor ;\n\tWed, 5 Aug 2020 21:41:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9174D6071B;\n\tWed, 5 Aug 2020 23:41:21 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 361126039D\n\tfor ;\n\tWed, 5 Aug 2020 23:41:20 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9B45F277;\n\tWed, 5 Aug 2020 23:41: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=\"eGejeld6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596663679;\n\tbh=01g5G99FyZHavwxZ5Izg5at1ZIVYoMx7mza480Ypq+I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eGejeld62Fy3uy2rNfaA4RuM6bXo6hVq5UX+PSN1Vu1TUsC9Q0zavpwpNlBHhf892\n\tRNOc3PGiTHLJVvNXdOgSIGmqJEp8LUtD42r2beiPnq33kXhNwcKnhES7JsZXZq6ThQ\n\tPWZyxjwiuF0TgbvpQqWN/Y6U6c0go4kUq+xAeZZc=","Date":"Thu, 6 Aug 2020 00:41:07 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200805214107.GO6751@pendragon.ideasonboard.com>","References":"<20200805123004.18794-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200805123004.18794-1-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Match V4L2\n\tspecification","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\" "}}]