[{"id":2512,"web_url":"https://patchwork.libcamera.org/comment/2512/","msgid":"<20190828110820.GN28351@bigcity.dyn.berto.se>","date":"2019-08-28T11:08:20","subject":"Re: [libcamera-devel] [PATCH v2 3/7] libcamera: controls: Add\n\tcamera properties IDs","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 2019-08-27 11:50:03 +0200, Jacopo Mondi wrote:\n> Add the PropertyID enumeration where to list the camera static\n> properties supported by libcamera. Initially add the Location and\n> Rotation properties\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> include/libcamera/control_ids.h | 11 +++++++\n> src/libcamera/controls.cpp | 55 +++++++++++++++++++++++++++++++++\n> 2 files changed, 66 insertions(+)\n> \n> diff --git a/include/libcamera/control_ids.h b/include/libcamera/control_ids.h\n> index 75b6a2d5cafe..d8c3c3265ee6 100644\n> --- a/include/libcamera/control_ids.h\n> +++ b/include/libcamera/control_ids.h\n> @@ -21,6 +21,17 @@ enum ControlId {\n> \tManualGain,\n> };\n> \n> +enum CameraLocation {\n> +\tCAMERA_LOCATION_EXTERNAL,\n> +\tCAMERA_LOCATION_FRONT,\n> +\tCAMERA_LOCATION_BACK,\n> +};\n> +\n> +enum PropertyId {\n> +\tLocation,\n> +\tRotation,\n> +};\n> +\n> } /* namespace libcamera */\n> \n> namespace std {\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 9adc3badc254..9562ecc189bb 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -555,4 +555,59 @@ void ControlList::update(const ControlList &other)\n> * Specify a fixed gain parameter\n> */\n> \n> +/**\n> + * \\enum CameraLocation\n> + * \\brief List the supported mounting location of a camera\n> + */\n> +\n> +/**\n> + * \\var CameraLocation::CAMERA_LOCATION_EXTERNAL\n> + * \\brief Identify an external camera\n> + *\n> + * The camera is connected to the main device through extension cables\n> + * and is freely movable. Typical examples of externally mounted cameras are\n> + * webcams and digital camera devices.\n> + */\n\nMaybe this can be rephrased to not mention cables, maybe one day there \nwill be a Bluetooth enabled camera which would be external ;-)\n\n> +\n> +/**\n> + * \\var CameraLocation::CAMERA_LOCATION_FRONT\n> + * \\brief Identify a camera mounted in the device front location\n> + *\n> + * The camera is mounted on the front side of the device, which is typically\n> + * the user facing side.\n> + */\n> +\n> +/**\n> + * \\var CameraLocation::CAMERA_LOCATION_BACK\n> + * \\brief Identify a camera mounted in the device back location\n> + *\n> + * The camera is mounted on the back side of the device, which is typically\n> + * the side opposed to the front one.\n> + */\n> +\n> +/**\n> + * \\enum PropertyId\n> + * \\brief Numerical properties ID\n> + *\n> + * List the identifiers of the static properties exposed by a Camera.\n> + */\n> +\n> +/**\n> + * \\var PropertyId::Location\n> + * \\brief The camera mounting location\n> + *\n> + * The Camera device location is expressed as the position relative to the\n> + * device intended usage orientation. Possible values are identified by the\n> + * libcamera::CameraLocation enumeration.\n> + */\n> +\n> +/**\n> + * \\var PropertyId::Rotation\n> + * \\brief The camera sensor rotation\n> + *\n> + * The Camera rotation is expressed as counter-clockwise rotation in degrees\n> + * in respect to the intended device orientation. Typical values are 0 for\n> + * upright mounted cameras, and 180 for cameras mounted upside down.\n> + */\n\nOut of curiosity why counter-clockwise and not clockwise? ;-)\n\nI think this could be improved by explicitly stating where the observe \nis when examining the rotation. Am I looking into the camera when I \nrotate it or am \"I\" the camera and rotate. Maybe this is a bit over \nzealous, bit I always find this type of information lacking when I read \nthis type of information and have to find out by train and error ;-)\n\n> +\n> } /* namespace libcamera */\n> -- \n> 2.23.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":"","Received":["from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9A1BB60BF6\n\tfor ;\n\tWed, 28 Aug 2019 13:08:22 +0200 (CEST)","by mail-lf1-x144.google.com with SMTP id h27so1807417lfp.2\n\tfor ;\n\tWed, 28 Aug 2019 04:08:22 -0700 (PDT)","from localhost (h-177-236.A463.priv.bahnhof.se. [217.31.177.236])\n\tby smtp.gmail.com with ESMTPSA id\n\tm4sm569958ljj.78.2019.08.28.04.08.20\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 28 Aug 2019 04:08:20 -0700 (PDT)"],"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\t:user-agent; bh=dX6FBrQwuizCFetL5FqupULsPb+9JHYvrbjoCFe5WLE=;\n\tb=OWKmCffHcHrN5n2kjP+ICj0YhYHmUQDnl+CamWKkHZU+Z7tsDuWVtBIt+cv86y5O9U\n\t85U75UQowM/5oTzKmtjQB7VSy++D3JPsOY1YxZBly6n/Egz7Drc4LGaZhpuBNCfpfV9X\n\tSx9QkVN86t0LzIu8ACiNeZuqYTbLsNeGpD09XgG50XJnGgW4mQJTHp/C4LfY5b9+8kyV\n\t8O5xGuxMNtQtXaeOxL96PAjFrmssT6IXzsYyqrQx2UpoXibhd8+Rz8djV3eVILXVgcvj\n\tVqNJhBnIZeBUwP4t3QgwQdXxyC8FNxXQmwkTjeeCdlLFqKAd8HxHx/V824Max64PdXrg\n\tOJYw==","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:user-agent;\n\tbh=dX6FBrQwuizCFetL5FqupULsPb+9JHYvrbjoCFe5WLE=;\n\tb=H90ZixJxWWcYLqX2P47vUyc25a+5wTpCFMaSvB8tlMOgrx8d3E0CVoo2QiZmvfjPmA\n\t1+SNf/Z7suGoHKfw1sTm3VBeht7qGDo5H86Nw/rsvMYDFwlTD5714q5ewFTI3lpkJcXZ\n\tvExafSJ7aANFdpSDCLOOlIBtZ4bzMLg2ICy7YK596nzGMDwuwXbQvPT3AdrDidGVUefZ\n\tI4LEORjLFN1OWuZ2eW3REX164XmlTA9JJGNEb8UY8TH4FR1zMu+THID1nZVz8kfuLaSz\n\tXpcFUK4COche8kKXqAEtMBPV8RywRoR6k+aeOHTrfUTiqQtnH4TzvjeQ1k9eWxzpRmEs\n\tbjxQ==","X-Gm-Message-State":"APjAAAXB1cpamUt7Ed7WYjOEv9L4ILdDM18WLIC8yZAZt7PRKc1NPqWN\n\t5YtsBU++gJwMM+mFZZHh/j62IM671e8=","X-Google-Smtp-Source":"APXvYqzSxaV7GUZy07dYSbGYDoxv1eYbUGt1rb4CqkqkbY63m31Jtrmty3gLy58Z+R/AGaMHNL++zg==","X-Received":"by 2002:a19:e04f:: with SMTP id g15mr1651622lfj.46.1566990502077;\n\tWed, 28 Aug 2019 04:08:22 -0700 (PDT)","Date":"Wed, 28 Aug 2019 13:08:20 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190828110820.GN28351@bigcity.dyn.berto.se>","References":"<20190827095008.11405-1-jacopo@jmondi.org>\n\t<20190827095008.11405-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190827095008.11405-4-jacopo@jmondi.org>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH v2 3/7] libcamera: controls: Add\n\tcamera properties IDs","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 28 Aug 2019 11:08:22 -0000"}},{"id":2572,"web_url":"https://patchwork.libcamera.org/comment/2572/","msgid":"<20190903201707.GB4788@pendragon.ideasonboard.com>","date":"2019-09-03T20:17:07","subject":"Re: [libcamera-devel] [PATCH v2 3/7] libcamera: controls: Add\n\tcamera properties IDs","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Wed, Aug 28, 2019 at 01:08:20PM +0200, Niklas Söderlund wrote:\n> On 2019-08-27 11:50:03 +0200, Jacopo Mondi wrote:\n> > Add the PropertyID enumeration where to list the camera static\n> > properties supported by libcamera. Initially add the Location and\n> > Rotation properties\n> > \n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > include/libcamera/control_ids.h | 11 +++++++\n> > src/libcamera/controls.cpp | 55 +++++++++++++++++++++++++++++++++\n> > 2 files changed, 66 insertions(+)\n> > \n> > diff --git a/include/libcamera/control_ids.h b/include/libcamera/control_ids.h\n> > index 75b6a2d5cafe..d8c3c3265ee6 100644\n> > --- a/include/libcamera/control_ids.h\n> > +++ b/include/libcamera/control_ids.h\n> > @@ -21,6 +21,17 @@ enum ControlId {\n> > \tManualGain,\n> > };\n> > \n> > +enum CameraLocation {\n> > +\tCAMERA_LOCATION_EXTERNAL,\n> > +\tCAMERA_LOCATION_FRONT,\n> > +\tCAMERA_LOCATION_BACK,\n> > +};\n> > +\n> > +enum PropertyId {\n> > +\tLocation,\n> > +\tRotation,\n> > +};\n> > +\n> > } /* namespace libcamera */\n> > \n> > namespace std {\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index 9adc3badc254..9562ecc189bb 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -555,4 +555,59 @@ void ControlList::update(const ControlList &other)\n> > * Specify a fixed gain parameter\n> > */\n> > \n> > +/**\n> > + * \\enum CameraLocation\n> > + * \\brief List the supported mounting location of a camera\n> > + */\n> > +\n> > +/**\n> > + * \\var CameraLocation::CAMERA_LOCATION_EXTERNAL\n> > + * \\brief Identify an external camera\n> > + *\n> > + * The camera is connected to the main device through extension cables\n> > + * and is freely movable. Typical examples of externally mounted cameras are\n> > + * webcams and digital camera devices.\n> > + */\n> \n> Maybe this can be rephrased to not mention cables, maybe one day there \n> will be a Bluetooth enabled camera which would be external ;-)\n\nI think I pointed this out during review of the kernel side patches. The\ndocumentation of the properties in libcamera will likely copy the one\nfrom the kernel, so I think we should wait until the kernel side is\nsettled and then respin this patch.\n\n> > +\n> > +/**\n> > + * \\var CameraLocation::CAMERA_LOCATION_FRONT\n> > + * \\brief Identify a camera mounted in the device front location\n> > + *\n> > + * The camera is mounted on the front side of the device, which is typically\n> > + * the user facing side.\n> > + */\n> > +\n> > +/**\n> > + * \\var CameraLocation::CAMERA_LOCATION_BACK\n> > + * \\brief Identify a camera mounted in the device back location\n> > + *\n> > + * The camera is mounted on the back side of the device, which is typically\n> > + * the side opposed to the front one.\n> > + */\n> > +\n> > +/**\n> > + * \\enum PropertyId\n> > + * \\brief Numerical properties ID\n> > + *\n> > + * List the identifiers of the static properties exposed by a Camera.\n> > + */\n> > +\n> > +/**\n> > + * \\var PropertyId::Location\n> > + * \\brief The camera mounting location\n> > + *\n> > + * The Camera device location is expressed as the position relative to the\n> > + * device intended usage orientation. Possible values are identified by the\n> > + * libcamera::CameraLocation enumeration.\n> > + */\n> > +\n> > +/**\n> > + * \\var PropertyId::Rotation\n> > + * \\brief The camera sensor rotation\n> > + *\n> > + * The Camera rotation is expressed as counter-clockwise rotation in degrees\n> > + * in respect to the intended device orientation. Typical values are 0 for\n> > + * upright mounted cameras, and 180 for cameras mounted upside down.\n> > + */\n> \n> Out of curiosity why counter-clockwise and not clockwise? ;-)\n> \n> I think this could be improved by explicitly stating where the observe \n> is when examining the rotation. Am I looking into the camera when I \n> rotate it or am \"I\" the camera and rotate. Maybe this is a bit over \n> zealous, bit I always find this type of information lacking when I read \n> this type of information and have to find out by train and error ;-)\n\nI've had a loooong discussion with Jacopo regarding how to express\nrotations. Please see \"[PATCH v2 03/10] media: v4l2-ctrl: Document\nV4L2_CID_CAMERA_SENSOR_ROTATION\". Rotations are very tricky and\ncounter-intuitive, and we need very detailed documentation that\ndescribes\n\n- the rotation axis (which is a vector, and thus has a direction)\n- the rotation angle (unit and meaning of positive/negative values)\n\nRegarding the angle, I think we should follow the usual practices of\ngeometry and trigonometry, I see no reason to depart from that.\nRegarding the axis, it should be perpendicular to the sensor plane, and\nthe direction is currently being debated in the abovementioned mail\nthread.\n\n> > +\n> > } /* namespace libcamera */","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 02E8D60BCF\n\tfor ;\n\tTue, 3 Sep 2019 22:17:16 +0200 (CEST)","from pendragon.ideasonboard.com (85-76-18-41-nat.elisa-mobile.fi\n\t[85.76.18.41])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 158AD542;\n\tTue, 3 Sep 2019 22:17:14 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1567541835;\n\tbh=uMkPpzUXaRDLrd/EeCRf1ej5/5jJWwT/v2KZzC6JuPs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kYcun0hxIShpbnoD9zrJgAETH6LuYfk9hu+kxXGSKCVS/fEFCWWFFE/2Xv6GKOb9s\n\tr5rf5TbeyNx3GVyZOb3UP2W27CxOiB6wknmnN4wFQHw+3XLgtlQoWYRELtaBkeLdrg\n\tmpGm4ddh328HZBmGEA0w1sS2LojoZPFnkEE5mlfg=","Date":"Tue, 3 Sep 2019 23:17:07 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","Message-ID":"<20190903201707.GB4788@pendragon.ideasonboard.com>","References":"<20190827095008.11405-1-jacopo@jmondi.org>\n\t<20190827095008.11405-4-jacopo@jmondi.org>\n\t<20190828110820.GN28351@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190828110820.GN28351@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/7] libcamera: controls: Add\n\tcamera properties IDs","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 03 Sep 2019 20:17:16 -0000"}}]