[{"id":4766,"web_url":"https://patchwork.libcamera.org/comment/4766/","msgid":"","date":"2020-05-08T11:09:17","subject":"Re: [libcamera-devel] [PATCH v10 06/13] media: v4l2-ctrls: Add\n\tcamera orientation and rotation","submitter":{"id":43,"url":"https://patchwork.libcamera.org/api/people/43/","name":"Hans Verkuil","email":"hverkuil-cisco@xs4all.nl"},"content":"On 08/05/2020 12:01, Jacopo Mondi wrote:\n> Add support for the newly defined V4L2_CID_CAMERA_ORIENTATION\n> and V4L2_CID_CAMERA_SENSOR_ROTATION read-only controls used to report\n> the camera device mounting position and orientation respectively.\n> \n> Reviewed-by: Laurent Pinchart \n> Signed-off-by: Jacopo Mondi \n> ---\n> drivers/media/v4l2-core/v4l2-ctrls.c | 15 +++++++++++++++\n> include/uapi/linux/v4l2-controls.h | 7 +++++++\n> 2 files changed, 22 insertions(+)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c\n> index 1c617b42a944d..97765a57654d2 100644\n> --- a/drivers/media/v4l2-core/v4l2-ctrls.c\n> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c\n> @@ -583,6 +583,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)\n> \t\t\"Annex B Start Code\",\n> \t\tNULL,\n> \t};\n> +\tstatic const char * const camera_orientation[] = {\n> +\t\t\"Front Camera\",\n> +\t\t\"Back Camera\",\n> +\t\t\"External Camera\",\n\nYou can drop 'Camera' here. The name of the control itself already specifies that\nit is about the camera orientation, so that does (and should) not be repeated here.\n\n> +\t\tNULL,\n> +\t};\n> \n> \tswitch (id) {\n> \tcase V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:\n> @@ -708,6 +714,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)\n> \t\treturn hevc_decode_mode;\n> \tcase V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:\n> \t\treturn hevc_start_code;\n> +\tcase V4L2_CID_CAMERA_ORIENTATION:\n> +\t\treturn camera_orientation;\n> \tdefault:\n> \t\treturn NULL;\n> \t}\n> @@ -1020,6 +1028,8 @@ const char *v4l2_ctrl_get_name(u32 id)\n> \tcase V4L2_CID_PAN_SPEED:\t\treturn \"Pan, Speed\";\n> \tcase V4L2_CID_TILT_SPEED:\t\treturn \"Tilt, Speed\";\n> \tcase V4L2_CID_UNIT_CELL_SIZE:\t\treturn \"Unit Cell Size\";\n> +\tcase V4L2_CID_CAMERA_ORIENTATION:\treturn \"Camera Orientation\";\n> +\tcase V4L2_CID_CAMERA_SENSOR_ROTATION:\treturn \"Camera Sensor Rotation\";\n> \n> \t/* FM Radio Modulator controls */\n> \t/* Keep the order of the 'case's the same as in v4l2-controls.h! */\n> @@ -1295,6 +1305,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,\n> \tcase V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:\n> \t\t*type = V4L2_CTRL_TYPE_MENU;\n> \t\tbreak;\n> +\tcase V4L2_CID_CAMERA_ORIENTATION:\n\nJust move this up to just below 'case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE'\nand add 'case V4L2_CID_CAMERA_ORIENTATION:' to the switch at the end of\nthis function that sets the READ_ONLY flag.\n\nI.e. this function has two switches: one that sets the type and one at the\nend that sets the flags. I see that the code is not entirely consistent anymore,\nbut we should try to keep to how it is supposed to be used.\n\n> +\t\t*type = V4L2_CTRL_TYPE_MENU;\n> +\t\t*flags |= V4L2_CTRL_FLAG_READ_ONLY;\n> +\t\tbreak;\n> \tcase V4L2_CID_LINK_FREQ:\n> \t\t*type = V4L2_CTRL_TYPE_INTEGER_MENU;\n> \t\tbreak;\n> @@ -1346,6 +1360,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,\n> \t\tbreak;\n> \tcase V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:\n> \tcase V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:\n> +\tcase V4L2_CID_CAMERA_SENSOR_ROTATION:\n> \t\t*type = V4L2_CTRL_TYPE_INTEGER;\n> \t\t*flags |= V4L2_CTRL_FLAG_READ_ONLY;\n\nSame here.\n\n> \t\tbreak;\n> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h\n> index 0ba1005c96519..3da37c9cf5046 100644\n> --- a/include/uapi/linux/v4l2-controls.h\n> +++ b/include/uapi/linux/v4l2-controls.h\n> @@ -923,6 +923,13 @@ enum v4l2_auto_focus_range {\n> #define V4L2_CID_PAN_SPEED\t\t\t(V4L2_CID_CAMERA_CLASS_BASE+32)\n> #define V4L2_CID_TILT_SPEED\t\t\t(V4L2_CID_CAMERA_CLASS_BASE+33)\n> \n> +#define V4L2_CID_CAMERA_ORIENTATION\t\t(V4L2_CID_CAMERA_CLASS_BASE+34)\n> +#define V4L2_ORIENTATION_FRONT\t\t\t0\n> +#define V4L2_ORIENTATION_BACK\t\t\t1\n> +#define V4L2_ORIENTATION_EXTERNAL\t\t2\n> +\n> +#define V4L2_CID_CAMERA_SENSOR_ROTATION\t\t(V4L2_CID_CAMERA_CLASS_BASE+35)\n> +\n> /* FM Modulator class control IDs */\n> \n> #define V4L2_CID_FM_TX_CLASS_BASE\t\t(V4L2_CTRL_CLASS_FM_TX | 0x900)\n> \n\nRegards,\n\n\tHans","headers":{"Return-Path":"","Received":["from lb1-smtp-cloud7.xs4all.net (lb1-smtp-cloud7.xs4all.net\n\t[194.109.24.24])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D357F600EA\n\tfor ;\n\tFri, 8 May 2020 13:09:21 +0200 (CEST)","from cust-b5b5937f ([IPv6:fc0c:c16d:66b8:757f:c639:739b:9d66:799d])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid X0sYjdud6tKAsX0sbje8ud; Fri, 08 May 2020 13:09:21 +0200"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=xs4all.nl header.i=@xs4all.nl\n\theader.b=\"pss3tNw5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s1;\n\tt=1588936161; bh=hj/vj/D3dynIwv0WWpFsMOwC18NJjLLUtWJpBPd5v6w=;\n\th=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From:\n\tSubject;\n\tb=pss3tNw5/HKD4xXjWlTJfWbcQDDSpy1ff50T7B5DikHCbzRFIqmK7y/cj4YE2OUhK\n\tyYcyqdEXNbmpWTTB8zL4dHSKRDaJAcnToaIX5DSLgO8vFpa/1Gd24TeZKQymgWPiQj\n\t001L/a/d3Nl8CNNyo62ecxB8z1/nyHzaemtpk/tpbzSL6bSuWzwRaJDGb+yOWkeA9y\n\tYrvP1GxussnbKGwEismGNGrez3gnMJJYejXztkj0AWXgWdaFEYB2dn/0EfAck1n2mc\n\tRhFI8uNgKnaMfUf3l/ToSNmCtFFocSZtSmY9dJi4xyOSMcyuNWaJDO/QioRsRWJsVz\n\tlZzBEepSW8u2w==","To":"Jacopo Mondi ,\n\t\"open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)\"\n\t, libcamera-devel@lists.libcamera.org,\n\tMauro Carvalho Chehab ,\n\tSakari Ailus ,\n\tLaurent Pinchart ","Cc":"tfiga@google.com, pavel@ucw.cz","References":"<20200508100158.3437161-1-jacopo@jmondi.org>\n\t<20200508100158.3437161-7-jacopo@jmondi.org>","From":"Hans Verkuil ","Message-ID":"","Date":"Fri, 8 May 2020 13:09:17 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.5.0","MIME-Version":"1.0","In-Reply-To":"<20200508100158.3437161-7-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfPktsdAgYxR5evL9XF0e7fcADex9wbRrWgKYq6jZaE4DpNu15XzGc7v8o2rPR9N5kC+4WT6gxxCWqcfZZJGUNwJtHJvmyF0sVut24yFukvYaZ3rGXDZz\n\tiWK5mfp8ibIwN9VfFeEjXYvHUeu7mAuA5SIwGSiDZMDp2Uqm6lggZxt6l8hGkpU5e2+Fbjm+RAUEuNscoj9Mhmd/tQ0RRlXshvt4ipbWUxNrEmbYx0/+afFn\n\tgGwg0GYNrYpCNVMG4ub+CDDNVcLu+T4CvQ6awlBJ5yfsYIGu0BCbvMTiE2LbmbFtoxHFTtdp/lAsGoaxif17e2hAd5b3+Zqv7wM9nEL1gL+R6mTG7kunLFtW\n\tDqfzl0DtXWmXiyesiJGDo8eM4e/vvPj6JIipqevVZ437ubLON5jOnxaBvOTjM4APaAxok+Wnn/G6m0u7y7lkZ4MdUeWyrw==","Subject":"Re: [libcamera-devel] [PATCH v10 06/13] media: v4l2-ctrls: Add\n\tcamera orientation and rotation","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, 08 May 2020 11:09:22 -0000"}}]