[libcamera-devel] [PATCH] libcamera: Fix CameraSensor::getFormat() search order
Jacopo Mondi
jacopo at jmondi.org
Wed Jun 12 10:22:10 CEST 2019
Hi Laurent
On Wed, Jun 12, 2019 at 11:12:00AM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Wed, Jun 12, 2019 at 10:00:46AM +0200, Jacopo Mondi wrote:
> > On Tue, Jun 11, 2019 at 04:25:52PM +0300, Laurent Pinchart wrote:
> > > On Tue, Jun 11, 2019 at 02:50:18PM +0200, Jacopo Mondi wrote:
> > >> On Tue, Jun 11, 2019 at 01:37:11PM +0200, Mickael Guene wrote:
> > >>> According to the documentation, CameraSensor::getFormat() should select Media
> > >>> bus code from mbusCodes parameter list. It should select the first code from the
> > >>
> > >> s/from mbusCodes parameter list
> > >> /from mbusCodes parameters in descresing preference order/
> > >
> > > This is redundant with the second sentence.
> > >
> > >>> list that is supported by the sensor. Current implementation will wrongly select
> > >>
> > >> s/Current/However, the current/
> > >>
> > >>> Media bus code from mbusCodes_ order instead.
> > >>> This patch aims to fix this wrong behavior.
> > >>
> > >> I would change the last line with
> > >>
> > >> "Fix this by first iterating on the mbus code list provided as parameter
> > >> instead of iterating on the sensor supported formats".
> > >>
> > >> If you agree with the change to the commit message, it could be
> > >> updated when applying the patch.
> > >
> > > I propose the following text.
> > >
> > > "According to the documentation, the CameraSensor::getFormat() method
> > > should select the first media bus code from the mbusCodes parameter that
> > > is supported by the sensor. However, the current implementation wrongly
> > > selects the first media bus code from the codes supported by the sensor
> > > that is listed in the mbusCodes parameter. This results in the
> > > preference order specified by the caller being ignored. Fix it."
> >
> > Agreed. Would you like to give your tag or should I apply the patch
> > right away?
>
> I'm pushed it already :-)
>
Hope you have not been pushed too hard :p
Thanks, I didn't notice it was on master already.
Thanks
j
> > >>> Signed-off-by: Mickael Guene <mickael.guene at st.com>
> > >>
> > >> Following this morning clarification on irc:
> > >> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> > >>
> > >>> ---
> > >>>
> > >>> src/libcamera/camera_sensor.cpp | 4 ++--
> > >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > >>> index 2b9d8fa..cb6649e 100644
> > >>> --- a/src/libcamera/camera_sensor.cpp
> > >>> +++ b/src/libcamera/camera_sensor.cpp
> > >>> @@ -191,8 +191,8 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu
> > >>> {
> > >>> V4L2SubdeviceFormat format{};
> > >>>
> > >>> - for (unsigned int code : mbusCodes_) {
> > >>> - if (std::any_of(mbusCodes.begin(), mbusCodes.end(),
> > >>> + for (unsigned int code : mbusCodes) {
> > >>> + if (std::any_of(mbusCodes_.begin(), mbusCodes_.end(),
> > >>> [code](unsigned int c) { return c == code; })) {
> > >>> format.mbus_code = code;
> > >>> break;
>
> --
> Regards,
>
> Laurent Pinchart
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190612/db4399d1/attachment.sig>
More information about the libcamera-devel
mailing list