[{"id":32033,"web_url":"https://patchwork.libcamera.org/comment/32033/","msgid":"<nflrcqjpwvog7g2xrcwmznttfbkccl3d5ks6imjgi6lvvjtyj7@cvrhbevbgwo6>","date":"2024-11-05T16:13:04","subject":"Re: [PATCH 1/1] libcamera: Add v4l2_subdev_format in\n\tV4L2SubdeviceFormat","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Harvey, Han-Lin\n\nOn Tue, Nov 05, 2024 at 10:49:21AM +0000, Harvey Yang wrote:\n> From: Han-Lin Chen <hanlinchen@chromium.org>\n>\n> Add v4l2_subdev_format in V4L2SubdeviceFormat and set the value when\n> setFormat() and getFormat().\n>\n> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>\n> Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>\n> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>\n> ---\n>  include/libcamera/internal/v4l2_subdevice.h |  2 ++\n>  src/libcamera/pipeline/simple/simple.cpp    |  2 +-\n>  src/libcamera/sensor/camera_sensor.cpp      |  1 +\n>  src/libcamera/v4l2_subdevice.cpp            | 10 ++++++++++\n>  4 files changed, 14 insertions(+), 1 deletion(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index 194382f84..d81296ee6 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -66,6 +66,8 @@ struct V4L2SubdeviceFormat {\n>  \tstd::optional<ColorSpace> colorSpace;\n>\n>  \tconst std::string toString() const;\n> +\n> +\tstruct v4l2_subdev_format subdevFmt;\n>  };\n>\n>  std::ostream &operator<<(std::ostream &out, const V4L2SubdeviceFormat &f);\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 3ddce71d3..6ec055596 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -1063,7 +1063,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>\n>  \tLOG(SimplePipeline, Debug)\n>  \t\t<< \"Picked \"\n> -\t\t<< V4L2SubdeviceFormat{ pipeConfig_->code, pipeConfig_->sensorSize, {} }\n> +\t\t<< V4L2SubdeviceFormat{ pipeConfig_->code, pipeConfig_->sensorSize, {}, {} }\n>  \t\t<< \" -> \" << pipeConfig_->captureSize\n>  \t\t<< \"-\" << pipeConfig_->captureFormat\n>  \t\t<< \" for max stream size \" << maxStreamSize;\n> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> index 1b224f198..ac96b4843 100644\n> --- a/src/libcamera/sensor/camera_sensor.cpp\n> +++ b/src/libcamera/sensor/camera_sensor.cpp\n> @@ -744,6 +744,7 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n>  \t\t.code = bestCode,\n>  \t\t.size = *bestSize,\n>  \t\t.colorSpace = ColorSpace::Raw,\n> +\t\t.subdevFmt = {},\n>  \t};\n>\n>  \treturn format;\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 9f2ec4798..677714890 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -808,6 +808,14 @@ const MediaBusFormatInfo &MediaBusFormatInfo::info(uint32_t code)\n>   * resulting color space is acceptable.\n>   */\n>\n> +/**\n> + * \\var V4L2SubdeviceFormat::subdevFmt\n> + * \\brief The whole v4l2_subdev_format after calling setFormat()/getFormat()\n> + *\n> + * It's used in some pipeline handlers that need extra information apart from\n> + * the existing fields.\n> + */\n> +\n>  /**\n>   * \\brief Assemble and return a string describing the format\n>   * \\return A string describing the V4L2SubdeviceFormat\n> @@ -1266,6 +1274,7 @@ int V4L2Subdevice::getFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n>  \tformat->size.height = subdevFmt.format.height;\n>  \tformat->code = subdevFmt.format.code;\n>  \tformat->colorSpace = toColorSpace(subdevFmt.format);\n> +\tformat->subdevFmt = subdevFmt;\n>\n>  \treturn 0;\n>  }\n> @@ -1324,6 +1333,7 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n>  \tformat->size.height = subdevFmt.format.height;\n>  \tformat->code = subdevFmt.format.code;\n>  \tformat->colorSpace = toColorSpace(subdevFmt.format);\n> +\tformat->subdevFmt = subdevFmt;\n\nI really don't like this last two bits. The getFormat and setFormat\nfunctions already use an instance of 'struct v4l2_subdev_format' and\ncopying it back and forth defeats the purpose of using the\nV4L2SubdeviceFormat libcamera types and the abstractions built around\nthem.\n\nLooking at V4L2SubdeviceFormat, most of the information contained in\n'struct v4l2_subdev_format' are already there (apart from field, if\nI'm seeing it right). The thing is that they get translated into the\ncorresponding libcamera representations and, if for some translating\nthem back to v4l2 fields is trivial (code and sizes in example) the\ntranslation of certain fields happens inside V4L2VideoDevice,\nparticularly the colorspace related fields that happens in\nV4L2Device::fromColorSpace()/V4L2Device::toColorSpace().\n\nIf you have to re-create a v4l2_subdev_format instance to pass it to\nthe MTK kernel interface through the V4L2_CID_MTK_CAM_RAW_RESOURCE_CALC\ncontrol, you would have to reimplement V4L2Device::fromColorSpace()/\nV4L2Device::toColorSpace() in the pipeline handler. Not ideal indeed.\n\nLet alone the fact, ideally at least, v4l2 abstractions should not\nbe present in the pipeline handlers.\n\nOne option could be to add and std::optional<v4l2_subdev_format> to\nV4L2SubdevFormat. If the caller of getFormat/setFormat populates it,\nit will be used in place of the local subdevFmt in those two functions.\n\nIt will need to be properly documented, and explain it is only needed\nin the rare case the pipeline handler needs to access the v4l2 subdev\ninterface.\n\nI would like to hear what others think as well.\n\nThanks\n  j\n\n>\n>  \treturn 0;\n>  }\n> --\n> 2.47.0.199.ga7371fff76-goog\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 DC9F8BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  5 Nov 2024 16:13:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0A5E3653F7;\n\tTue,  5 Nov 2024 17:13:18 +0100 (CET)","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 951B76035B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  5 Nov 2024 17:13:16 +0100 (CET)","from ideasonboard.com (mob-5-90-141-83.net.vodafone.it\n\t[5.90.141.83])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8D7F0194C;\n\tTue,  5 Nov 2024 17:13:03 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OrcZ2A9+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730823183;\n\tbh=nUuUfDKH2gBr/2BRPP+cho1oFmtbMujn5dJ79MtIEUA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OrcZ2A9+LAIJjybR+kNELSETupm81UEnDxGAZhMpKiV9FKfA/6JhQw2izOFNaGZs+\n\tbOqwbhp67LMKeaY+P8vk8wsyVPZrQ7JpwzF8uM0b78VFBLaKkVk/pA14b7v6sXQ+eU\n\t/WLJX4L34R0r/jFlAlaO2coIWGGwe0fum71Q2DEc=","Date":"Tue, 5 Nov 2024 17:13:04 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Harvey Yang <chenghaoyang@chromium.org>","Cc":"libcamera-devel@lists.libcamera.org, \n\tHan-Lin Chen <hanlinchen@chromium.org>","Subject":"Re: [PATCH 1/1] libcamera: Add v4l2_subdev_format in\n\tV4L2SubdeviceFormat","Message-ID":"<nflrcqjpwvog7g2xrcwmznttfbkccl3d5ks6imjgi6lvvjtyj7@cvrhbevbgwo6>","References":"<20241105105445.1468954-1-chenghaoyang@chromium.org>\n\t<20241105105445.1468954-2-chenghaoyang@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241105105445.1468954-2-chenghaoyang@chromium.org>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32049,"web_url":"https://patchwork.libcamera.org/comment/32049/","msgid":"<20241106125442.GE9369@pendragon.ideasonboard.com>","date":"2024-11-06T12:54:42","subject":"Re: [PATCH 1/1] libcamera: Add v4l2_subdev_format in\n\tV4L2SubdeviceFormat","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Nov 05, 2024 at 05:13:04PM +0100, Jacopo Mondi wrote:\n> On Tue, Nov 05, 2024 at 10:49:21AM +0000, Harvey Yang wrote:\n> > From: Han-Lin Chen <hanlinchen@chromium.org>\n> >\n> > Add v4l2_subdev_format in V4L2SubdeviceFormat and set the value when\n> > setFormat() and getFormat().\n> >\n> > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>\n> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>\n> > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>\n> > ---\n> >  include/libcamera/internal/v4l2_subdevice.h |  2 ++\n> >  src/libcamera/pipeline/simple/simple.cpp    |  2 +-\n> >  src/libcamera/sensor/camera_sensor.cpp      |  1 +\n> >  src/libcamera/v4l2_subdevice.cpp            | 10 ++++++++++\n> >  4 files changed, 14 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> > index 194382f84..d81296ee6 100644\n> > --- a/include/libcamera/internal/v4l2_subdevice.h\n> > +++ b/include/libcamera/internal/v4l2_subdevice.h\n> > @@ -66,6 +66,8 @@ struct V4L2SubdeviceFormat {\n> >  \tstd::optional<ColorSpace> colorSpace;\n> >\n> >  \tconst std::string toString() const;\n> > +\n> > +\tstruct v4l2_subdev_format subdevFmt;\n> >  };\n> >\n> >  std::ostream &operator<<(std::ostream &out, const V4L2SubdeviceFormat &f);\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 3ddce71d3..6ec055596 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -1063,7 +1063,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> >\n> >  \tLOG(SimplePipeline, Debug)\n> >  \t\t<< \"Picked \"\n> > -\t\t<< V4L2SubdeviceFormat{ pipeConfig_->code, pipeConfig_->sensorSize, {} }\n> > +\t\t<< V4L2SubdeviceFormat{ pipeConfig_->code, pipeConfig_->sensorSize, {}, {} }\n> >  \t\t<< \" -> \" << pipeConfig_->captureSize\n> >  \t\t<< \"-\" << pipeConfig_->captureFormat\n> >  \t\t<< \" for max stream size \" << maxStreamSize;\n> > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> > index 1b224f198..ac96b4843 100644\n> > --- a/src/libcamera/sensor/camera_sensor.cpp\n> > +++ b/src/libcamera/sensor/camera_sensor.cpp\n> > @@ -744,6 +744,7 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n> >  \t\t.code = bestCode,\n> >  \t\t.size = *bestSize,\n> >  \t\t.colorSpace = ColorSpace::Raw,\n> > +\t\t.subdevFmt = {},\n> >  \t};\n> >\n> >  \treturn format;\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index 9f2ec4798..677714890 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -808,6 +808,14 @@ const MediaBusFormatInfo &MediaBusFormatInfo::info(uint32_t code)\n> >   * resulting color space is acceptable.\n> >   */\n> >\n> > +/**\n> > + * \\var V4L2SubdeviceFormat::subdevFmt\n> > + * \\brief The whole v4l2_subdev_format after calling setFormat()/getFormat()\n> > + *\n> > + * It's used in some pipeline handlers that need extra information apart from\n> > + * the existing fields.\n> > + */\n> > +\n> >  /**\n> >   * \\brief Assemble and return a string describing the format\n> >   * \\return A string describing the V4L2SubdeviceFormat\n> > @@ -1266,6 +1274,7 @@ int V4L2Subdevice::getFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> >  \tformat->size.height = subdevFmt.format.height;\n> >  \tformat->code = subdevFmt.format.code;\n> >  \tformat->colorSpace = toColorSpace(subdevFmt.format);\n> > +\tformat->subdevFmt = subdevFmt;\n> >\n> >  \treturn 0;\n> >  }\n> > @@ -1324,6 +1333,7 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> >  \tformat->size.height = subdevFmt.format.height;\n> >  \tformat->code = subdevFmt.format.code;\n> >  \tformat->colorSpace = toColorSpace(subdevFmt.format);\n> > +\tformat->subdevFmt = subdevFmt;\n> \n> I really don't like this last two bits. The getFormat and setFormat\n> functions already use an instance of 'struct v4l2_subdev_format' and\n> copying it back and forth defeats the purpose of using the\n> V4L2SubdeviceFormat libcamera types and the abstractions built around\n> them.\n> \n> Looking at V4L2SubdeviceFormat, most of the information contained in\n> 'struct v4l2_subdev_format' are already there (apart from field, if\n> I'm seeing it right). The thing is that they get translated into the\n> corresponding libcamera representations and, if for some translating\n> them back to v4l2 fields is trivial (code and sizes in example) the\n> translation of certain fields happens inside V4L2VideoDevice,\n> particularly the colorspace related fields that happens in\n> V4L2Device::fromColorSpace()/V4L2Device::toColorSpace().\n> \n> If you have to re-create a v4l2_subdev_format instance to pass it to\n> the MTK kernel interface through the V4L2_CID_MTK_CAM_RAW_RESOURCE_CALC\n> control, you would have to reimplement V4L2Device::fromColorSpace()/\n> V4L2Device::toColorSpace() in the pipeline handler. Not ideal indeed.\n> \n> Let alone the fact, ideally at least, v4l2 abstractions should not\n> be present in the pipeline handlers.\n> \n> One option could be to add and std::optional<v4l2_subdev_format> to\n> V4L2SubdevFormat. If the caller of getFormat/setFormat populates it,\n> it will be used in place of the local subdevFmt in those two functions.\n> \n> It will need to be properly documented, and explain it is only needed\n> in the rare case the pipeline handler needs to access the v4l2 subdev\n> interface.\n> \n> I would like to hear what others think as well.\n\nUnsurprisingly, I'm not thrilled. V4L2_CID_MTK_CAM_RAW_RESOURCE_CALC\nseems to be a big hack (not even commenting on the fact that the control\n*value* contains userspace pointers). I think the driver will need to be\nsubstantially reworked, and APIs will need to be cleaned up. We\ncertainly shouldn't merge this in the libcamera core before the\nsituation on the kernel side improves.\n\n> >\n> >  \treturn 0;\n> >  }","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 AC467BDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  6 Nov 2024 12:54:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC6E56541E;\n\tWed,  6 Nov 2024 13:54:50 +0100 (CET)","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 96113653C5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  6 Nov 2024 13:54:49 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1C66159D;\n\tWed,  6 Nov 2024 13:54:41 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"X9clZCT7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730897681;\n\tbh=IaSJw2BSWgB80A7G7/90rEjV6ZOs9ZcEuHi1XaL0sDs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=X9clZCT7PFJkpiKFQ2U8SvDFveoMRonlXbiNhPTw/WRL46g1o2h+XdefPKlNSoHTK\n\tBxxUyr+h/6vyZfocawqR3w1aAOZe3/f4e6mBN8n8c9bcc/xmNX+8+dCOtLYkH87CNX\n\tbQMGtoGxZyAlrJ5ZuPnoSajXwB/eLP6QBAFauxGU=","Date":"Wed, 6 Nov 2024 14:54:42 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Harvey Yang <chenghaoyang@chromium.org>,\n\tlibcamera-devel@lists.libcamera.org,\n\tHan-Lin Chen <hanlinchen@chromium.org>","Subject":"Re: [PATCH 1/1] libcamera: Add v4l2_subdev_format in\n\tV4L2SubdeviceFormat","Message-ID":"<20241106125442.GE9369@pendragon.ideasonboard.com>","References":"<20241105105445.1468954-1-chenghaoyang@chromium.org>\n\t<20241105105445.1468954-2-chenghaoyang@chromium.org>\n\t<nflrcqjpwvog7g2xrcwmznttfbkccl3d5ks6imjgi6lvvjtyj7@cvrhbevbgwo6>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<nflrcqjpwvog7g2xrcwmznttfbkccl3d5ks6imjgi6lvvjtyj7@cvrhbevbgwo6>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32065,"web_url":"https://patchwork.libcamera.org/comment/32065/","msgid":"<CAEB1ahvSWBykzV=BMO5-AKQ0ZGSA1=DKcTENTuNUtne-ua4q7A@mail.gmail.com>","date":"2024-11-07T15:48:05","subject":"Re: [PATCH 1/1] libcamera: Add v4l2_subdev_format in\n\tV4L2SubdeviceFormat","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Laurent and Jacopo,\n\nOn Wed, Nov 6, 2024 at 8:54 PM Laurent Pinchart\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> On Tue, Nov 05, 2024 at 05:13:04PM +0100, Jacopo Mondi wrote:\n> > On Tue, Nov 05, 2024 at 10:49:21AM +0000, Harvey Yang wrote:\n> > > From: Han-Lin Chen <hanlinchen@chromium.org>\n> > >\n> > > Add v4l2_subdev_format in V4L2SubdeviceFormat and set the value when\n> > > setFormat() and getFormat().\n> > >\n> > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>\n> > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>\n> > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>\n> > > ---\n> > >  include/libcamera/internal/v4l2_subdevice.h |  2 ++\n> > >  src/libcamera/pipeline/simple/simple.cpp    |  2 +-\n> > >  src/libcamera/sensor/camera_sensor.cpp      |  1 +\n> > >  src/libcamera/v4l2_subdevice.cpp            | 10 ++++++++++\n> > >  4 files changed, 14 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> > > index 194382f84..d81296ee6 100644\n> > > --- a/include/libcamera/internal/v4l2_subdevice.h\n> > > +++ b/include/libcamera/internal/v4l2_subdevice.h\n> > > @@ -66,6 +66,8 @@ struct V4L2SubdeviceFormat {\n> > >     std::optional<ColorSpace> colorSpace;\n> > >\n> > >     const std::string toString() const;\n> > > +\n> > > +   struct v4l2_subdev_format subdevFmt;\n> > >  };\n> > >\n> > >  std::ostream &operator<<(std::ostream &out, const V4L2SubdeviceFormat &f);\n> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > index 3ddce71d3..6ec055596 100644\n> > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > @@ -1063,7 +1063,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> > >\n> > >     LOG(SimplePipeline, Debug)\n> > >             << \"Picked \"\n> > > -           << V4L2SubdeviceFormat{ pipeConfig_->code, pipeConfig_->sensorSize, {} }\n> > > +           << V4L2SubdeviceFormat{ pipeConfig_->code, pipeConfig_->sensorSize, {}, {} }\n> > >             << \" -> \" << pipeConfig_->captureSize\n> > >             << \"-\" << pipeConfig_->captureFormat\n> > >             << \" for max stream size \" << maxStreamSize;\n> > > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> > > index 1b224f198..ac96b4843 100644\n> > > --- a/src/libcamera/sensor/camera_sensor.cpp\n> > > +++ b/src/libcamera/sensor/camera_sensor.cpp\n> > > @@ -744,6 +744,7 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n> > >             .code = bestCode,\n> > >             .size = *bestSize,\n> > >             .colorSpace = ColorSpace::Raw,\n> > > +           .subdevFmt = {},\n> > >     };\n> > >\n> > >     return format;\n> > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > > index 9f2ec4798..677714890 100644\n> > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > @@ -808,6 +808,14 @@ const MediaBusFormatInfo &MediaBusFormatInfo::info(uint32_t code)\n> > >   * resulting color space is acceptable.\n> > >   */\n> > >\n> > > +/**\n> > > + * \\var V4L2SubdeviceFormat::subdevFmt\n> > > + * \\brief The whole v4l2_subdev_format after calling setFormat()/getFormat()\n> > > + *\n> > > + * It's used in some pipeline handlers that need extra information apart from\n> > > + * the existing fields.\n> > > + */\n> > > +\n> > >  /**\n> > >   * \\brief Assemble and return a string describing the format\n> > >   * \\return A string describing the V4L2SubdeviceFormat\n> > > @@ -1266,6 +1274,7 @@ int V4L2Subdevice::getFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> > >     format->size.height = subdevFmt.format.height;\n> > >     format->code = subdevFmt.format.code;\n> > >     format->colorSpace = toColorSpace(subdevFmt.format);\n> > > +   format->subdevFmt = subdevFmt;\n> > >\n> > >     return 0;\n> > >  }\n> > > @@ -1324,6 +1333,7 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> > >     format->size.height = subdevFmt.format.height;\n> > >     format->code = subdevFmt.format.code;\n> > >     format->colorSpace = toColorSpace(subdevFmt.format);\n> > > +   format->subdevFmt = subdevFmt;\n> >\n> > I really don't like this last two bits. The getFormat and setFormat\n> > functions already use an instance of 'struct v4l2_subdev_format' and\n> > copying it back and forth defeats the purpose of using the\n> > V4L2SubdeviceFormat libcamera types and the abstractions built around\n> > them.\n> >\n> > Looking at V4L2SubdeviceFormat, most of the information contained in\n> > 'struct v4l2_subdev_format' are already there (apart from field, if\n> > I'm seeing it right). The thing is that they get translated into the\n> > corresponding libcamera representations and, if for some translating\n> > them back to v4l2 fields is trivial (code and sizes in example) the\n> > translation of certain fields happens inside V4L2VideoDevice,\n> > particularly the colorspace related fields that happens in\n> > V4L2Device::fromColorSpace()/V4L2Device::toColorSpace().\n> >\n> > If you have to re-create a v4l2_subdev_format instance to pass it to\n> > the MTK kernel interface through the V4L2_CID_MTK_CAM_RAW_RESOURCE_CALC\n> > control, you would have to reimplement V4L2Device::fromColorSpace()/\n> > V4L2Device::toColorSpace() in the pipeline handler. Not ideal indeed.\n> >\n> > Let alone the fact, ideally at least, v4l2 abstractions should not\n> > be present in the pipeline handlers.\n> >\n> > One option could be to add and std::optional<v4l2_subdev_format> to\n> > V4L2SubdevFormat. If the caller of getFormat/setFormat populates it,\n> > it will be used in place of the local subdevFmt in those two functions.\n> >\n> > It will need to be properly documented, and explain it is only needed\n> > in the rare case the pipeline handler needs to access the v4l2 subdev\n> > interface.\n> >\n> > I would like to hear what others think as well.\n>\n> Unsurprisingly, I'm not thrilled. V4L2_CID_MTK_CAM_RAW_RESOURCE_CALC\n> seems to be a big hack (not even commenting on the fact that the control\n> *value* contains userspace pointers). I think the driver will need to be\n> substantially reworked, and APIs will need to be cleaned up. We\n> certainly shouldn't merge this in the libcamera core before the\n> situation on the kernel side improves.\n\nYeah, I agree.\nAlso, I've tried to provide only width & height in struct v4l2_mbus_framefmt,\nand the kernel still works [1].\n\nLet's abandon this CL for now. I'll wait for MTK's comments though.\n\nBR,\nHarvey\n\n[1]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/kernel/v6.1/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-raw.c;l=607\n\n>\n> > >\n> > >     return 0;\n> > >  }\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 91D55BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  7 Nov 2024 15:48:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6178A6547A;\n\tThu,  7 Nov 2024 16:48:18 +0100 (CET)","from mail-lj1-x231.google.com (mail-lj1-x231.google.com\n\t[IPv6:2a00:1450:4864:20::231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E094165474\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 Nov 2024 16:48:16 +0100 (CET)","by mail-lj1-x231.google.com with SMTP id\n\t38308e7fff4ca-2fb5be4381dso11864111fa.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 07 Nov 2024 07:48:16 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"n1pN71Ba\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1730994496; x=1731599296;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=NSDVqqPNU3wTeH6RHAjyZvWIPJjFXuZovBZ+nNAHAPA=;\n\tb=n1pN71BaC8iVa4kS/vu+CkLt1ah03tytal5KgqXuDR7616APoXK8FhPGGLfCCkdU2q\n\tEDUXTg1X4vdYQlrlzZAwm4Z4OHRJxUUQTvYMzE01wLkDBxkkvB633h7OL3LnnTcBLFtU\n\tvCXrdoiDXYubkN+Yqr+LaLRjFEdhy1EP5eink=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1730994496; x=1731599296;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=NSDVqqPNU3wTeH6RHAjyZvWIPJjFXuZovBZ+nNAHAPA=;\n\tb=jHxsLeLG7x3JT3md4/G6hzQodnE0hQoQxtLjJILscYLU1lySVrCj3lqO40PM1SB7qQ\n\t8BhpSRVFmxRWtRxZWJHw48SUfpvDHVWdw+ZYh2R1pW1HPiJMCceEXoAXpPoxQHlIJpQQ\n\t4+WE4p0BRiVoOsUN6RbhpD0T6hEg+0vRjf1NuVUmPg6VKS+P/3U4DjsEzAGWBM8aJdGx\n\tn9uUlvMu5RomPRFgEXK18d55Koyoj57DI9bCHFwuUroVUKT5wvz1VFaTboumF6/FBztg\n\tATt4FHl2fJcuiIDaLgiUHGsvamF886+e7Hd4cbkQq8JJVLVWd1gFYKOWwXFB88OSMhbx\n\tcAqQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCWrgUVGyA69qydJexFS9EGpnGEPMvQ1waodTA+HhMZ8e5n3sjbKWw/jOm7G06TrdMXnO8oITJMgfbyWXbF/o9c=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YyKa5cYb3mEY4w3ysndjF5T1ZsW9NyyPKUASNBW9VTBD+dKB6kB\n\t1bycT0GfMe8GbFelNK8RWYNtrOBnvLv7VWc3WXbrlYHwG1odSeRrx+K85ctM6bXs4hEJkVvyHk7\n\tB1RtOg3v5aNQky63u85UdReH/yCYx6VKHMH5n2jujv45dwTKeHY7o","X-Google-Smtp-Source":"AGHT+IG4bY6uSKmmHie584e2RtcutLN7t9WOZxJ45Pqp0E4YDVi5diywtoSE1FA4aTEjVHEIN1EZhqMqeweMSoKgXPc=","X-Received":"by 2002:a2e:a543:0:b0:2fa:cc50:41b with SMTP id\n\t38308e7fff4ca-2fcbdf5fa9amr244265181fa.5.1730994495828;\n\tThu, 07 Nov 2024 07:48:15 -0800 (PST)","MIME-Version":"1.0","References":"<20241105105445.1468954-1-chenghaoyang@chromium.org>\n\t<20241105105445.1468954-2-chenghaoyang@chromium.org>\n\t<nflrcqjpwvog7g2xrcwmznttfbkccl3d5ks6imjgi6lvvjtyj7@cvrhbevbgwo6>\n\t<20241106125442.GE9369@pendragon.ideasonboard.com>","In-Reply-To":"<20241106125442.GE9369@pendragon.ideasonboard.com>","From":"Cheng-Hao Yang <chenghaoyang@chromium.org>","Date":"Thu, 7 Nov 2024 23:48:05 +0800","Message-ID":"<CAEB1ahvSWBykzV=BMO5-AKQ0ZGSA1=DKcTENTuNUtne-ua4q7A@mail.gmail.com>","Subject":"Re: [PATCH 1/1] libcamera: Add v4l2_subdev_format in\n\tV4L2SubdeviceFormat","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org, \n\tHan-Lin Chen <hanlinchen@chromium.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32157,"web_url":"https://patchwork.libcamera.org/comment/32157/","msgid":"<CAEB1ahvBG1mLReZVeCn6GtXsyo_hCgSeZn9w8sDNRrxP_k4HcA@mail.gmail.com>","date":"2024-11-14T05:33:20","subject":"Re: [PATCH 1/1] libcamera: Add v4l2_subdev_format in\n\tV4L2SubdeviceFormat","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi folks,\n\nOn Thu, Nov 7, 2024 at 11:48 PM Cheng-Hao Yang\n<chenghaoyang@chromium.org> wrote:\n>\n> Hi Laurent and Jacopo,\n>\n> On Wed, Nov 6, 2024 at 8:54 PM Laurent Pinchart\n> <laurent.pinchart@ideasonboard.com> wrote:\n> >\n> > On Tue, Nov 05, 2024 at 05:13:04PM +0100, Jacopo Mondi wrote:\n> > > On Tue, Nov 05, 2024 at 10:49:21AM +0000, Harvey Yang wrote:\n> > > > From: Han-Lin Chen <hanlinchen@chromium.org>\n> > > >\n> > > > Add v4l2_subdev_format in V4L2SubdeviceFormat and set the value when\n> > > > setFormat() and getFormat().\n> > > >\n> > > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>\n> > > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>\n> > > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>\n> > > > ---\n> > > >  include/libcamera/internal/v4l2_subdevice.h |  2 ++\n> > > >  src/libcamera/pipeline/simple/simple.cpp    |  2 +-\n> > > >  src/libcamera/sensor/camera_sensor.cpp      |  1 +\n> > > >  src/libcamera/v4l2_subdevice.cpp            | 10 ++++++++++\n> > > >  4 files changed, 14 insertions(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> > > > index 194382f84..d81296ee6 100644\n> > > > --- a/include/libcamera/internal/v4l2_subdevice.h\n> > > > +++ b/include/libcamera/internal/v4l2_subdevice.h\n> > > > @@ -66,6 +66,8 @@ struct V4L2SubdeviceFormat {\n> > > >     std::optional<ColorSpace> colorSpace;\n> > > >\n> > > >     const std::string toString() const;\n> > > > +\n> > > > +   struct v4l2_subdev_format subdevFmt;\n> > > >  };\n> > > >\n> > > >  std::ostream &operator<<(std::ostream &out, const V4L2SubdeviceFormat &f);\n> > > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > > index 3ddce71d3..6ec055596 100644\n> > > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > > @@ -1063,7 +1063,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> > > >\n> > > >     LOG(SimplePipeline, Debug)\n> > > >             << \"Picked \"\n> > > > -           << V4L2SubdeviceFormat{ pipeConfig_->code, pipeConfig_->sensorSize, {} }\n> > > > +           << V4L2SubdeviceFormat{ pipeConfig_->code, pipeConfig_->sensorSize, {}, {} }\n> > > >             << \" -> \" << pipeConfig_->captureSize\n> > > >             << \"-\" << pipeConfig_->captureFormat\n> > > >             << \" for max stream size \" << maxStreamSize;\n> > > > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> > > > index 1b224f198..ac96b4843 100644\n> > > > --- a/src/libcamera/sensor/camera_sensor.cpp\n> > > > +++ b/src/libcamera/sensor/camera_sensor.cpp\n> > > > @@ -744,6 +744,7 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n> > > >             .code = bestCode,\n> > > >             .size = *bestSize,\n> > > >             .colorSpace = ColorSpace::Raw,\n> > > > +           .subdevFmt = {},\n> > > >     };\n> > > >\n> > > >     return format;\n> > > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > > > index 9f2ec4798..677714890 100644\n> > > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > > @@ -808,6 +808,14 @@ const MediaBusFormatInfo &MediaBusFormatInfo::info(uint32_t code)\n> > > >   * resulting color space is acceptable.\n> > > >   */\n> > > >\n> > > > +/**\n> > > > + * \\var V4L2SubdeviceFormat::subdevFmt\n> > > > + * \\brief The whole v4l2_subdev_format after calling setFormat()/getFormat()\n> > > > + *\n> > > > + * It's used in some pipeline handlers that need extra information apart from\n> > > > + * the existing fields.\n> > > > + */\n> > > > +\n> > > >  /**\n> > > >   * \\brief Assemble and return a string describing the format\n> > > >   * \\return A string describing the V4L2SubdeviceFormat\n> > > > @@ -1266,6 +1274,7 @@ int V4L2Subdevice::getFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> > > >     format->size.height = subdevFmt.format.height;\n> > > >     format->code = subdevFmt.format.code;\n> > > >     format->colorSpace = toColorSpace(subdevFmt.format);\n> > > > +   format->subdevFmt = subdevFmt;\n> > > >\n> > > >     return 0;\n> > > >  }\n> > > > @@ -1324,6 +1333,7 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> > > >     format->size.height = subdevFmt.format.height;\n> > > >     format->code = subdevFmt.format.code;\n> > > >     format->colorSpace = toColorSpace(subdevFmt.format);\n> > > > +   format->subdevFmt = subdevFmt;\n> > >\n> > > I really don't like this last two bits. The getFormat and setFormat\n> > > functions already use an instance of 'struct v4l2_subdev_format' and\n> > > copying it back and forth defeats the purpose of using the\n> > > V4L2SubdeviceFormat libcamera types and the abstractions built around\n> > > them.\n> > >\n> > > Looking at V4L2SubdeviceFormat, most of the information contained in\n> > > 'struct v4l2_subdev_format' are already there (apart from field, if\n> > > I'm seeing it right). The thing is that they get translated into the\n> > > corresponding libcamera representations and, if for some translating\n> > > them back to v4l2 fields is trivial (code and sizes in example) the\n> > > translation of certain fields happens inside V4L2VideoDevice,\n> > > particularly the colorspace related fields that happens in\n> > > V4L2Device::fromColorSpace()/V4L2Device::toColorSpace().\n> > >\n> > > If you have to re-create a v4l2_subdev_format instance to pass it to\n> > > the MTK kernel interface through the V4L2_CID_MTK_CAM_RAW_RESOURCE_CALC\n> > > control, you would have to reimplement V4L2Device::fromColorSpace()/\n> > > V4L2Device::toColorSpace() in the pipeline handler. Not ideal indeed.\n> > >\n> > > Let alone the fact, ideally at least, v4l2 abstractions should not\n> > > be present in the pipeline handlers.\n> > >\n> > > One option could be to add and std::optional<v4l2_subdev_format> to\n> > > V4L2SubdevFormat. If the caller of getFormat/setFormat populates it,\n> > > it will be used in place of the local subdevFmt in those two functions.\n> > >\n> > > It will need to be properly documented, and explain it is only needed\n> > > in the rare case the pipeline handler needs to access the v4l2 subdev\n> > > interface.\n> > >\n> > > I would like to hear what others think as well.\n> >\n> > Unsurprisingly, I'm not thrilled. V4L2_CID_MTK_CAM_RAW_RESOURCE_CALC\n> > seems to be a big hack (not even commenting on the fact that the control\n> > *value* contains userspace pointers). I think the driver will need to be\n> > substantially reworked, and APIs will need to be cleaned up. We\n> > certainly shouldn't merge this in the libcamera core before the\n> > situation on the kernel side improves.\n>\n> Yeah, I agree.\n> Also, I've tried to provide only width & height in struct v4l2_mbus_framefmt,\n> and the kernel still works [1].\n>\n> Let's abandon this CL for now. I'll wait for MTK's comments though.\n\nConfirmed with MTK: the kernel only needs width and height. We should\nupdate the kernel that takes the two values only.\n\nBR,\nHarvey\n\n>\n> BR,\n> Harvey\n>\n> [1]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/kernel/v6.1/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-raw.c;l=607\n>\n> >\n> > > >\n> > > >     return 0;\n> > > >  }\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 50CDBBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 14 Nov 2024 05:33:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B26DE6582F;\n\tThu, 14 Nov 2024 06:33:33 +0100 (CET)","from mail-lj1-x234.google.com (mail-lj1-x234.google.com\n\t[IPv6:2a00:1450:4864:20::234])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EC289618BC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 14 Nov 2024 06:33:31 +0100 (CET)","by mail-lj1-x234.google.com with SMTP id\n\t38308e7fff4ca-2fb498a92f6so1758601fa.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 13 Nov 2024 21:33:31 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"LGeOYVW3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1731562411; x=1732167211;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=Me5st5zmffnJiZFumugN0jC+sFcGQfeJt8bVl7TcGbY=;\n\tb=LGeOYVW3pQmkKHc5csCV738TfhQ0zpuDqOvXPkSVx2atrWxTSAqpOVdA5tIG6Acfh4\n\tLFl1p5JnOPc3G81i+rbOHBtRNcSxSwxOR+TpaVMoAZC6YPu31kWsQA+2y8RhwCeGgDWD\n\tJl94tMQgonQx7nTRoN2LVc4MzkUA3EXkC/UoA=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1731562411; x=1732167211;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=Me5st5zmffnJiZFumugN0jC+sFcGQfeJt8bVl7TcGbY=;\n\tb=jNTnv/G2mMy+r3vEd5o1VjBpz9Xcoh4p6N11rcms1BXn35HM3Uxv9gk0we12uNUBHm\n\tXlGkM1iv5fVByJMUeECKi5guaVJR7NGxAN13usADUox9WZwj2ORvvXJG0qAHSmrbVygg\n\toGQ+cGS+N4PUBjpYC2kpFJdYHqjauUg3aaJGGCHnvjq0wrgDdauebXA0RUEJd9btEFTI\n\toWmI3cuJXfEeWIZMQ5dsBYUTksPq1yPeKr1oGP3KVh1yFteAJN8NKeee3R0WzMx8OxoJ\n\tZuG8P7L7ixpQ5nquFEFGIwWqxtrAGkKigI+2l1m0pTxb+AicA7O5yyK5vG5lxlbCvbRC\n\tCgmA==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCV/8KzVoVmf03d1QgRoFXHkvt0wVAnCRUc283/uSHQ99iHOpIsIgGyjKYYvkR3C2I1tAilU3b6qG9ZVX1ClyHM=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YzJfDF7O1oWd4hY+fNk97NZx4KuOuloutidWkc8NBhm7tGVlKRp\n\tLt8G8T4Xi5ZL/vzow7pHv1bcFbWv2cm4x0gLEJFIzUh7Yc3eK1CISDFC4ni66ejjR2vijSccrcm\n\tmjno4PAJIsAWASv/kkwKjPxMS6Z9D8DcPU2K0","X-Google-Smtp-Source":"AGHT+IGGHhypEA0dCd/g6FF/g438RgWQGm050hRjURXxTUYpVxkfyNHNBob57W2vGN776QES4Z7y0bEmMrziO6m7iJs=","X-Received":"by 2002:a2e:8905:0:b0:2fb:3a12:a582 with SMTP id\n\t38308e7fff4ca-2ff202808bfmr87923521fa.23.1731562410997;\n\tWed, 13 Nov 2024 21:33:30 -0800 (PST)","MIME-Version":"1.0","References":"<20241105105445.1468954-1-chenghaoyang@chromium.org>\n\t<20241105105445.1468954-2-chenghaoyang@chromium.org>\n\t<nflrcqjpwvog7g2xrcwmznttfbkccl3d5ks6imjgi6lvvjtyj7@cvrhbevbgwo6>\n\t<20241106125442.GE9369@pendragon.ideasonboard.com>\n\t<CAEB1ahvSWBykzV=BMO5-AKQ0ZGSA1=DKcTENTuNUtne-ua4q7A@mail.gmail.com>","In-Reply-To":"<CAEB1ahvSWBykzV=BMO5-AKQ0ZGSA1=DKcTENTuNUtne-ua4q7A@mail.gmail.com>","From":"Cheng-Hao Yang <chenghaoyang@chromium.org>","Date":"Thu, 14 Nov 2024 13:33:20 +0800","Message-ID":"<CAEB1ahvBG1mLReZVeCn6GtXsyo_hCgSeZn9w8sDNRrxP_k4HcA@mail.gmail.com>","Subject":"Re: [PATCH 1/1] libcamera: Add v4l2_subdev_format in\n\tV4L2SubdeviceFormat","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org, \n\tHan-Lin Chen <hanlinchen@chromium.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]