[{"id":15593,"web_url":"https://patchwork.libcamera.org/comment/15593/","msgid":"<20210311081642.r2p54tqqvtj3nqbe@uno.localdomain>","date":"2021-03-11T08:16:42","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Fix\n\tformats() returning only sizes for last mbus code","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hello Marian,\n\nOn Tue, Mar 09, 2021 at 03:44:57PM +0100, Marian Cichy wrote:\n> V4L2Subdevice::formats() wants to enumerate all media bus codes and\n> frame sizes on a pad, however, the current implementation always\n> overwrites the SizeRange of the last mbus code, effectively returning\n> only this SizeRange.\n>\n> This can be even fatal, as camera_sensor is using this function to\n> enumerate all codes and sizes. If the last mbus code has no SizeRange\n> for whatever reason, the camera_sensor will error out, claiming that it\n> didn't found any image formats at all.\n\nWhat is the use case for a driver returning an mbus code without any\nimage size associated ? Smells like a driver bug to me.\n\n>\n> Signed-off-by: Marian Cichy <m.cichy@pengutronix.de>\n> ---\n>  src/libcamera/v4l2_subdevice.cpp | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 9af1c0ab..37bafb9b 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -354,10 +354,10 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)\n>  \t\treturn {};\n>  \t}\n>\n> +\tstd::vector<SizeRange> sizes = {};\n>  \tfor (unsigned int code : enumPadCodes(pad)) {\n> -\t\tstd::vector<SizeRange> sizes = enumPadSizes(pad, code);\n> -\t\tif (sizes.empty())\n> -\t\t\treturn {};\n> +\t\tstd::vector<SizeRange> codeSizeRange = enumPadSizes(pad, code);\n> +\t\tsizes.insert(sizes.end(), codeSizeRange.begin(), codeSizeRange.end());\n\nAlso, this builds 'sizes' incrementally, associating to the newly\nenumerated 'code' the sizes previously enumerated for other codes\nif I'm not mistaken.\n\nIs this intentional ?\n\nThanks\n   j\n\n>\n>  \t\tconst auto inserted = formats.insert({ code, sizes });\n>  \t\tif (!inserted.second) {\n> --\n> 2.29.2\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 985ABBD80C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 11 Mar 2021 08:16:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D970668C6B;\n\tThu, 11 Mar 2021 09:16:13 +0100 (CET)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9A73A68AA3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 11 Mar 2021 09:16:12 +0100 (CET)","from uno.localdomain (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id D245B60005;\n\tThu, 11 Mar 2021 08:16:11 +0000 (UTC)"],"X-Originating-IP":"93.61.96.190","Date":"Thu, 11 Mar 2021 09:16:42 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Marian Cichy <m.cichy@pengutronix.de>","Message-ID":"<20210311081642.r2p54tqqvtj3nqbe@uno.localdomain>","References":"<20210309144457.32412-1-m.cichy@pengutronix.de>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210309144457.32412-1-m.cichy@pengutronix.de>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Fix\n\tformats() returning only sizes for last mbus code","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>","Cc":"libcamera-devel@lists.libcamera.org, graphics@pengutronix.de","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":15594,"web_url":"https://patchwork.libcamera.org/comment/15594/","msgid":"<ba2a11f2-ff89-d7e9-adbd-12a3eca4bd57@pengutronix.de>","date":"2021-03-11T11:15:48","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Fix\n\tformats() returning only sizes for last mbus code","submitter":{"id":81,"url":"https://patchwork.libcamera.org/api/people/81/","name":"Marian Cichy","email":"mci@pengutronix.de"},"content":"On 3/11/21 9:16 AM, Jacopo Mondi wrote:\n> Hello Marian,\n>\n> On Tue, Mar 09, 2021 at 03:44:57PM +0100, Marian Cichy wrote:\n>> V4L2Subdevice::formats() wants to enumerate all media bus codes and\n>> frame sizes on a pad, however, the current implementation always\n>> overwrites the SizeRange of the last mbus code, effectively returning\n>> only this SizeRange.\n>>\n>> This can be even fatal, as camera_sensor is using this function to\n>> enumerate all codes and sizes. If the last mbus code has no SizeRange\n>> for whatever reason, the camera_sensor will error out, claiming that it\n>> didn't found any image formats at all.\n> What is the use case for a driver returning an mbus code without any\n> image size associated ? Smells like a driver bug to me.\n\nThe driver I am using has a code which is only used by the \nhardware-internal test pattern generator and it doesn't return a size \nfor this code. It might still very well a driver bug though.\n\n>> Signed-off-by: Marian Cichy <m.cichy@pengutronix.de>\n>> ---\n>>   src/libcamera/v4l2_subdevice.cpp | 6 +++---\n>>   1 file changed, 3 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n>> index 9af1c0ab..37bafb9b 100644\n>> --- a/src/libcamera/v4l2_subdevice.cpp\n>> +++ b/src/libcamera/v4l2_subdevice.cpp\n>> @@ -354,10 +354,10 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)\n>>   \t\treturn {};\n>>   \t}\n>>\n>> +\tstd::vector<SizeRange> sizes = {};\n>>   \tfor (unsigned int code : enumPadCodes(pad)) {\n>> -\t\tstd::vector<SizeRange> sizes = enumPadSizes(pad, code);\n>> -\t\tif (sizes.empty())\n>> -\t\t\treturn {};\n>> +\t\tstd::vector<SizeRange> codeSizeRange = enumPadSizes(pad, code);\n>> +\t\tsizes.insert(sizes.end(), codeSizeRange.begin(), codeSizeRange.end());\n> Also, this builds 'sizes' incrementally, associating to the newly\n> enumerated 'code' the sizes previously enumerated for other codes\n> if I'm not mistaken.\n>\n> Is this intentional ?\n\nRight, this merges all sizes and all codes together and is probably not \nwanted. The previous implementation is actually correct, expect that I \nfall into the empty sizes case.\n\n>\n> Thanks\n>     j\n>\n>>   \t\tconst auto inserted = formats.insert({ code, sizes });\n>>   \t\tif (!inserted.second) {\n>> --\n>> 2.29.2\n>>\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel","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 18F00BD80C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 11 Mar 2021 11:15:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 68076602ED;\n\tThu, 11 Mar 2021 12:15:51 +0100 (CET)","from metis.ext.pengutronix.de (metis.ext.pengutronix.de\n\t[IPv6:2001:67c:670:201:290:27ff:fe1d:cc33])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B6140602E8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 11 Mar 2021 12:15:49 +0100 (CET)","from erdnuss.hi.pengutronix.de\n\t([2001:67c:670:100:2e4d:54ff:fe9d:849c])\n\tby metis.ext.pengutronix.de with esmtp (Exim 4.92)\n\t(envelope-from <mci@pengutronix.de>)\n\tid 1lKJID-0001GC-5Z; Thu, 11 Mar 2021 12:15:49 +0100"],"To":"Jacopo Mondi <jacopo@jmondi.org>","References":"<20210309144457.32412-1-m.cichy@pengutronix.de>\n\t<20210311081642.r2p54tqqvtj3nqbe@uno.localdomain>","From":"Marian Cichy <mci@pengutronix.de>","Message-ID":"<ba2a11f2-ff89-d7e9-adbd-12a3eca4bd57@pengutronix.de>","Date":"Thu, 11 Mar 2021 12:15:48 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.8.0","MIME-Version":"1.0","In-Reply-To":"<20210311081642.r2p54tqqvtj3nqbe@uno.localdomain>","Content-Language":"en-US","X-SA-Exim-Connect-IP":"2001:67c:670:100:2e4d:54ff:fe9d:849c","X-SA-Exim-Mail-From":"mci@pengutronix.de","X-SA-Exim-Scanned":"No (on metis.ext.pengutronix.de);\n\tSAEximRunCond expanded to false","X-PTX-Original-Recipient":"libcamera-devel@lists.libcamera.org","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Fix\n\tformats() returning only sizes for last mbus code","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>","Cc":"libcamera-devel@lists.libcamera.org, graphics@pengutronix.de","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":15595,"web_url":"https://patchwork.libcamera.org/comment/15595/","msgid":"<20210311112805.lg7tzgseu3xypczi@uno.localdomain>","date":"2021-03-11T11:28:05","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Fix\n\tformats() returning only sizes for last mbus code","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Marian,\n\nOn Thu, Mar 11, 2021 at 12:15:48PM +0100, Marian Cichy wrote:\n>\n>\n> On 3/11/21 9:16 AM, Jacopo Mondi wrote:\n> > Hello Marian,\n> >\n> > On Tue, Mar 09, 2021 at 03:44:57PM +0100, Marian Cichy wrote:\n> > > V4L2Subdevice::formats() wants to enumerate all media bus codes and\n> > > frame sizes on a pad, however, the current implementation always\n> > > overwrites the SizeRange of the last mbus code, effectively returning\n> > > only this SizeRange.\n> > >\n> > > This can be even fatal, as camera_sensor is using this function to\n> > > enumerate all codes and sizes. If the last mbus code has no SizeRange\n> > > for whatever reason, the camera_sensor will error out, claiming that it\n> > > didn't found any image formats at all.\n> > What is the use case for a driver returning an mbus code without any\n> > image size associated ? Smells like a driver bug to me.\n>\n> The driver I am using has a code which is only used by the hardware-internal\n> test pattern generator and it doesn't return a size for this code. It might\n> still very well a driver bug though.\n>\n\nI see. It might be a valid use case, even if test patterns have a size,\ndoesn't they ?\n\n> > > Signed-off-by: Marian Cichy <m.cichy@pengutronix.de>\n> > > ---\n> > >   src/libcamera/v4l2_subdevice.cpp | 6 +++---\n> > >   1 file changed, 3 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > > index 9af1c0ab..37bafb9b 100644\n> > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > @@ -354,10 +354,10 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)\n> > >   \t\treturn {};\n> > >   \t}\n> > >\n> > > +\tstd::vector<SizeRange> sizes = {};\n> > >   \tfor (unsigned int code : enumPadCodes(pad)) {\n> > > -\t\tstd::vector<SizeRange> sizes = enumPadSizes(pad, code);\n> > > -\t\tif (sizes.empty())\n> > > -\t\t\treturn {};\n> > > +\t\tstd::vector<SizeRange> codeSizeRange = enumPadSizes(pad, code);\n> > > +\t\tsizes.insert(sizes.end(), codeSizeRange.begin(), codeSizeRange.end());\n> > Also, this builds 'sizes' incrementally, associating to the newly\n> > enumerated 'code' the sizes previously enumerated for other codes\n> > if I'm not mistaken.\n> >\n> > Is this intentional ?\n>\n> Right, this merges all sizes and all codes together and is probably not\n> wanted. The previous implementation is actually correct, expect that I fall\n> into the empty sizes case.\n\nOne thing you could do is to simply 'continue' if no size is\nassociated with a media bus code. I presume we bailed-out to be able\nto catch errors returned by enumPadSizes(), which in case you decide\nto 'continue' would be missed.\n\nAlternatively, the enumPad[Code|Size] could be changed to accept a\nvector of [unsigned int|SizeRange] respectively as output parameter\nand return an error code to be able to distinguish an error condition\nfrom an empty list of sizes.\n\nAnyway, I would first make sure if the driver behaviour is intended or\nnot.\n\nThanks\n   j\n\n>\n> >\n> > Thanks\n> >     j\n> >\n> > >   \t\tconst auto inserted = formats.insert({ code, sizes });\n> > >   \t\tif (!inserted.second) {\n> > > --\n> > > 2.29.2\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\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 043F5BD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 11 Mar 2021 11:27:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7B5CC68C69;\n\tThu, 11 Mar 2021 12:27:36 +0100 (CET)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0816A602E8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 11 Mar 2021 12:27:35 +0100 (CET)","from uno.localdomain (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 48B9710000B;\n\tThu, 11 Mar 2021 11:27:33 +0000 (UTC)"],"Date":"Thu, 11 Mar 2021 12:28:05 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Marian Cichy <mci@pengutronix.de>","Message-ID":"<20210311112805.lg7tzgseu3xypczi@uno.localdomain>","References":"<20210309144457.32412-1-m.cichy@pengutronix.de>\n\t<20210311081642.r2p54tqqvtj3nqbe@uno.localdomain>\n\t<ba2a11f2-ff89-d7e9-adbd-12a3eca4bd57@pengutronix.de>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<ba2a11f2-ff89-d7e9-adbd-12a3eca4bd57@pengutronix.de>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Fix\n\tformats() returning only sizes for last mbus code","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>","Cc":"libcamera-devel@lists.libcamera.org, graphics@pengutronix.de","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":15597,"web_url":"https://patchwork.libcamera.org/comment/15597/","msgid":"<YEoCBuHY9qfRrg1X@pendragon.ideasonboard.com>","date":"2021-03-11T11:41:58","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Fix\n\tformats() returning only sizes for last mbus code","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Thu, Mar 11, 2021 at 12:28:05PM +0100, Jacopo Mondi wrote:\n> On Thu, Mar 11, 2021 at 12:15:48PM +0100, Marian Cichy wrote:\n> > On 3/11/21 9:16 AM, Jacopo Mondi wrote:\n> > > On Tue, Mar 09, 2021 at 03:44:57PM +0100, Marian Cichy wrote:\n> > > > V4L2Subdevice::formats() wants to enumerate all media bus codes and\n> > > > frame sizes on a pad, however, the current implementation always\n> > > > overwrites the SizeRange of the last mbus code, effectively returning\n> > > > only this SizeRange.\n> > > >\n> > > > This can be even fatal, as camera_sensor is using this function to\n> > > > enumerate all codes and sizes. If the last mbus code has no SizeRange\n> > > > for whatever reason, the camera_sensor will error out, claiming that it\n> > > > didn't found any image formats at all.\n> > >\n> > > What is the use case for a driver returning an mbus code without any\n> > > image size associated ? Smells like a driver bug to me.\n> >\n> > The driver I am using has a code which is only used by the hardware-internal\n> > test pattern generator and it doesn't return a size for this code. It might\n> > still very well a driver bug though.\n> \n> I see. It might be a valid use case, even if test patterns have a size,\n> doesn't they ?\n\nA media bus code without sizes seems a bit weird. Marian, on which\nsubdev/pad is this, and what's the code ?\n\n> > > > Signed-off-by: Marian Cichy <m.cichy@pengutronix.de>\n> > > > ---\n> > > >   src/libcamera/v4l2_subdevice.cpp | 6 +++---\n> > > >   1 file changed, 3 insertions(+), 3 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > > > index 9af1c0ab..37bafb9b 100644\n> > > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > > @@ -354,10 +354,10 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)\n> > > >   \t\treturn {};\n> > > >   \t}\n> > > >\n> > > > +\tstd::vector<SizeRange> sizes = {};\n> > > >   \tfor (unsigned int code : enumPadCodes(pad)) {\n> > > > -\t\tstd::vector<SizeRange> sizes = enumPadSizes(pad, code);\n> > > > -\t\tif (sizes.empty())\n> > > > -\t\t\treturn {};\n> > > > +\t\tstd::vector<SizeRange> codeSizeRange = enumPadSizes(pad, code);\n> > > > +\t\tsizes.insert(sizes.end(), codeSizeRange.begin(), codeSizeRange.end());\n> > >\n> > > Also, this builds 'sizes' incrementally, associating to the newly\n> > > enumerated 'code' the sizes previously enumerated for other codes\n> > > if I'm not mistaken.\n> > >\n> > > Is this intentional ?\n> >\n> > Right, this merges all sizes and all codes together and is probably not\n> > wanted. The previous implementation is actually correct, expect that I fall\n> > into the empty sizes case.\n> \n> One thing you could do is to simply 'continue' if no size is\n> associated with a media bus code. I presume we bailed-out to be able\n> to catch errors returned by enumPadSizes(), which in case you decide\n> to 'continue' would be missed.\n> \n> Alternatively, the enumPad[Code|Size] could be changed to accept a\n> vector of [unsigned int|SizeRange] respectively as output parameter\n> and return an error code to be able to distinguish an error condition\n> from an empty list of sizes.\n> \n> Anyway, I would first make sure if the driver behaviour is intended or\n> not.\n> \n> > > >   \t\tconst auto inserted = formats.insert({ code, sizes });\n> > > >   \t\tif (!inserted.second) {","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 C688ABD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 11 Mar 2021 11:42:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 46865602ED;\n\tThu, 11 Mar 2021 12:42:35 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EB933602E8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 11 Mar 2021 12:42:32 +0100 (CET)","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 58D43879;\n\tThu, 11 Mar 2021 12:42:32 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BhGaTDAG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615462952;\n\tbh=R5BQuyA+MKonUZcF8r5KYqRBTYRSxUwavxTmemAWKew=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BhGaTDAGeGCR7m8GkDAQjGBatc4UrPl0ihRNt+tk6h4QQFehUrfTRSG4GC5Uk4215\n\tJsY3gAnSy2BqtOeGvwz/zXdy5UQ50E1kaEWEVc2w7P5S1lH79OED2ax+Mm6zAM8xMN\n\tPfdICqRC7zEkuWJQ+vuUIGix/vR84WDlt13RIbQQ=","Date":"Thu, 11 Mar 2021 13:41:58 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YEoCBuHY9qfRrg1X@pendragon.ideasonboard.com>","References":"<20210309144457.32412-1-m.cichy@pengutronix.de>\n\t<20210311081642.r2p54tqqvtj3nqbe@uno.localdomain>\n\t<ba2a11f2-ff89-d7e9-adbd-12a3eca4bd57@pengutronix.de>\n\t<20210311112805.lg7tzgseu3xypczi@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210311112805.lg7tzgseu3xypczi@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Fix\n\tformats() returning only sizes for last mbus code","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>","Cc":"libcamera-devel@lists.libcamera.org, graphics@pengutronix.de","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]