[{"id":5243,"web_url":"https://patchwork.libcamera.org/comment/5243/","msgid":"<20200617145931.jqmwlaxbypzkxcqa@uno.localdomain>","date":"2020-06-17T14:59:31","subject":"Re: [libcamera-devel] [PATCH 07/15] v4l2: v4l2_camera_proxy: Fix\n\tv4l2-compliance format tests","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Tue, Jun 16, 2020 at 10:12:36PM +0900, Paul Elder wrote:\n> Fix v4l2-compliance format tests, for enum_fmt, try_fmt, and g/s_fmt.\n>\n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> ---\n>  src/v4l2/v4l2_camera_proxy.cpp | 14 ++++++++++++--\n>  1 file changed, 12 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index a5fa478..fd2785f 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -179,6 +179,9 @@ void V4L2CameraProxy::setFmtFromConfig(StreamConfiguration &streamConfig)\n>  \t\t\t  curV4L2Format_.fmt.pix.width,\n>  \t\t\t  curV4L2Format_.fmt.pix.height);\n>  \tcurV4L2Format_.fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;\n> +\tcurV4L2Format_.fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;\n> +\tcurV4L2Format_.fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_601;\n> +\tcurV4L2Format_.fmt.pix.quantization = V4L2_QUANTIZATION_LIM_RANGE;\n\nAny specific reason why you chose these encoding and quantization ?\nWhy not simply use the _DEFAULT ones ?\n\n>  }\n>\n>  unsigned int V4L2CameraProxy::calculateSizeImage(StreamConfiguration &streamConfig)\n> @@ -283,11 +286,15 @@ int V4L2CameraProxy::vidioc_enum_fmt(int fd, struct v4l2_fmtdesc *arg)\n>  \t    arg->index >= streamConfig_.formats().pixelformats().size())\n>  \t\treturn -EINVAL;\n>\n> +\t/* \\todo Set V4L2_FMT_FLAG_COMPRESSED for compressed formats. */\n> +\targ->flags = 0;\n>  \t/* \\todo Add map from format to description. */\n> -\tutils::strlcpy(reinterpret_cast<char *>(arg->description), \"Video Format Description\",\n> -\t\t       sizeof(arg->description));\n> +\tutils::strlcpy(reinterpret_cast<char *>(arg->description),\n> +\t\t       \"Video Format Description\", sizeof(arg->description));\n>  \targ->pixelformat = drmToV4L2(streamConfig_.formats().pixelformats()[arg->index]);\n>\n> +\tmemset(arg->reserved, 0, sizeof(arg->reserved));\n> +\n\nIf I were to be picky, I would have split this to a separate patch.\nAt least please expand the commit message to mention what you're\nfixing\n\nThanks\n  j\n\n>  \treturn 0;\n>  }\n>\n> @@ -330,6 +337,9 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)\n>  \t\t\t\t\t      arg->fmt.pix.width,\n>  \t\t\t\t\t      arg->fmt.pix.height);\n>  \targ->fmt.pix.colorspace   = V4L2_COLORSPACE_SRGB;\n> +\targ->fmt.pix.priv         = V4L2_PIX_FMT_PRIV_MAGIC;\n> +\targ->fmt.pix.ycbcr_enc    = V4L2_YCBCR_ENC_601;\n> +\targ->fmt.pix.quantization = V4L2_QUANTIZATION_LIM_RANGE;\n>  }\n>\n>  int V4L2CameraProxy::vidioc_s_fmt(int fd, struct v4l2_format *arg)\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":"<jacopo@jmondi.org>","Received":["from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 88CB7603BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 17 Jun 2020 16:56:06 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 20A281C0005;\n\tWed, 17 Jun 2020 14:56:05 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 17 Jun 2020 16:59:31 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200617145931.jqmwlaxbypzkxcqa@uno.localdomain>","References":"<20200616131244.70308-1-paul.elder@ideasonboard.com>\n\t<20200616131244.70308-8-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200616131244.70308-8-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 07/15] v4l2: v4l2_camera_proxy: Fix\n\tv4l2-compliance format tests","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 17 Jun 2020 14:56:06 -0000"}},{"id":5256,"web_url":"https://patchwork.libcamera.org/comment/5256/","msgid":"<20200617154911.GK5838@pendragon.ideasonboard.com>","date":"2020-06-17T15:49:11","subject":"Re: [libcamera-devel] [PATCH 07/15] v4l2: v4l2_camera_proxy: Fix\n\tv4l2-compliance format tests","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Jun 17, 2020 at 04:59:31PM +0200, Jacopo Mondi wrote:\n> On Tue, Jun 16, 2020 at 10:12:36PM +0900, Paul Elder wrote:\n> > Fix v4l2-compliance format tests, for enum_fmt, try_fmt, and g/s_fmt.\n> >\n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > ---\n> >  src/v4l2/v4l2_camera_proxy.cpp | 14 ++++++++++++--\n> >  1 file changed, 12 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > index a5fa478..fd2785f 100644\n> > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > @@ -179,6 +179,9 @@ void V4L2CameraProxy::setFmtFromConfig(StreamConfiguration &streamConfig)\n> >  \t\t\t  curV4L2Format_.fmt.pix.width,\n> >  \t\t\t  curV4L2Format_.fmt.pix.height);\n> >  \tcurV4L2Format_.fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;\n> > +\tcurV4L2Format_.fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;\n\nAh, here's the V4L2_PIX_FMT_PRIV_MAGIC I requested in a previous patch\n:-)\n\n> > +\tcurV4L2Format_.fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_601;\n> > +\tcurV4L2Format_.fmt.pix.quantization = V4L2_QUANTIZATION_LIM_RANGE;\n> \n> Any specific reason why you chose these encoding and quantization ?\n> Why not simply use the _DEFAULT ones ?\n\nThe _DEFAULT values are meant for userspace to ask the kernel to pick a\ndefault. The kernel should return real values.\n\n> >  }\n> >\n> >  unsigned int V4L2CameraProxy::calculateSizeImage(StreamConfiguration &streamConfig)\n> > @@ -283,11 +286,15 @@ int V4L2CameraProxy::vidioc_enum_fmt(int fd, struct v4l2_fmtdesc *arg)\n> >  \t    arg->index >= streamConfig_.formats().pixelformats().size())\n> >  \t\treturn -EINVAL;\n> >\n> > +\t/* \\todo Set V4L2_FMT_FLAG_COMPRESSED for compressed formats. */\n> > +\targ->flags = 0;\n> >  \t/* \\todo Add map from format to description. */\n> > -\tutils::strlcpy(reinterpret_cast<char *>(arg->description), \"Video Format Description\",\n> > -\t\t       sizeof(arg->description));\n> > +\tutils::strlcpy(reinterpret_cast<char *>(arg->description),\n> > +\t\t       \"Video Format Description\", sizeof(arg->description));\n> >  \targ->pixelformat = drmToV4L2(streamConfig_.formats().pixelformats()[arg->index]);\n> >\n> > +\tmemset(arg->reserved, 0, sizeof(arg->reserved));\n> > +\n> \n> If I were to be picky, I would have split this to a separate patch.\n> At least please expand the commit message to mention what you're\n> fixing\n\nI'd indeed move the memset to the patch that adds support for\nV4L2_CAP_EXT_PIX_FORMAT. It should be done for all the format-related\nioctls (of not already there).\n\n> >  \treturn 0;\n> >  }\n> >\n> > @@ -330,6 +337,9 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)\n> >  \t\t\t\t\t      arg->fmt.pix.width,\n> >  \t\t\t\t\t      arg->fmt.pix.height);\n> >  \targ->fmt.pix.colorspace   = V4L2_COLORSPACE_SRGB;\n> > +\targ->fmt.pix.priv         = V4L2_PIX_FMT_PRIV_MAGIC;\n> > +\targ->fmt.pix.ycbcr_enc    = V4L2_YCBCR_ENC_601;\n> > +\targ->fmt.pix.quantization = V4L2_QUANTIZATION_LIM_RANGE;\n> >  }\n> >\n> >  int V4L2CameraProxy::vidioc_s_fmt(int fd, struct v4l2_format *arg)","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 11E58603BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 17 Jun 2020 17:49:35 +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 75D41F9;\n\tWed, 17 Jun 2020 17:49:34 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"IkSEb+/1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1592408974;\n\tbh=BCONt2jqXwhMbAEoS7qWl6gxDZPassC8R8JGEFrf+0c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IkSEb+/1k0LPbSNo2IvgyNYlBwvof7JDS7MCgo5yNp91ZfkCoR7HxL1pCgmzVLcVI\n\t0iUhrsqzI5qdbIPJihFZrU8U2L2i5ECuVihXdDAxwWcH+7a2Ptlxdq+Ouhq7fNXoSf\n\txQDcBixv/7WAv+G5a60JEkJpgeLvnl3hOHMEqtbI=","Date":"Wed, 17 Jun 2020 18:49:11 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200617154911.GK5838@pendragon.ideasonboard.com>","References":"<20200616131244.70308-1-paul.elder@ideasonboard.com>\n\t<20200616131244.70308-8-paul.elder@ideasonboard.com>\n\t<20200617145931.jqmwlaxbypzkxcqa@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200617145931.jqmwlaxbypzkxcqa@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 07/15] v4l2: v4l2_camera_proxy: Fix\n\tv4l2-compliance format tests","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 17 Jun 2020 15:49:35 -0000"}},{"id":5258,"web_url":"https://patchwork.libcamera.org/comment/5258/","msgid":"<20200617160121.moh22yxqwjindc65@uno.localdomain>","date":"2020-06-17T16:01:21","subject":"Re: [libcamera-devel] [PATCH 07/15] v4l2: v4l2_camera_proxy: Fix\n\tv4l2-compliance format tests","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Wed, Jun 17, 2020 at 06:49:11PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Wed, Jun 17, 2020 at 04:59:31PM +0200, Jacopo Mondi wrote:\n> > On Tue, Jun 16, 2020 at 10:12:36PM +0900, Paul Elder wrote:\n> > > Fix v4l2-compliance format tests, for enum_fmt, try_fmt, and g/s_fmt.\n> > >\n> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > > ---\n> > >  src/v4l2/v4l2_camera_proxy.cpp | 14 ++++++++++++--\n> > >  1 file changed, 12 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > > index a5fa478..fd2785f 100644\n> > > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > @@ -179,6 +179,9 @@ void V4L2CameraProxy::setFmtFromConfig(StreamConfiguration &streamConfig)\n> > >  \t\t\t  curV4L2Format_.fmt.pix.width,\n> > >  \t\t\t  curV4L2Format_.fmt.pix.height);\n> > >  \tcurV4L2Format_.fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;\n> > > +\tcurV4L2Format_.fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;\n>\n> Ah, here's the V4L2_PIX_FMT_PRIV_MAGIC I requested in a previous patch\n> :-)\n>\n> > > +\tcurV4L2Format_.fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_601;\n> > > +\tcurV4L2Format_.fmt.pix.quantization = V4L2_QUANTIZATION_LIM_RANGE;\n> >\n> > Any specific reason why you chose these encoding and quantization ?\n> > Why not simply use the _DEFAULT ones ?\n>\n> The _DEFAULT values are meant for userspace to ask the kernel to pick a\n> default. The kernel should return real values.\n>\n\nAre you sure ?\n\n$ git grep QUANTIZATION_DEFAULT drivers/media/i2c/ | wc -l\n16\n$ git grep V4L2_YCBCR_ENC_DEFAULT drivers/media/i2c/ | wc -l\n13\n\nand they're all assignements.\n\nThe V4L2 documentation reports:\n\nV4L2_QUANTIZATION_DEFAULT\nUse the default quantization encoding as defined by the colorspace.\nThis is always full range for R’G’B’ (except for the BT.2020\ncolorspace) and HSV. It is usually limited range for Y’CbCr.\n\nV4L2_YCBCR_ENC_DEFAULT\nUse the default Y’CbCr encoding as defined by the colorspace.\n\n\n> > >  }\n> > >\n> > >  unsigned int V4L2CameraProxy::calculateSizeImage(StreamConfiguration &streamConfig)\n> > > @@ -283,11 +286,15 @@ int V4L2CameraProxy::vidioc_enum_fmt(int fd, struct v4l2_fmtdesc *arg)\n> > >  \t    arg->index >= streamConfig_.formats().pixelformats().size())\n> > >  \t\treturn -EINVAL;\n> > >\n> > > +\t/* \\todo Set V4L2_FMT_FLAG_COMPRESSED for compressed formats. */\n> > > +\targ->flags = 0;\n> > >  \t/* \\todo Add map from format to description. */\n> > > -\tutils::strlcpy(reinterpret_cast<char *>(arg->description), \"Video Format Description\",\n> > > -\t\t       sizeof(arg->description));\n> > > +\tutils::strlcpy(reinterpret_cast<char *>(arg->description),\n> > > +\t\t       \"Video Format Description\", sizeof(arg->description));\n> > >  \targ->pixelformat = drmToV4L2(streamConfig_.formats().pixelformats()[arg->index]);\n> > >\n> > > +\tmemset(arg->reserved, 0, sizeof(arg->reserved));\n> > > +\n> >\n> > If I were to be picky, I would have split this to a separate patch.\n> > At least please expand the commit message to mention what you're\n> > fixing\n>\n> I'd indeed move the memset to the patch that adds support for\n> V4L2_CAP_EXT_PIX_FORMAT. It should be done for all the format-related\n> ioctls (of not already there).\n>\n> > >  \treturn 0;\n> > >  }\n> > >\n> > > @@ -330,6 +337,9 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)\n> > >  \t\t\t\t\t      arg->fmt.pix.width,\n> > >  \t\t\t\t\t      arg->fmt.pix.height);\n> > >  \targ->fmt.pix.colorspace   = V4L2_COLORSPACE_SRGB;\n> > > +\targ->fmt.pix.priv         = V4L2_PIX_FMT_PRIV_MAGIC;\n> > > +\targ->fmt.pix.ycbcr_enc    = V4L2_YCBCR_ENC_601;\n> > > +\targ->fmt.pix.quantization = V4L2_QUANTIZATION_LIM_RANGE;\n> > >  }\n> > >\n> > >  int V4L2CameraProxy::vidioc_s_fmt(int fd, struct v4l2_format *arg)\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 41598603BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 17 Jun 2020 17:57:57 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 7216DE0012;\n\tWed, 17 Jun 2020 15:57:56 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 17 Jun 2020 18:01:21 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200617160121.moh22yxqwjindc65@uno.localdomain>","References":"<20200616131244.70308-1-paul.elder@ideasonboard.com>\n\t<20200616131244.70308-8-paul.elder@ideasonboard.com>\n\t<20200617145931.jqmwlaxbypzkxcqa@uno.localdomain>\n\t<20200617154911.GK5838@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200617154911.GK5838@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 07/15] v4l2: v4l2_camera_proxy: Fix\n\tv4l2-compliance format tests","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 17 Jun 2020 15:57:57 -0000"}},{"id":5259,"web_url":"https://patchwork.libcamera.org/comment/5259/","msgid":"<20200617160055.GM5838@pendragon.ideasonboard.com>","date":"2020-06-17T16:00:55","subject":"Re: [libcamera-devel] [PATCH 07/15] v4l2: v4l2_camera_proxy: Fix\n\tv4l2-compliance format tests","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Jun 17, 2020 at 06:01:21PM +0200, Jacopo Mondi wrote:\n> On Wed, Jun 17, 2020 at 06:49:11PM +0300, Laurent Pinchart wrote:\n> > On Wed, Jun 17, 2020 at 04:59:31PM +0200, Jacopo Mondi wrote:\n> >> On Tue, Jun 16, 2020 at 10:12:36PM +0900, Paul Elder wrote:\n> >>> Fix v4l2-compliance format tests, for enum_fmt, try_fmt, and g/s_fmt.\n> >>>\n> >>> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> >>> ---\n> >>>  src/v4l2/v4l2_camera_proxy.cpp | 14 ++++++++++++--\n> >>>  1 file changed, 12 insertions(+), 2 deletions(-)\n> >>>\n> >>> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> >>> index a5fa478..fd2785f 100644\n> >>> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> >>> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> >>> @@ -179,6 +179,9 @@ void V4L2CameraProxy::setFmtFromConfig(StreamConfiguration &streamConfig)\n> >>>  \t\t\t  curV4L2Format_.fmt.pix.width,\n> >>>  \t\t\t  curV4L2Format_.fmt.pix.height);\n> >>>  \tcurV4L2Format_.fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;\n> >>> +\tcurV4L2Format_.fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;\n> >\n> > Ah, here's the V4L2_PIX_FMT_PRIV_MAGIC I requested in a previous patch\n> > :-)\n> >\n> >>> +\tcurV4L2Format_.fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_601;\n> >>> +\tcurV4L2Format_.fmt.pix.quantization = V4L2_QUANTIZATION_LIM_RANGE;\n> >>\n> >> Any specific reason why you chose these encoding and quantization ?\n> >> Why not simply use the _DEFAULT ones ?\n> >\n> > The _DEFAULT values are meant for userspace to ask the kernel to pick a\n> > default. The kernel should return real values.\n> >\n> \n> Are you sure ?\n> \n> $ git grep QUANTIZATION_DEFAULT drivers/media/i2c/ | wc -l\n> 16\n> $ git grep V4L2_YCBCR_ENC_DEFAULT drivers/media/i2c/ | wc -l\n> 13\n> \n> and they're all assignements.\n> \n> The V4L2 documentation reports:\n> \n> V4L2_QUANTIZATION_DEFAULT\n> Use the default quantization encoding as defined by the colorspace.\n> This is always full range for R’G’B’ (except for the BT.2020\n> colorspace) and HSV. It is usually limited range for Y’CbCr.\n> \n> V4L2_YCBCR_ENC_DEFAULT\n> Use the default Y’CbCr encoding as defined by the colorspace.\n\nYou may be right. V4L2_COLORSPACE_DEFAULT is not allowed to be reported\nby drivers, but for the other fields, it may be fine. Let's ask Hans.\n\n> >>>  }\n> >>>\n> >>>  unsigned int V4L2CameraProxy::calculateSizeImage(StreamConfiguration &streamConfig)\n> >>> @@ -283,11 +286,15 @@ int V4L2CameraProxy::vidioc_enum_fmt(int fd, struct v4l2_fmtdesc *arg)\n> >>>  \t    arg->index >= streamConfig_.formats().pixelformats().size())\n> >>>  \t\treturn -EINVAL;\n> >>>\n> >>> +\t/* \\todo Set V4L2_FMT_FLAG_COMPRESSED for compressed formats. */\n> >>> +\targ->flags = 0;\n> >>>  \t/* \\todo Add map from format to description. */\n> >>> -\tutils::strlcpy(reinterpret_cast<char *>(arg->description), \"Video Format Description\",\n> >>> -\t\t       sizeof(arg->description));\n> >>> +\tutils::strlcpy(reinterpret_cast<char *>(arg->description),\n> >>> +\t\t       \"Video Format Description\", sizeof(arg->description));\n> >>>  \targ->pixelformat = drmToV4L2(streamConfig_.formats().pixelformats()[arg->index]);\n> >>>\n> >>> +\tmemset(arg->reserved, 0, sizeof(arg->reserved));\n> >>> +\n> >>\n> >> If I were to be picky, I would have split this to a separate patch.\n> >> At least please expand the commit message to mention what you're\n> >> fixing\n> >\n> > I'd indeed move the memset to the patch that adds support for\n> > V4L2_CAP_EXT_PIX_FORMAT. It should be done for all the format-related\n> > ioctls (of not already there).\n> >\n> >>>  \treturn 0;\n> >>>  }\n> >>>\n> >>> @@ -330,6 +337,9 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)\n> >>>  \t\t\t\t\t      arg->fmt.pix.width,\n> >>>  \t\t\t\t\t      arg->fmt.pix.height);\n> >>>  \targ->fmt.pix.colorspace   = V4L2_COLORSPACE_SRGB;\n> >>> +\targ->fmt.pix.priv         = V4L2_PIX_FMT_PRIV_MAGIC;\n> >>> +\targ->fmt.pix.ycbcr_enc    = V4L2_YCBCR_ENC_601;\n> >>> +\targ->fmt.pix.quantization = V4L2_QUANTIZATION_LIM_RANGE;\n> >>>  }\n> >>>\n> >>>  int V4L2CameraProxy::vidioc_s_fmt(int fd, struct v4l2_format *arg)","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A7D1603BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 17 Jun 2020 18:01:19 +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 7B644F9;\n\tWed, 17 Jun 2020 18:01:18 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"NPXcfdC6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1592409678;\n\tbh=2aBtTO+syorTbOLUXi8Xrttjri/Qm90q4dFE1+ulWqA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NPXcfdC6WWUw+l6//LKPJucsOelIWRXu9zMfPwX5B0iqL8WHD8KjsVP4j0cAD56Fi\n\tdqI1mkjwueRG3ElfxDoADYpKlcAHmgh1ygtWMBtDROfgOB2jKJoEL6i57s/hdVrdlA\n\tBG+ICEEU+Zmyd2W7PYmwO/ofxYVzpx1vrdkJj9cM=","Date":"Wed, 17 Jun 2020 19:00:55 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200617160055.GM5838@pendragon.ideasonboard.com>","References":"<20200616131244.70308-1-paul.elder@ideasonboard.com>\n\t<20200616131244.70308-8-paul.elder@ideasonboard.com>\n\t<20200617145931.jqmwlaxbypzkxcqa@uno.localdomain>\n\t<20200617154911.GK5838@pendragon.ideasonboard.com>\n\t<20200617160121.moh22yxqwjindc65@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200617160121.moh22yxqwjindc65@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 07/15] v4l2: v4l2_camera_proxy: Fix\n\tv4l2-compliance format tests","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 17 Jun 2020 16:01:19 -0000"}}]