[{"id":4139,"web_url":"https://patchwork.libcamera.org/comment/4139/","msgid":"<383cd00a-be9f-2304-5425-39dd9d232d88@ideasonboard.com>","date":"2020-03-20T14:28:47","subject":"Re: [libcamera-devel] [PATCH v2 2/6] libcamera: uvcvideo: Update\n\texposure/gain ctrls set with new units","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nOn 09/03/2020 12:33, Naushir Patuck wrote:\n> The ManualExposure control now uses units of 1 micro-second, and UVC\n> uses units of 100 micro-seconds. Correctly map the values before\n> setting V4L2_CID_EXPOSURE_ABSOLUTE on the V4L2 device.\n> \n> The ManualGain control now uses floats to allow fractional gain values.\n> Since UVC has no explicit gain units, map the default gain value to 1.0\n> and linearly map to the requested value.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> src/libcamera/pipeline/uvcvideo.cpp | 17 +++++++++++++++--\n> 1 file changed, 15 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 29afb121..aff86803 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -245,9 +245,22 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> \t\t\tcontrols.set(V4L2_CID_SATURATION, value);\n> \t\t} else if (id == controls::ManualExposure) {\n> \t\t\tcontrols.set(V4L2_CID_EXPOSURE_AUTO, static_cast(1));> -\t\t\tcontrols.set(V4L2_CID_EXPOSURE_ABSOLUTE, value);\n> +\t\t\t/*\n> +\t\t\t * controls::ManualExposure is in units of 1 us, and UVC\n> +\t\t\t * expects V4L2_CID_EXPOSURE_ABSOLUTE in units of 100 us.\n> +\t\t\t * So divide by 100 when setting the control.\n\nSounds fine, but possibly the last line isn't needed, or I'd at least\nremove the 'So' ... but that's trivial/minor.\n\n\n> +\t\t\t */\n> +\t\t\tcontrols.set(V4L2_CID_EXPOSURE_ABSOLUTE, value.get() / 100);\n> \t\t} else if (id == controls::ManualGain) {\n> -\t\t\tcontrols.set(V4L2_CID_GAIN, value);\n> +\t\t\t/*\n> +\t\t\t * controls::ManualGain is specified as an absolute float value.\n> +\t\t\t * Map this in a linear way such that 1.0 -> default gain\n> +\t\t\t * of the V4L2_CID_GAIN control.\n\nThis doesn't seem very clear to me yet. (I get the intent it's just the\ndescription), and if we're 'mapping' the value like this should it be\ndocumented at the control level?\n\nOr do you expect this to apply only to UVC? (Presumably if a device\ndoesn't specify a default gain, it's just '1.0' ?)\n\nPerhaps going back to patch 1/6 we need to explain more about how\nManualGain is represented to document it's usage clearly there...\n\n\n> +\t\t\t */\n> +\t\t\tControlRange gainInfo = controls.infoMap()->at(V4L2_CID_GAIN);\n> +\t\t\tfloat requestGain = value.get();\n> +\t\t\tint32_t gain = requestGain * gainInfo.def().get();\n> +\t\t\tcontrols.set(V4L2_CID_GAIN, gain);\n> \t\t}\n> \t}\n> \n>","headers":{"Return-Path":"","Received":["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 7FF4660415\n\tfor ;\n\tFri, 20 Mar 2020 15:28:51 +0100 (CET)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 03E24504;\n\tFri, 20 Mar 2020 15:28:50 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584714531;\n\tbh=J7ooy5MkErlJ8kEkKSNEDiwmx6LtuQ1qw/JvnFaBVYY=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=Mnl188Sy6XlN+MBIFgYM+OAFnXMma+itPtFkvms9q3NYDRKjSNC/6l3C2BsAtHNab\n\t4i3lT3joRuzKDCot1g6KCjgFRzpJxPs2KtC305ege9bUoELWhUxetOM48JZGTSNO/m\n\tJDbS8369wJ4wkUFg2oAwpgP15m7GhGy+Uc0t/MFk=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200309123319.630-1-naush@raspberrypi.com>\n\t<20200309123319.630-3-naush@raspberrypi.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","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":"<383cd00a-be9f-2304-5425-39dd9d232d88@ideasonboard.com>","Date":"Fri, 20 Mar 2020 14:28:47 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.1","MIME-Version":"1.0","In-Reply-To":"<20200309123319.630-3-naush@raspberrypi.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 2/6] libcamera: uvcvideo: Update\n\texposure/gain ctrls set with new units","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","X-List-Received-Date":"Fri, 20 Mar 2020 14:28:51 -0000"}},{"id":4224,"web_url":"https://patchwork.libcamera.org/comment/4224/","msgid":"","date":"2020-03-23T16:03:57","subject":"Re: [libcamera-devel] [PATCH v2 2/6] libcamera: uvcvideo: Update\n\texposure/gain ctrls set with new units","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\nOn Fri, 20 Mar 2020 at 14:28, Kieran Bingham\n wrote:\n>\n> Hi Naush,\n>\n> On 09/03/2020 12:33, Naushir Patuck wrote:\n> > The ManualExposure control now uses units of 1 micro-second, and UVC\n> > uses units of 100 micro-seconds. Correctly map the values before\n> > setting V4L2_CID_EXPOSURE_ABSOLUTE on the V4L2 device.\n> >\n> > The ManualGain control now uses floats to allow fractional gain values.\n> > Since UVC has no explicit gain units, map the default gain value to 1.0\n> > and linearly map to the requested value.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/libcamera/pipeline/uvcvideo.cpp | 17 +++++++++++++++--\n> > 1 file changed, 15 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index 29afb121..aff86803 100644\n> > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > @@ -245,9 +245,22 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> > controls.set(V4L2_CID_SATURATION, value);\n> > } else if (id == controls::ManualExposure) {\n> > controls.set(V4L2_CID_EXPOSURE_AUTO, static_cast(1));> - controls.set(V4L2_CID_EXPOSURE_ABSOLUTE, value);\n> > + /*\n> > + * controls::ManualExposure is in units of 1 us, and UVC\n> > + * expects V4L2_CID_EXPOSURE_ABSOLUTE in units of 100 us.\n> > + * So divide by 100 when setting the control.\n>\n> Sounds fine, but possibly the last line isn't needed, or I'd at least\n> remove the 'So' ... but that's trivial/minor.\n\nWill do.\n\n>\n>\n> > + */\n> > + controls.set(V4L2_CID_EXPOSURE_ABSOLUTE, value.get() / 100);\n> > } else if (id == controls::ManualGain) {\n> > - controls.set(V4L2_CID_GAIN, value);\n> > + /*\n> > + * controls::ManualGain is specified as an absolute float value.\n> > + * Map this in a linear way such that 1.0 -> default gain\n> > + * of the V4L2_CID_GAIN control.\n>\n> This doesn't seem very clear to me yet. (I get the intent it's just the\n> description), and if we're 'mapping' the value like this should it be\n> documented at the control level?\n>\n> Or do you expect this to apply only to UVC? (Presumably if a device\n> doesn't specify a default gain, it's just '1.0' ?)\n\nThis mapping is explicitly for UVC.\n\nThe ManualExposure control is meant to be an explicit exposure time to\nuse, and likewise, ManualGain is an explicit gain multiplier applied\nto all samples. From Laurent's comment on patch v1, he noted that UVC\ndoesn't specify units for gain. I took that to mean that the gain\ncontrol is just a range-type control to increase the brightness, and\nthe mapping in the uvcvideo pipeline is to sanitise the control usage\nin this case.\n\n>\n> Perhaps going back to patch 1/6 we need to explain more about how\n> ManualGain is represented to document it's usage clearly there...\n>\n\nI can certainly be more descriptive in the description of the control if needed.\n\n>\n> > + */\n> > + ControlRange gainInfo = controls.infoMap()->at(V4L2_CID_GAIN);\n> > + float requestGain = value.get();\n> > + int32_t gain = requestGain * gainInfo.def().get();\n> > + controls.set(V4L2_CID_GAIN, gain);\n> > }\n> > }\n> >\n> >\n>\n> --\n> Regards\n> --\n> Kieran\n\nRegards,\nNaush","headers":{"Return-Path":"","Received":["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 B293F60429\n\tfor ;\n\tMon, 23 Mar 2020 17:04:15 +0100 (CET)","by mail-lf1-x142.google.com with SMTP id v4so6979750lfo.12\n\tfor ;\n\tMon, 23 Mar 2020 09:04:15 -0700 (PDT)"],"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=TQ24USB38Dlu8cdLVJUceUkNIe754Z57OI/UFB8CdwM=;\n\tb=TtOaZ23XaWCdSAkYD7QHz8ZLKtpuZQcQQeV7/XChbekZa3jiMj7HQ0bUXUVbUHT3Z2\n\tD60imhl0l318GjJSQkwAykKYFfLoO7X131HvXgt1OpMTvSmy4YC7hnwFy3eYi7onnlMa\n\tjowHtKl74qDrKbjRZBAXwkPdfB2MRuzLdTl97kLIdzhToaj/AZiC/ysmcCPI2kYfzMax\n\tYMT64ksT5WSMLK3lQ2KxSC8GuPJ8/aJHvSzdQNngoFrCQTjAdlsIxTqV6UQyeXmDgdUD\n\tuci66rDFk7Ub7ThEjnnt7mPTtzmqEn4mTFIXoscc1ryvWeNI6SFnHVZrkSPgBlatF0zz\n\txe5w==","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=TQ24USB38Dlu8cdLVJUceUkNIe754Z57OI/UFB8CdwM=;\n\tb=HQ7HovSLjLlQ4eZKT4shKWSl6Eor3btiYplYZSTy/+Cu1dfIbR9XH3tlkWBmJo9Vrx\n\tYiJnCGGg+ErGvCqXs68LDj5HaqQdVfxeIAMLrpE57qsr0h3El575eqTj0kJCfwPkznf4\n\tbCyWRgbBDlPwFH+ismhs95gNnIlV33/T/26DR/gKjcLlkbAgqPz8mC4Mnxo8spdUGEGs\n\tNZ4ZKhhRVjvghu4hsOfJ4DEwL0YzvRsTTkefQrt+A4aNEYTuteAZgoa1HLbmctaI0pGl\n\ttW847gpYxHGF3SL+DM31Hgn5CakAazfS9Y4Pts4WPGVI8LPeDgCPJCyBFlZNh5V3Hfkl\n\tp/oA==","X-Gm-Message-State":"ANhLgQ1to1Qen3nHxznwiXe/qy1aszf0CQsVcrwyt1j3ZXDv5vqEzLko\n\tggYE2Yd4T682bxLr/Usex69XK3NYvm+UHIa2uzd/cw==","X-Google-Smtp-Source":"ADFU+vtWq/OcmcjxxAdn+vPAcI02j2nFAnC5/d222eKivcICx6phhd+y5NHoGSZnA2AiHp4EKT1uOAENdUDwJe0KrXQ=","X-Received":"by 2002:a19:ad4c:: with SMTP id\n\ts12mr12094704lfd.109.1584979455057; \n\tMon, 23 Mar 2020 09:04:15 -0700 (PDT)","MIME-Version":"1.0","References":"<20200309123319.630-1-naush@raspberrypi.com>\n\t<20200309123319.630-3-naush@raspberrypi.com>\n\t<383cd00a-be9f-2304-5425-39dd9d232d88@ideasonboard.com>","In-Reply-To":"<383cd00a-be9f-2304-5425-39dd9d232d88@ideasonboard.com>","From":"Naushir Patuck ","Date":"Mon, 23 Mar 2020 16:03:57 +0000","Message-ID":"","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 2/6] libcamera: uvcvideo: Update\n\texposure/gain ctrls set with new units","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","X-List-Received-Date":"Mon, 23 Mar 2020 16:04:15 -0000"}},{"id":4310,"web_url":"https://patchwork.libcamera.org/comment/4310/","msgid":"<20200326154425.GA14181@pendragon.ideasonboard.com>","date":"2020-03-26T15:44:25","subject":"Re: [libcamera-devel] [PATCH v2 2/6] libcamera: uvcvideo: Update\n\texposure/gain ctrls set with new units","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Mon, Mar 23, 2020 at 04:03:57PM +0000, Naushir Patuck wrote:\n> On Fri, 20 Mar 2020 at 14:28, Kieran Bingham wrote:\n> > On 09/03/2020 12:33, Naushir Patuck wrote:\n> > > The ManualExposure control now uses units of 1 micro-second, and UVC\n> > > uses units of 100 micro-seconds. Correctly map the values before\n> > > setting V4L2_CID_EXPOSURE_ABSOLUTE on the V4L2 device.\n> > >\n> > > The ManualGain control now uses floats to allow fractional gain values.\n> > > Since UVC has no explicit gain units, map the default gain value to 1.0\n> > > and linearly map to the requested value.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > src/libcamera/pipeline/uvcvideo.cpp | 17 +++++++++++++++--\n> > > 1 file changed, 15 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > > index 29afb121..aff86803 100644\n> > > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > > @@ -245,9 +245,22 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> > > controls.set(V4L2_CID_SATURATION, value);\n> > > } else if (id == controls::ManualExposure) {\n> > > controls.set(V4L2_CID_EXPOSURE_AUTO, static_cast(1));\n> > > - controls.set(V4L2_CID_EXPOSURE_ABSOLUTE, value);\n> > > + /*\n> > > + * controls::ManualExposure is in units of 1 us, and UVC\n> > > + * expects V4L2_CID_EXPOSURE_ABSOLUTE in units of 100 us.\n> > > + * So divide by 100 when setting the control.\n> >\n> > Sounds fine, but possibly the last line isn't needed, or I'd at least\n> > remove the 'So' ... but that's trivial/minor.\n> \n> Will do.\n> \n> > > + */\n> > > + controls.set(V4L2_CID_EXPOSURE_ABSOLUTE, value.get() / 100);\n> > > } else if (id == controls::ManualGain) {\n> > > - controls.set(V4L2_CID_GAIN, value);\n> > > + /*\n> > > + * controls::ManualGain is specified as an absolute float value.\n> > > + * Map this in a linear way such that 1.0 -> default gain\n> > > + * of the V4L2_CID_GAIN control.\n> >\n> > This doesn't seem very clear to me yet. (I get the intent it's just the\n> > description), and if we're 'mapping' the value like this should it be\n> > documented at the control level?\n> >\n> > Or do you expect this to apply only to UVC? (Presumably if a device\n> > doesn't specify a default gain, it's just '1.0' ?)\n> \n> This mapping is explicitly for UVC.\n> \n> The ManualExposure control is meant to be an explicit exposure time to\n> use, and likewise, ManualGain is an explicit gain multiplier applied\n> to all samples. From Laurent's comment on patch v1, he noted that UVC\n> doesn't specify units for gain. I took that to mean that the gain\n> control is just a range-type control to increase the brightness, and\n> the mapping in the uvcvideo pipeline is to sanitise the control usage\n> in this case.\n> \n> > Perhaps going back to patch 1/6 we need to explain more about how\n> > ManualGain is represented to document it's usage clearly there...\n> \n> I can certainly be more descriptive in the description of the control if needed.\n> \n> > > + */\n> > > + ControlRange gainInfo = controls.infoMap()->at(V4L2_CID_GAIN);\n\nYou can make this a\n\n\t\t\t const ControlRange &gainInfo = controls.infoMap()->at(V4L2_CID_GAIN);\n\nto avoid making a copy.\n\nApart from that,\n\nReviewed-by: Laurent Pinchart \n\n> > > + float requestGain = value.get();\n> > > + int32_t gain = requestGain * gainInfo.def().get();\n\nMinor comment, you could possible use\n\n\t\t\tint32_t gain = value.get()\n\t\t\t\t * gainInfo.def().get();\n\nto avoid the requestGain variable.\n\nReviewed-by: Laurent Pinchart \n\n> > > + controls.set(V4L2_CID_GAIN, gain);\n> > > }\n> > > }\n> > >","headers":{"Return-Path":"","Received":["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 8BAFC60414\n\tfor ;\n\tThu, 26 Mar 2020 16:44:30 +0100 (CET)","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 EC9652DC;\n\tThu, 26 Mar 2020 16:44:29 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"irVYENYD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1585237470;\n\tbh=8vm2JacUZM2iofpKOy5g0TsMHvP665mFo3tC0xhUmKo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=irVYENYDjrHJ008kg2rtQ/Hm3V6VL52BsF1SQ0NNjaBznsozj9fkizzOX/aCICIqx\n\ta9tw34DCEAIbOwipZLuEYdC8OwPu+Ah1FLD19H+zmw7tDYgtOKpKRs6SolGymyaY4k\n\tHKxI+IYZaohwlBSXBD+60KyeIndWRAHa4dzNTZxE=","Date":"Thu, 26 Mar 2020 17:44:25 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200326154425.GA14181@pendragon.ideasonboard.com>","References":"<20200309123319.630-1-naush@raspberrypi.com>\n\t<20200309123319.630-3-naush@raspberrypi.com>\n\t<383cd00a-be9f-2304-5425-39dd9d232d88@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 2/6] libcamera: uvcvideo: Update\n\texposure/gain ctrls set with new units","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","X-List-Received-Date":"Thu, 26 Mar 2020 15:44:30 -0000"}}]