[{"id":1844,"web_url":"https://patchwork.libcamera.org/comment/1844/","msgid":"<20190611125018.jwvlvzcmntdzgwnc@uno.localdomain>","date":"2019-06-11T12:50:18","subject":"Re: [libcamera-devel] [PATCH] libcamera: Fix\n\tCameraSensor::getFormat() search order","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Mickael,\n   thanks for the patch\n\nOn Tue, Jun 11, 2019 at 01:37:11PM +0200, Mickael Guene wrote:\n>  According to the documentation, CameraSensor::getFormat() should select Media\n> bus code from mbusCodes parameter list. It should select the first code from the\ns/from mbusCodes parameter list\n /from mbusCodes parameters in descresing preference order/\n> list that is supported by the sensor. Current implementation will wrongly select\n\ns/Current/However, the current/\n\n> Media bus code from mbusCodes_ order instead.\n>  This patch aims to fix this wrong behavior.\n\nI would change the last line with\n\n\"Fix this by first iterating on the mbus code list provided as parameter\ninstead of iterating on the sensor supported formats\".\n\nIf you agree with the change to the commit message, it could be\nupdated when applying the patch.\n\n>\n> Signed-off-by: Mickael Guene <mickael.guene@st.com>\n\nFollowing this morning clarification on irc:\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n\n> ---\n>\n>  src/libcamera/camera_sensor.cpp | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 2b9d8fa..cb6649e 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -191,8 +191,8 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n>  {\n>  \tV4L2SubdeviceFormat format{};\n>\n> -\tfor (unsigned int code : mbusCodes_) {\n> -\t\tif (std::any_of(mbusCodes.begin(), mbusCodes.end(),\n> +\tfor (unsigned int code : mbusCodes) {\n> +\t\tif (std::any_of(mbusCodes_.begin(), mbusCodes_.end(),\n>  \t\t\t\t[code](unsigned int c) { return c == code; })) {\n>  \t\t\tformat.mbus_code = code;\n>  \t\t\tbreak;\n> --\n> 2.7.4\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 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 7F80462F95\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 11 Jun 2019 14:49:06 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 13AC76000C;\n\tTue, 11 Jun 2019 12:49:05 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 11 Jun 2019 14:50:18 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Mickael Guene <mickael.guene@st.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190611125018.jwvlvzcmntdzgwnc@uno.localdomain>","References":"<1560253031-98823-1-git-send-email-mickael.guene@st.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"mudqfcz2qbudxakf\"","Content-Disposition":"inline","In-Reply-To":"<1560253031-98823-1-git-send-email-mickael.guene@st.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH] libcamera: Fix\n\tCameraSensor::getFormat() search order","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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":"Tue, 11 Jun 2019 12:49:06 -0000"}},{"id":1848,"web_url":"https://patchwork.libcamera.org/comment/1848/","msgid":"<20190611132552.GA21796@pendragon.ideasonboard.com>","date":"2019-06-11T13:25:52","subject":"Re: [libcamera-devel] [PATCH] libcamera: Fix\n\tCameraSensor::getFormat() search order","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Jun 11, 2019 at 02:50:18PM +0200, Jacopo Mondi wrote:\n> On Tue, Jun 11, 2019 at 01:37:11PM +0200, Mickael Guene wrote:\n> >  According to the documentation, CameraSensor::getFormat() should select Media\n> > bus code from mbusCodes parameter list. It should select the first code from the\n> \n> s/from mbusCodes parameter list\n>  /from mbusCodes parameters in descresing preference order/\n\nThis is redundant with the second sentence.\n\n> > list that is supported by the sensor. Current implementation will wrongly select\n> \n> s/Current/However, the current/\n> \n> > Media bus code from mbusCodes_ order instead.\n> >  This patch aims to fix this wrong behavior.\n> \n> I would change the last line with\n> \n> \"Fix this by first iterating on the mbus code list provided as parameter\n> instead of iterating on the sensor supported formats\".\n> \n> If you agree with the change to the commit message, it could be\n> updated when applying the patch.\n\nI propose the following text.\n\n\"According to the documentation, the CameraSensor::getFormat() method \nshould select the first media bus code from the mbusCodes parameter that \nis supported by the sensor. However, the current implementation wrongly\nselects the first media bus code from the codes supported by the sensor\nthat is listed in the mbusCodes parameter. This results in the\npreference order specified by the caller being ignored. Fix it.\"\n\n> > Signed-off-by: Mickael Guene <mickael.guene@st.com>\n> \n> Following this morning clarification on irc:\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> \n> > ---\n> >\n> >  src/libcamera/camera_sensor.cpp | 4 ++--\n> >  1 file changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 2b9d8fa..cb6649e 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -191,8 +191,8 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n> >  {\n> >  \tV4L2SubdeviceFormat format{};\n> >\n> > -\tfor (unsigned int code : mbusCodes_) {\n> > -\t\tif (std::any_of(mbusCodes.begin(), mbusCodes.end(),\n> > +\tfor (unsigned int code : mbusCodes) {\n> > +\t\tif (std::any_of(mbusCodes_.begin(), mbusCodes_.end(),\n> >  \t\t\t\t[code](unsigned int c) { return c == code; })) {\n> >  \t\t\tformat.mbus_code = code;\n> >  \t\t\tbreak;","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 B703162F96\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 11 Jun 2019 15:26:07 +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 3127DFA0;\n\tTue, 11 Jun 2019 15:26:07 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1560259567;\n\tbh=D9oz53BDaLJIhEOOWJT1HvbeUJw/A4NB1uvQ6yVV4z0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OGJh5YP83p56bUtKMmUTFDe+Xu5OgN3lYFbyJFfDfgqLbWlO6X8J0iK4+T++P0snk\n\tKME+aqIWYdTbpU0XJ8Xw+ga0dihN0NaKSodH3zstrcUtfrHRKIeZ7a+fWWNcQec6I8\n\tYTvBTryEbcT4Rcu/V25hcjlgJ04i8yaPCuO1ddzU=","Date":"Tue, 11 Jun 2019 16:25:52 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"Mickael Guene <mickael.guene@st.com>, libcamera-devel@lists.libcamera.org","Message-ID":"<20190611132552.GA21796@pendragon.ideasonboard.com>","References":"<1560253031-98823-1-git-send-email-mickael.guene@st.com>\n\t<20190611125018.jwvlvzcmntdzgwnc@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190611125018.jwvlvzcmntdzgwnc@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: Fix\n\tCameraSensor::getFormat() search order","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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":"Tue, 11 Jun 2019 13:26:07 -0000"}},{"id":1850,"web_url":"https://patchwork.libcamera.org/comment/1850/","msgid":"<1454a110-3931-34df-1edd-de1d23518db0@st.com>","date":"2019-06-11T13:36:50","subject":"Re: [libcamera-devel] [PATCH] libcamera: Fix\n\tCameraSensor::getFormat() search order","submitter":{"id":19,"url":"https://patchwork.libcamera.org/api/people/19/","name":"Mickael GUENE","email":"mickael.guene@st.com"},"content":"Hi Jacopo,\n\n It's ok for me if you update with commit message changed.\n\nregards\nMickael\n\nOn 6/11/19 14:50, Jacopo Mondi wrote:\n> Hi Mickael,\n>    thanks for the patch\n> \n> On Tue, Jun 11, 2019 at 01:37:11PM +0200, Mickael Guene wrote:\n>>  According to the documentation, CameraSensor::getFormat() should select Media\n>> bus code from mbusCodes parameter list. It should select the first code from the\n> s/from mbusCodes parameter list\n>  /from mbusCodes parameters in descresing preference order/\n>> list that is supported by the sensor. Current implementation will wrongly select\n> \n> s/Current/However, the current/\n> \n>> Media bus code from mbusCodes_ order instead.\n>>  This patch aims to fix this wrong behavior.\n> \n> I would change the last line with\n> \n> \"Fix this by first iterating on the mbus code list provided as parameter\n> instead of iterating on the sensor supported formats\".\n> \n> If you agree with the change to the commit message, it could be\n> updated when applying the patch.","headers":{"Return-Path":"<mickael.guene@st.com>","Received":["from mx07-00178001.pphosted.com (mx08-00178001.pphosted.com\n\t[91.207.212.93])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 803FA62FA8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 11 Jun 2019 15:36:53 +0200 (CEST)","from pps.filterd (m0046660.ppops.net [127.0.0.1])\n\tby mx08-00178001.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id\n\tx5BDaI9i031804; Tue, 11 Jun 2019 15:36:52 +0200","from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35])\n\tby mx08-00178001.pphosted.com with ESMTP id 2t2a4y10h7-1\n\t(version=TLSv1 cipher=ECDHE-RSA-AES256-SHA bits=256 verify=NOT);\n\tTue, 11 Jun 2019 15:36:52 +0200","from zeta.dmz-eu.st.com (zeta.dmz-eu.st.com [164.129.230.9])\n\tby beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 6D4303D;\n\tTue, 11 Jun 2019 13:36:51 +0000 (GMT)","from Webmail-eu.st.com (sfhdag5node1.st.com [10.75.127.13])\n\tby zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 0731F2BFB;\n\tTue, 11 Jun 2019 13:36:51 +0000 (GMT)","from SFHDAG5NODE3.st.com (10.75.127.15) by SFHDAG5NODE1.st.com\n\t(10.75.127.13) with Microsoft SMTP Server (TLS) id 15.0.1347.2;\n\tTue, 11 Jun 2019 15:36:50 +0200","from SFHDAG5NODE3.st.com ([fe80::7c09:5d6b:d2c7:5f47]) by\n\tSFHDAG5NODE3.st.com ([fe80::7c09:5d6b:d2c7:5f47%20]) with mapi id\n\t15.00.1473.003; Tue, 11 Jun 2019 15:36:50 +0200"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=st.com;\n\th=from : to : cc : subject\n\t: date : message-id : references : in-reply-to : content-type :\n\tcontent-id\n\t: content-transfer-encoding : mime-version; s=STMicroelectronics;\n\tbh=cLvb+amADCcfHYvshmZPw7fXIW7BBbBsm7aNxpjOGLw=;\n\tb=uPBw0ZuRDtRV6w2PZmiwBY6Da4MDH+xrp7oBdfXVI/h1optFbGTANwCJ8LJZRHoi1UDc\n\t7aNurSqfZxFiRwCD985niCWgffqnvFIGzrfB1IKD/ZdBWTwx9Q3gpGH++L1ntnLhoNTH\n\tqNZBhtoy/ZR/4SASdxHZigs5lYp5bqG5JEZtqsoYGWoDWhh38m/ZGwl+o1s0YhOwzMEo\n\tZbk1mPpx1onhWc49bOIhcAnK/yf6rnFBxK5AAtFuAew+y9SRpWFv5EGV/q7SOxDTS0eD\n\tEd5KNVDlqwpJyYxi9eZFUXzama/CjCjbTlJky2f9CJv9TMElNJOhwrlalZFxfLOVLRnC\n\tiQ== ","From":"Mickael GUENE <mickael.guene@st.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","CC":"\"libcamera-devel@lists.libcamera.org\"\n\t<libcamera-devel@lists.libcamera.org>","Thread-Topic":"[libcamera-devel] [PATCH] libcamera: Fix\n\tCameraSensor::getFormat() search order","Thread-Index":"AQHVIFQQdW6YX3nIgE6nDSfAOiTuaqaWU0IA","Date":"Tue, 11 Jun 2019 13:36:50 +0000","Message-ID":"<1454a110-3931-34df-1edd-de1d23518db0@st.com>","References":"<1560253031-98823-1-git-send-email-mickael.guene@st.com>\n\t<20190611125018.jwvlvzcmntdzgwnc@uno.localdomain>","In-Reply-To":"<20190611125018.jwvlvzcmntdzgwnc@uno.localdomain>","Accept-Language":"fr-FR, en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","user-agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.0","x-ms-exchange-messagesentrepresentingtype":"1","x-ms-exchange-transport-fromentityheader":"Hosted","x-originating-ip":"[10.75.127.50]","Content-Type":"text/plain; charset=\"utf-8\"","Content-ID":"<3271E17FEB70544A995E496A73C79D8B@st.com>","Content-Transfer-Encoding":"base64","MIME-Version":"1.0","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10434:, ,\n\tdefinitions=2019-06-11_06:, , signatures=0","X-Mailman-Approved-At":"Tue, 11 Jun 2019 15:44:59 +0200","Subject":"Re: [libcamera-devel] [PATCH] libcamera: Fix\n\tCameraSensor::getFormat() search order","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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":"Tue, 11 Jun 2019 13:36:53 -0000"}},{"id":1867,"web_url":"https://patchwork.libcamera.org/comment/1867/","msgid":"<20190612080046.p56rylgcj5mt3akp@uno.localdomain>","date":"2019-06-12T08:00:46","subject":"Re: [libcamera-devel] [PATCH] libcamera: Fix\n\tCameraSensor::getFormat() search order","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Jun 11, 2019 at 04:25:52PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Tue, Jun 11, 2019 at 02:50:18PM +0200, Jacopo Mondi wrote:\n> > On Tue, Jun 11, 2019 at 01:37:11PM +0200, Mickael Guene wrote:\n> > >  According to the documentation, CameraSensor::getFormat() should select Media\n> > > bus code from mbusCodes parameter list. It should select the first code from the\n> >\n> > s/from mbusCodes parameter list\n> >  /from mbusCodes parameters in descresing preference order/\n>\n> This is redundant with the second sentence.\n>\n> > > list that is supported by the sensor. Current implementation will wrongly select\n> >\n> > s/Current/However, the current/\n> >\n> > > Media bus code from mbusCodes_ order instead.\n> > >  This patch aims to fix this wrong behavior.\n> >\n> > I would change the last line with\n> >\n> > \"Fix this by first iterating on the mbus code list provided as parameter\n> > instead of iterating on the sensor supported formats\".\n> >\n> > If you agree with the change to the commit message, it could be\n> > updated when applying the patch.\n>\n> I propose the following text.\n>\n> \"According to the documentation, the CameraSensor::getFormat() method\n> should select the first media bus code from the mbusCodes parameter that\n> is supported by the sensor. However, the current implementation wrongly\n> selects the first media bus code from the codes supported by the sensor\n> that is listed in the mbusCodes parameter. This results in the\n> preference order specified by the caller being ignored. Fix it.\"\n>\n\nAgreed. Would you like to give your tag or should I apply the patch\nright away?\n\nThanks\n  j\n\n> > > Signed-off-by: Mickael Guene <mickael.guene@st.com>\n> >\n> > Following this morning clarification on irc:\n> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> >\n> > > ---\n> > >\n> > >  src/libcamera/camera_sensor.cpp | 4 ++--\n> > >  1 file changed, 2 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > index 2b9d8fa..cb6649e 100644\n> > > --- a/src/libcamera/camera_sensor.cpp\n> > > +++ b/src/libcamera/camera_sensor.cpp\n> > > @@ -191,8 +191,8 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n> > >  {\n> > >  \tV4L2SubdeviceFormat format{};\n> > >\n> > > -\tfor (unsigned int code : mbusCodes_) {\n> > > -\t\tif (std::any_of(mbusCodes.begin(), mbusCodes.end(),\n> > > +\tfor (unsigned int code : mbusCodes) {\n> > > +\t\tif (std::any_of(mbusCodes_.begin(), mbusCodes_.end(),\n> > >  \t\t\t\t[code](unsigned int c) { return c == code; })) {\n> > >  \t\t\tformat.mbus_code = code;\n> > >  \t\t\tbreak;\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 4251961B63\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 12 Jun 2019 09:59:35 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 97073E0017;\n\tWed, 12 Jun 2019 07:59:34 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 12 Jun 2019 10:00:46 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Mickael Guene <mickael.guene@st.com>, libcamera-devel@lists.libcamera.org","Message-ID":"<20190612080046.p56rylgcj5mt3akp@uno.localdomain>","References":"<1560253031-98823-1-git-send-email-mickael.guene@st.com>\n\t<20190611125018.jwvlvzcmntdzgwnc@uno.localdomain>\n\t<20190611132552.GA21796@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"vorvby5hdejogxoz\"","Content-Disposition":"inline","In-Reply-To":"<20190611132552.GA21796@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH] libcamera: Fix\n\tCameraSensor::getFormat() search order","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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, 12 Jun 2019 07:59:35 -0000"}},{"id":1868,"web_url":"https://patchwork.libcamera.org/comment/1868/","msgid":"<20190612081200.GA5035@pendragon.ideasonboard.com>","date":"2019-06-12T08:12:00","subject":"Re: [libcamera-devel] [PATCH] libcamera: Fix\n\tCameraSensor::getFormat() search order","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 12, 2019 at 10:00:46AM +0200, Jacopo Mondi wrote:\n> On Tue, Jun 11, 2019 at 04:25:52PM +0300, Laurent Pinchart wrote:\n> > On Tue, Jun 11, 2019 at 02:50:18PM +0200, Jacopo Mondi wrote:\n> >> On Tue, Jun 11, 2019 at 01:37:11PM +0200, Mickael Guene wrote:\n> >>>  According to the documentation, CameraSensor::getFormat() should select Media\n> >>> bus code from mbusCodes parameter list. It should select the first code from the\n> >>\n> >> s/from mbusCodes parameter list\n> >>  /from mbusCodes parameters in descresing preference order/\n> >\n> > This is redundant with the second sentence.\n> >\n> >>> list that is supported by the sensor. Current implementation will wrongly select\n> >>\n> >> s/Current/However, the current/\n> >>\n> >>> Media bus code from mbusCodes_ order instead.\n> >>>  This patch aims to fix this wrong behavior.\n> >>\n> >> I would change the last line with\n> >>\n> >> \"Fix this by first iterating on the mbus code list provided as parameter\n> >> instead of iterating on the sensor supported formats\".\n> >>\n> >> If you agree with the change to the commit message, it could be\n> >> updated when applying the patch.\n> >\n> > I propose the following text.\n> >\n> > \"According to the documentation, the CameraSensor::getFormat() method\n> > should select the first media bus code from the mbusCodes parameter that\n> > is supported by the sensor. However, the current implementation wrongly\n> > selects the first media bus code from the codes supported by the sensor\n> > that is listed in the mbusCodes parameter. This results in the\n> > preference order specified by the caller being ignored. Fix it.\"\n> \n> Agreed. Would you like to give your tag or should I apply the patch\n> right away?\n\nI'm pushed it already :-)\n\n> >>> Signed-off-by: Mickael Guene <mickael.guene@st.com>\n> >>\n> >> Following this morning clarification on irc:\n> >> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> >>\n> >>> ---\n> >>>\n> >>>  src/libcamera/camera_sensor.cpp | 4 ++--\n> >>>  1 file changed, 2 insertions(+), 2 deletions(-)\n> >>>\n> >>> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> >>> index 2b9d8fa..cb6649e 100644\n> >>> --- a/src/libcamera/camera_sensor.cpp\n> >>> +++ b/src/libcamera/camera_sensor.cpp\n> >>> @@ -191,8 +191,8 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n> >>>  {\n> >>>  \tV4L2SubdeviceFormat format{};\n> >>>\n> >>> -\tfor (unsigned int code : mbusCodes_) {\n> >>> -\t\tif (std::any_of(mbusCodes.begin(), mbusCodes.end(),\n> >>> +\tfor (unsigned int code : mbusCodes) {\n> >>> +\t\tif (std::any_of(mbusCodes_.begin(), mbusCodes_.end(),\n> >>>  \t\t\t\t[code](unsigned int c) { return c == code; })) {\n> >>>  \t\t\tformat.mbus_code = code;\n> >>>  \t\t\tbreak;","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 990E561B63\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 12 Jun 2019 10:12:16 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 09444336;\n\tWed, 12 Jun 2019 10:12:15 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1560327136;\n\tbh=D+PSHQni0ZNbLfX2C6y3itL0j9XMPOUbfOF7cl2GAug=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jmgA1ouSLMJ3FJIyDZCESGy5bCV7Mx+FkaYIUdk9NjECX293QLCievXYWfAAePoTC\n\tEEbc/d/z4VVGhdhBxXuX/y3AAr+V1FqYffS5t4p781NeIDrTtc9LXV2OrWFKHhYq0u\n\tNekftvGdOFYPttJ0Wt/Nsa7OQHADSHtDSwJMaW4s=","Date":"Wed, 12 Jun 2019 11:12:00 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"Mickael Guene <mickael.guene@st.com>, libcamera-devel@lists.libcamera.org","Message-ID":"<20190612081200.GA5035@pendragon.ideasonboard.com>","References":"<1560253031-98823-1-git-send-email-mickael.guene@st.com>\n\t<20190611125018.jwvlvzcmntdzgwnc@uno.localdomain>\n\t<20190611132552.GA21796@pendragon.ideasonboard.com>\n\t<20190612080046.p56rylgcj5mt3akp@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190612080046.p56rylgcj5mt3akp@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: Fix\n\tCameraSensor::getFormat() search order","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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, 12 Jun 2019 08:12:16 -0000"}},{"id":1869,"web_url":"https://patchwork.libcamera.org/comment/1869/","msgid":"<20190612081950.mb2q3x7opyyhf6ra@uno.localdomain>","date":"2019-06-12T08:22:10","subject":"Re: [libcamera-devel] [PATCH] libcamera: Fix\n\tCameraSensor::getFormat() search order","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 12, 2019 at 11:12:00AM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Wed, Jun 12, 2019 at 10:00:46AM +0200, Jacopo Mondi wrote:\n> > On Tue, Jun 11, 2019 at 04:25:52PM +0300, Laurent Pinchart wrote:\n> > > On Tue, Jun 11, 2019 at 02:50:18PM +0200, Jacopo Mondi wrote:\n> > >> On Tue, Jun 11, 2019 at 01:37:11PM +0200, Mickael Guene wrote:\n> > >>>  According to the documentation, CameraSensor::getFormat() should select Media\n> > >>> bus code from mbusCodes parameter list. It should select the first code from the\n> > >>\n> > >> s/from mbusCodes parameter list\n> > >>  /from mbusCodes parameters in descresing preference order/\n> > >\n> > > This is redundant with the second sentence.\n> > >\n> > >>> list that is supported by the sensor. Current implementation will wrongly select\n> > >>\n> > >> s/Current/However, the current/\n> > >>\n> > >>> Media bus code from mbusCodes_ order instead.\n> > >>>  This patch aims to fix this wrong behavior.\n> > >>\n> > >> I would change the last line with\n> > >>\n> > >> \"Fix this by first iterating on the mbus code list provided as parameter\n> > >> instead of iterating on the sensor supported formats\".\n> > >>\n> > >> If you agree with the change to the commit message, it could be\n> > >> updated when applying the patch.\n> > >\n> > > I propose the following text.\n> > >\n> > > \"According to the documentation, the CameraSensor::getFormat() method\n> > > should select the first media bus code from the mbusCodes parameter that\n> > > is supported by the sensor. However, the current implementation wrongly\n> > > selects the first media bus code from the codes supported by the sensor\n> > > that is listed in the mbusCodes parameter. This results in the\n> > > preference order specified by the caller being ignored. Fix it.\"\n> >\n> > Agreed. Would you like to give your tag or should I apply the patch\n> > right away?\n>\n> I'm pushed it already :-)\n>\n\nHope you have not been pushed too hard :p\n\nThanks, I didn't notice it was on master already.\n\nThanks\n  j\n\n> > >>> Signed-off-by: Mickael Guene <mickael.guene@st.com>\n> > >>\n> > >> Following this morning clarification on irc:\n> > >> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> > >>\n> > >>> ---\n> > >>>\n> > >>>  src/libcamera/camera_sensor.cpp | 4 ++--\n> > >>>  1 file changed, 2 insertions(+), 2 deletions(-)\n> > >>>\n> > >>> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > >>> index 2b9d8fa..cb6649e 100644\n> > >>> --- a/src/libcamera/camera_sensor.cpp\n> > >>> +++ b/src/libcamera/camera_sensor.cpp\n> > >>> @@ -191,8 +191,8 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n> > >>>  {\n> > >>>  \tV4L2SubdeviceFormat format{};\n> > >>>\n> > >>> -\tfor (unsigned int code : mbusCodes_) {\n> > >>> -\t\tif (std::any_of(mbusCodes.begin(), mbusCodes.end(),\n> > >>> +\tfor (unsigned int code : mbusCodes) {\n> > >>> +\t\tif (std::any_of(mbusCodes_.begin(), mbusCodes_.end(),\n> > >>>  \t\t\t\t[code](unsigned int c) { return c == code; })) {\n> > >>>  \t\t\tformat.mbus_code = code;\n> > >>>  \t\t\tbreak;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 A8D8C61E83\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 12 Jun 2019 10:20:58 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id EE05D1C0011;\n\tWed, 12 Jun 2019 08:20:56 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 12 Jun 2019 10:22:10 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Mickael Guene <mickael.guene@st.com>, libcamera-devel@lists.libcamera.org","Message-ID":"<20190612081950.mb2q3x7opyyhf6ra@uno.localdomain>","References":"<1560253031-98823-1-git-send-email-mickael.guene@st.com>\n\t<20190611125018.jwvlvzcmntdzgwnc@uno.localdomain>\n\t<20190611132552.GA21796@pendragon.ideasonboard.com>\n\t<20190612080046.p56rylgcj5mt3akp@uno.localdomain>\n\t<20190612081200.GA5035@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"53fhmvmhosdx66jk\"","Content-Disposition":"inline","In-Reply-To":"<20190612081200.GA5035@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH] libcamera: Fix\n\tCameraSensor::getFormat() search order","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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, 12 Jun 2019 08:20:58 -0000"}}]