[{"id":24255,"web_url":"https://patchwork.libcamera.org/comment/24255/","msgid":"","date":"2022-07-31T20:17:44","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2: Pass set colorspace\n\tflags","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Fri, Jul 29, 2022 at 08:20:42PM +0530, Umang Jain via libcamera-devel wrote:\n> The colorspace fields as read-only from an application point of view,\n\ns/as read-only/are read-only/\n\n> both on video devices and on subdevs, unless the\n> V4L2_PIX_FMT_FLAG_SET_CSC or V4L2_MBUS_FRAMEFMT_SET_CSC flags\n> (respectively) are set when calling the S_FMT ioctl.\n> \n> Signed-off-by: Umang Jain \n> ---\n> src/libcamera/v4l2_subdevice.cpp | 3 ++-\n> src/libcamera/v4l2_videodevice.cpp | 6 ++++--\n> 2 files changed, 6 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 98a3911a..815c676e 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -459,7 +459,8 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> \tsubdevFmt.format.height = format->size.height;\n> \tsubdevFmt.format.code = format->mbus_code;\n> \tsubdevFmt.format.field = V4L2_FIELD_NONE;\n> -\tfromColorSpace(format->colorSpace, subdevFmt.format);\n> +\tif (fromColorSpace(format->colorSpace, subdevFmt.format) == 0)\n\nIf format->colorSpace is nullopt, should be set the CSC flag ?\n\n> +\t\tsubdevFmt.format.flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;\n\nThe flag is meant for source pads only, can you avoid setting it on sink\npads ?\n\n> \n> \tint ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);\n> \tif (ret) {\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index 63911339..a969f7fa 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -940,7 +940,8 @@ int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set)\n> \tpix->pixelformat = format->fourcc;\n> \tpix->num_planes = format->planesCount;\n> \tpix->field = V4L2_FIELD_NONE;\n> -\tfromColorSpace(format->colorSpace, *pix);\n> +\tif (fromColorSpace(format->colorSpace, *pix) == 0)\n> +\t\tpix->flags |= V4L2_PIX_FMT_FLAG_SET_CSC;\n\nSimilarly, the CSC flag is meant for capture devices only.\n\n> \n> \tASSERT(pix->num_planes <= std::size(pix->plane_fmt));\n> \n> @@ -1010,7 +1011,8 @@ int V4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set)\n> \tpix->pixelformat = format->fourcc;\n> \tpix->bytesperline = format->planes[0].bpl;\n> \tpix->field = V4L2_FIELD_NONE;\n> -\tfromColorSpace(format->colorSpace, *pix);\n> +\tif (fromColorSpace(format->colorSpace, *pix) == 0)\n> +\t\tpix->flags |= V4L2_PIX_FMT_FLAG_SET_CSC;\n> \n> \tret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);\n> \tif (ret) {","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 CB214C3275\n\tfor ;\n\tSun, 31 Jul 2022 20:17:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3B24063312;\n\tSun, 31 Jul 2022 22:17:51 +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 8B603603E9\n\tfor ;\n\tSun, 31 Jul 2022 22:17:49 +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 D6980415;\n\tSun, 31 Jul 2022 22:17:48 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659298671;\n\tbh=Dkf4/6ZaLeGswFpf+ojS0txtgbbzY+hdoeM8SqgBDwI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Kf42+FWi9xs/R+c2xutxOGeHpovG/9LJxYFPmHdKuZVOQHY3jUZLB0YhvESd/IW5o\n\tlPbVWNS560KaKaSqKlKVy0Hhs8KJHZe6kbNhJ0rPEYmhAfNLFXyMqf2WG8LusG/7TZ\n\t6hJFndep3obmIUNeToQmcSk8+ATnq3KZ74DZdOICvW55DlsjN1j3ha/Qf9z0zKfevI\n\tzx7pXzW/T6qxArjpV9uA7Z+wODcWLkecArmWOWDxi5CuvIvx9hJtKgojesgYMmo91H\n\tcyz1OLxNcDYSTwD7sJjyne9lPtFfg7l5cCIOTVzL9916Sxt3fJ9SSTwSEmv2VZZf8p\n\tZVn3b7sDz1how==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659298669;\n\tbh=Dkf4/6ZaLeGswFpf+ojS0txtgbbzY+hdoeM8SqgBDwI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MJRSpr/4Cq7pEwX6vw9ANjn8cmR9T/zkDzOa7VGtgVw3ErUm2sz6iLI5gAT75JKjA\n\tWTd0tPSO1ANBoqzu16m4RlmJlU2lasTUDjkMZMrLd5AjTDZOxTpzlFR8wDWEafKe3y\n\tH9sMpUlHe9QZp/0glsJn+NBVA+czoebCiZXdbG3A="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"MJRSpr/4\"; dkim-atps=neutral","Date":"Sun, 31 Jul 2022 23:17:44 +0300","To":"Umang Jain ","Message-ID":"","References":"<20220729145042.531757-1-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220729145042.531757-1-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2: Pass set colorspace\n\tflags","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"rishikeshdonadkar@gmail.com, libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24258,"web_url":"https://patchwork.libcamera.org/comment/24258/","msgid":"<6d39128a-f7c3-b1e7-aac0-b422cc623ad9@ideasonboard.com>","date":"2022-08-01T06:45:33","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2: Pass set colorspace\n\tflags","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 8/1/22 01:47, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> Thank you for the patch.\n>\n> On Fri, Jul 29, 2022 at 08:20:42PM +0530, Umang Jain via libcamera-devel wrote:\n>> The colorspace fields as read-only from an application point of view,\n> s/as read-only/are read-only/\n>\n>> both on video devices and on subdevs, unless the\n>> V4L2_PIX_FMT_FLAG_SET_CSC or V4L2_MBUS_FRAMEFMT_SET_CSC flags\n>> (respectively) are set when calling the S_FMT ioctl.\n>>\n>> Signed-off-by: Umang Jain\n>> ---\n>> src/libcamera/v4l2_subdevice.cpp | 3 ++-\n>> src/libcamera/v4l2_videodevice.cpp | 6 ++++--\n>> 2 files changed, 6 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n>> index 98a3911a..815c676e 100644\n>> --- a/src/libcamera/v4l2_subdevice.cpp\n>> +++ b/src/libcamera/v4l2_subdevice.cpp\n>> @@ -459,7 +459,8 @@ intV4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n>> \tsubdevFmt.format.height = format->size.height;\n>> \tsubdevFmt.format.code = format->mbus_code;\n>> \tsubdevFmt.format.field = V4L2_FIELD_NONE;\n>> -\tfromColorSpace(format->colorSpace, subdevFmt.format);\n>> +\tif (fromColorSpace(format->colorSpace, subdevFmt.format) == 0)\n> If format->colorSpace is nullopt, should be set the CSC flag ?\n\n\nGood question, I remember I was wondering that as well...\n\nThe documentation mentions:\n\n'''\n\n|V4L2_MBUS_FRAMEFMT_SET_CSC| then the application can set this field on \nthe source pad to request a specific colorspace for the media bus data\n\n'''\n\nSo it seems flag is used to request a \"specific\" colorspace. So \n*_DEFAULT are specific, I don't think so.\n\nSo I think if format->colorSpace is nullopt, we should avoid setting \nthis flag, unless, we start mapping to *_DEFAULT to some kind of \nspecifics in the future (in libcamera).\n\n>\n>> +\t\tsubdevFmt.format.flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;\n> The flag is meant for source pads only, can you avoid setting it on sink\n> pads ?\n\n\nack.\n\n>\n>> \n>> \tint ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);\n>> \tif (ret) {\n>> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n>> index 63911339..a969f7fa 100644\n>> --- a/src/libcamera/v4l2_videodevice.cpp\n>> +++ b/src/libcamera/v4l2_videodevice.cpp\n>> @@ -940,7 +940,8 @@ intV4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set)\n>> \tpix->pixelformat = format->fourcc;\n>> \tpix->num_planes = format->planesCount;\n>> \tpix->field = V4L2_FIELD_NONE;\n>> -\tfromColorSpace(format->colorSpace, *pix);\n>> +\tif (fromColorSpace(format->colorSpace, *pix) == 0)\n>> +\t\tpix->flags |= V4L2_PIX_FMT_FLAG_SET_CSC;\n> Similarly, the CSC flag is meant for capture devices only.\n\n\nack\n\n>\n>> \n>> \tASSERT(pix->num_planes <=std::size(pix->plane_fmt));\n>> \n>> @@ -1010,7 +1011,8 @@ intV4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set)\n>> \tpix->pixelformat = format->fourcc;\n>> \tpix->bytesperline = format->planes[0].bpl;\n>> \tpix->field = V4L2_FIELD_NONE;\n>> -\tfromColorSpace(format->colorSpace, *pix);\n>> +\tif (fromColorSpace(format->colorSpace, *pix) == 0)\n>> +\t\tpix->flags |= V4L2_PIX_FMT_FLAG_SET_CSC;\n>> \n>> \tret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);\n>> \tif (ret) {","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 5F9B8BE173\n\tfor ;\n\tMon, 1 Aug 2022 06:45:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8B11763312;\n\tMon, 1 Aug 2022 08:45:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E7C41603F2\n\tfor ;\n\tMon, 1 Aug 2022 08:45:39 +0200 (CEST)","from [IPV6:2401:4900:1f3e:f7a:bc8f:12ed:b45f:c35d] (unknown\n\t[IPv6:2401:4900:1f3e:f7a:bc8f:12ed:b45f:c35d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 838F648F;\n\tMon, 1 Aug 2022 08:45:38 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659336341;\n\tbh=k5PusXER121chNGEVnLxHgzoo4ds6nkm3GNK5A7PW/Y=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=c1l+kCo9LhCI0k7f2AJfKaeocXAatWT5mlbC3fSQSmLyLaExpCYLZciXI30zGf4CC\n\tqlGr2wvMLLKl6BSvuJA+Kd31nzk1QVL03lfZwJn0DisA6objFrgekyUVP7totqaXdQ\n\tUXH0kLtUT8M0f+Pawwup8g7w4RloI2ApvwwKSUcoXXJBrn98eVLfSgk3A5cnxdHm2P\n\tLgdStseaDBSP0x2FNy+baniLjYYnR6Y9UTlA1mjylFVgEDzOM/8rOiwKiRGUnloNOf\n\tkZxIPIwA9oEbqTIqELwWlhN2K0QQ2SpKr03To7x/H4GJN4Y+dMyP15sk8AaUYtN4lg\n\tt3d9t5kJRy1eg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659336339;\n\tbh=k5PusXER121chNGEVnLxHgzoo4ds6nkm3GNK5A7PW/Y=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=n/CFpSqaVXPoVI3/hC7HElm5IsgMvGKnhKEfwvnj0j8+kOvkEFqWUYRDyLCvjzFqU\n\tT9MHYnyzqn/fjPzUybj+C0lw3Dzq2Kd0Cuzf7ZpMa5nUPnAT53PxUcQMcD45f2jEDD\n\tHvGc4/dOlhqopvpsFFwIJ0TE7UqzZ2HIY7HSHMpk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"n/CFpSqa\"; dkim-atps=neutral","Content-Type":"multipart/alternative;\n\tboundary=\"------------8rHLZ9S60lzPSgHn59bG2UQH\"","Message-ID":"<6d39128a-f7c3-b1e7-aac0-b422cc623ad9@ideasonboard.com>","Date":"Mon, 1 Aug 2022 12:15:33 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Laurent Pinchart ","References":"<20220729145042.531757-1-umang.jain@ideasonboard.com>\n\t","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2: Pass set colorspace\n\tflags","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Cc":"rishikeshdonadkar@gmail.com, libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24259,"web_url":"https://patchwork.libcamera.org/comment/24259/","msgid":"","date":"2022-08-01T07:29:50","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2: Pass set colorspace\n\tflags","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Mon, Aug 01, 2022 at 12:15:33PM +0530, Umang Jain wrote:\n> On 8/1/22 01:47, Laurent Pinchart wrote:\n> > On Fri, Jul 29, 2022 at 08:20:42PM +0530, Umang Jain via libcamera-devel wrote:\n> >> The colorspace fields as read-only from an application point of view,\n> > \n> > s/as read-only/are read-only/\n> >\n> >> both on video devices and on subdevs, unless the\n> >> V4L2_PIX_FMT_FLAG_SET_CSC or V4L2_MBUS_FRAMEFMT_SET_CSC flags\n> >> (respectively) are set when calling the S_FMT ioctl.\n> >>\n> >> Signed-off-by: Umang Jain\n> >> ---\n> >> src/libcamera/v4l2_subdevice.cpp | 3 ++-\n> >> src/libcamera/v4l2_videodevice.cpp | 6 ++++--\n> >> 2 files changed, 6 insertions(+), 3 deletions(-)\n> >>\n> >> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> >> index 98a3911a..815c676e 100644\n> >> --- a/src/libcamera/v4l2_subdevice.cpp\n> >> +++ b/src/libcamera/v4l2_subdevice.cpp\n> >> @@ -459,7 +459,8 @@ intV4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> >> \tsubdevFmt.format.height = format->size.height;\n> >> \tsubdevFmt.format.code = format->mbus_code;\n> >> \tsubdevFmt.format.field = V4L2_FIELD_NONE;\n> >> -\tfromColorSpace(format->colorSpace, subdevFmt.format);\n> >> +\tif (fromColorSpace(format->colorSpace, subdevFmt.format) == 0)\n> >\n> > If format->colorSpace is nullopt, should be set the CSC flag ?\n> \n> Good question, I remember I was wondering that as well...\n> \n> The documentation mentions:\n> \n> '''\n> \n> |V4L2_MBUS_FRAMEFMT_SET_CSC| then the application can set this field on \n> the source pad to request a specific colorspace for the media bus data\n> \n> '''\n> \n> So it seems flag is used to request a \"specific\" colorspace. So \n> *_DEFAULT are specific, I don't think so.\n> \n> So I think if format->colorSpace is nullopt, we should avoid setting \n> this flag, unless, we start mapping to *_DEFAULT to some kind of \n> specifics in the future (in libcamera).\n\nIt's probably harmless to set the CSC flag with all the colorspace\nfields to *_DEFAULT, as it means the driver should then not modify what\nit does by default. I just feel a bit uncomfortable doing so, probably\nbecause there are only two drivers in the kernel that honour the CSC\nflag that it's hard to know what the right thing to do is.\n\n> >> +\t\tsubdevFmt.format.flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;\n> >\n> > The flag is meant for source pads only, can you avoid setting it on sink\n> > pads ?\n> \n> ack.\n> \n> >> \n> >> \tint ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);\n> >> \tif (ret) {\n> >> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> >> index 63911339..a969f7fa 100644\n> >> --- a/src/libcamera/v4l2_videodevice.cpp\n> >> +++ b/src/libcamera/v4l2_videodevice.cpp\n> >> @@ -940,7 +940,8 @@ intV4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set)\n> >> \tpix->pixelformat = format->fourcc;\n> >> \tpix->num_planes = format->planesCount;\n> >> \tpix->field = V4L2_FIELD_NONE;\n> >> -\tfromColorSpace(format->colorSpace, *pix);\n> >> +\tif (fromColorSpace(format->colorSpace, *pix) == 0)\n> >> +\t\tpix->flags |= V4L2_PIX_FMT_FLAG_SET_CSC;\n> >\n> > Similarly, the CSC flag is meant for capture devices only.\n> \n> ack\n> \n> >> \n> >> \tASSERT(pix->num_planes <=std::size(pix->plane_fmt));\n> >> \n> >> @@ -1010,7 +1011,8 @@ intV4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set)\n> >> \tpix->pixelformat = format->fourcc;\n> >> \tpix->bytesperline = format->planes[0].bpl;\n> >> \tpix->field = V4L2_FIELD_NONE;\n> >> -\tfromColorSpace(format->colorSpace, *pix);\n> >> +\tif (fromColorSpace(format->colorSpace, *pix) == 0)\n> >> +\t\tpix->flags |= V4L2_PIX_FMT_FLAG_SET_CSC;\n> >> \n> >> \tret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);\n> >> \tif (ret) {","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 B9FAFC3275\n\tfor ;\n\tMon, 1 Aug 2022 07:29:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3AEB96330F;\n\tMon, 1 Aug 2022 09:29:56 +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 7CFAD603E7\n\tfor ;\n\tMon, 1 Aug 2022 09:29:54 +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 D0A612F3;\n\tMon, 1 Aug 2022 09:29:53 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659338996;\n\tbh=yQK/UESbGR+aKrYpmibCGAQ2M4xhCRq2TYiiXl6P4r8=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=J+7T57Y3Amw0RjMqWW7NelAkBki/1+xIIJRMXh/L4vhQv1N3NtoQe0iwfNN8KpALJ\n\t00Gvu3Y3qxnemQ+AEJIvwbZ6kiD/nEBjKmgTKBIFwgv+8m0xuah3o73f5NxFPEbo04\n\t1XloZQn903ZeCj4AhZCyVLPzojw1bvz6SLP9CX2lq18nl7EjJq3Lp2ZxDlKK8uTbOD\n\tZiUggRTlfWL3jCfV7Ig7dFnDs9uip1eDxjEBAVD0jB0HPkKQT6/yNwvYjRHh2mTgAx\n\t2obLglfqC2pVqjU1XNVV7y46q2d/6ROOgaqXEIp3qfd/fciN0oOAgvCiQPb+jbBlFD\n\tmNj2cIxLCrn7w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659338994;\n\tbh=yQK/UESbGR+aKrYpmibCGAQ2M4xhCRq2TYiiXl6P4r8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=avUWWo963skNlNHqvvhZn33/sp9cjVNpDIMe6SBc5ku/HbzrbrYRES7PkJOQjJKS6\n\t0G0Spw6VmrcKpSlAp0u5aIVHTV+nReny430qJlbkItthpGomtahj+z2lpFsEFM0W3G\n\tz5fDLHfkuoxUkSTemBXyBgqf10G1co7GbsJDioeI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"avUWWo96\"; dkim-atps=neutral","Date":"Mon, 1 Aug 2022 10:29:50 +0300","To":"Umang Jain ","Message-ID":"","References":"<20220729145042.531757-1-umang.jain@ideasonboard.com>\n\t\n\t<6d39128a-f7c3-b1e7-aac0-b422cc623ad9@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<6d39128a-f7c3-b1e7-aac0-b422cc623ad9@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2: Pass set colorspace\n\tflags","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"rishikeshdonadkar@gmail.com, libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24260,"web_url":"https://patchwork.libcamera.org/comment/24260/","msgid":"","date":"2022-08-01T07:34:49","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2: Pass set colorspace\n\tflags","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi,\n\nOn 8/1/22 12:59, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> On Mon, Aug 01, 2022 at 12:15:33PM +0530, Umang Jain wrote:\n>> On 8/1/22 01:47, Laurent Pinchart wrote:\n>>> On Fri, Jul 29, 2022 at 08:20:42PM +0530, Umang Jain via libcamera-devel wrote:\n>>>> The colorspace fields as read-only from an application point of view,\n>>> s/as read-only/are read-only/\n>>>\n>>>> both on video devices and on subdevs, unless the\n>>>> V4L2_PIX_FMT_FLAG_SET_CSC or V4L2_MBUS_FRAMEFMT_SET_CSC flags\n>>>> (respectively) are set when calling the S_FMT ioctl.\n>>>>\n>>>> Signed-off-by: Umang Jain\n>>>> ---\n>>>> src/libcamera/v4l2_subdevice.cpp | 3 ++-\n>>>> src/libcamera/v4l2_videodevice.cpp | 6 ++++--\n>>>> 2 files changed, 6 insertions(+), 3 deletions(-)\n>>>>\n>>>> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n>>>> index 98a3911a..815c676e 100644\n>>>> --- a/src/libcamera/v4l2_subdevice.cpp\n>>>> +++ b/src/libcamera/v4l2_subdevice.cpp\n>>>> @@ -459,7 +459,8 @@ intV4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n>>>> \tsubdevFmt.format.height = format->size.height;\n>>>> \tsubdevFmt.format.code = format->mbus_code;\n>>>> \tsubdevFmt.format.field = V4L2_FIELD_NONE;\n>>>> -\tfromColorSpace(format->colorSpace, subdevFmt.format);\n>>>> +\tif (fromColorSpace(format->colorSpace, subdevFmt.format) == 0)\n>>> If format->colorSpace is nullopt, should be set the CSC flag ?\n>> Good question, I remember I was wondering that as well...\n>>\n>> The documentation mentions:\n>>\n>> '''\n>>\n>> |V4L2_MBUS_FRAMEFMT_SET_CSC| then the application can set this field on\n>> the source pad to request a specific colorspace for the media bus data\n>>\n>> '''\n>>\n>> So it seems flag is used to request a \"specific\" colorspace. So\n>> *_DEFAULT are specific, I don't think so.\n>>\n>> So I think if format->colorSpace is nullopt, we should avoid setting\n>> this flag, unless, we start mapping to *_DEFAULT to some kind of\n>> specifics in the future (in libcamera).\n> It's probably harmless to set the CSC flag with all the colorspace\n> fields to *_DEFAULT, as it means the driver should then not modify what\n> it does by default. I just feel a bit uncomfortable doing so, probably\n> because there are only two drivers in the kernel that honour the CSC\n> flag that it's hard to know what the right thing to do is.\n\nIndeed, hard to infer as the flag is not currently widely used.\n\nLooking from libcamera perspective, it doesn't make sense to set the \nflag with colorspace is nullopt.\n\nTo someone oblivious to the kernel in general, it might be confusing to \nlook setting the flags, when colorspace is nullopt.\n\n>\n>>>> +\t\tsubdevFmt.format.flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;\n>>> The flag is meant for source pads only, can you avoid setting it on sink\n>>> pads ?\n>> ack.\n>>\n>>>> \n>>>> \tint ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);\n>>>> \tif (ret) {\n>>>> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n>>>> index 63911339..a969f7fa 100644\n>>>> --- a/src/libcamera/v4l2_videodevice.cpp\n>>>> +++ b/src/libcamera/v4l2_videodevice.cpp\n>>>> @@ -940,7 +940,8 @@ intV4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set)\n>>>> \tpix->pixelformat = format->fourcc;\n>>>> \tpix->num_planes = format->planesCount;\n>>>> \tpix->field = V4L2_FIELD_NONE;\n>>>> -\tfromColorSpace(format->colorSpace, *pix);\n>>>> +\tif (fromColorSpace(format->colorSpace, *pix) == 0)\n>>>> +\t\tpix->flags |= V4L2_PIX_FMT_FLAG_SET_CSC;\n>>> Similarly, the CSC flag is meant for capture devices only.\n>> ack\n>>\n>>>> \n>>>> \tASSERT(pix->num_planes <=std::size(pix->plane_fmt));\n>>>> \n>>>> @@ -1010,7 +1011,8 @@ intV4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set)\n>>>> \tpix->pixelformat = format->fourcc;\n>>>> \tpix->bytesperline = format->planes[0].bpl;\n>>>> \tpix->field = V4L2_FIELD_NONE;\n>>>> -\tfromColorSpace(format->colorSpace, *pix);\n>>>> +\tif (fromColorSpace(format->colorSpace, *pix) == 0)\n>>>> +\t\tpix->flags |= V4L2_PIX_FMT_FLAG_SET_CSC;\n>>>> \n>>>> \tret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);\n>>>> \tif (ret) {","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 AB60FBE173\n\tfor ;\n\tMon, 1 Aug 2022 07:34:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 10FBE63312;\n\tMon, 1 Aug 2022 09:34:58 +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 3520A603E7\n\tfor ;\n\tMon, 1 Aug 2022 09:34:57 +0200 (CEST)","from [IPV6:2401:4900:1f3e:f7a:bc8f:12ed:b45f:c35d] (unknown\n\t[IPv6:2401:4900:1f3e:f7a:bc8f:12ed:b45f:c35d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D5FE72F3;\n\tMon, 1 Aug 2022 09:34:55 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659339298;\n\tbh=9xjRZko6bS8ArpgpdiowKqKbjYGgKcQx2yEFaS/K41k=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=1FZwVzjHBNmbGVjjHjLZsLUE3G6lzSDHtgSsDzGQasj9w5j7IHDcGHuapF9HTUg+w\n\tD4tsAwUIfwsTEi0W7UxBbxhXsMy6u005RQMiGOqBGDjrqyYLU9gZ5BLX458HjCFTO/\n\tIdlMJ+QYZUxNjxzs8JWFx6X/VJdtdqgAAqR5llyOHk+3EScyvTuIoIMBDxkrdnAD6n\n\tvBObSub98k3nG75wikkgINxjag5+PwXLAALmGptEUr0uF9ZpPwB/wNdTpYxYMp+rhQ\n\tJN1mY2TjAGB3ocjh4ekIiqltOC0BOTfNLPQ59j+K0Bp8voLOtEjw04W8dbXf7LX+RL\n\tmuTRxdWSsOsmg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659339296;\n\tbh=9xjRZko6bS8ArpgpdiowKqKbjYGgKcQx2yEFaS/K41k=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=wUmSfXK0OPFJaHPmwjVPMSuuAoY3EJd0tHFM646B+pT24Pq7WHQgiAQ81bx8hMIxA\n\tSqkYp17fjcxaAXOwU1btQ8APHDkKvqgchkZCD9BXF5PHu7ZbxIudB8bY8PiZUQJp/b\n\t1LufVNMeHQBtF1a8sdoTM/xHbz6Fa5i8u1da9r1s="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"wUmSfXK0\"; dkim-atps=neutral","Message-ID":"","Date":"Mon, 1 Aug 2022 13:04:49 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Laurent Pinchart ","References":"<20220729145042.531757-1-umang.jain@ideasonboard.com>\n\t\n\t<6d39128a-f7c3-b1e7-aac0-b422cc623ad9@ideasonboard.com>\n\t","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2: Pass set colorspace\n\tflags","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Cc":"rishikeshdonadkar@gmail.com, libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]