[libcamera-devel] [PATCH 06/13] libcamera: v4l2_subdevice: Change V4L2Subdevice::Whence

[Kieran Bingham]

Quoting Laurent Pinchart via libcamera-devel (2022-08-02 11:35:33)
> Hi Jacopo,
> 
> Thank you for the patch :-)
> 
> On Mon, Aug 01, 2022 at 03:05:36AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > From: Jacopo Mondi <jacopo at jmondi.org>
> > 
> > The V4L2Subdevice::Whence enumerations defines two values that should
> > correspond to the V4L2_SUBDEV_FORMAT_ACTIVE and V4L2_SUBDEV_FORMAT_TRY
> > definitions.
> > 
> > The V4L2 symbols are defined as:
> > 
> >       V4L2_SUBDEV_FORMAT_TRY = 0,
> >       V4L2_SUBDEV_FORMAT_ACTIVE = 1,
> > 
> > While the libcamera defined enumeration is:
> > 
> >       enum Whence {
> >               ActiveFormat,
> >               TryFormat,
> >       }
> > 
> > As V4L2Subdevice::Whence values are used to populate data types
> > defined in v4l2-subdev.h used as arguments to VIDIOC_SUBDEV_*
> > ioctls, the V4L2Subdevice class is required to adjust their value as:
> > 
> >        subdevFmt.which = whence == ActiveFormat ? V4L2_SUBDEV_FORMAT_ACTIVE
> >                        : V4L2_SUBDEV_FORMAT_TRY;
> > 
> > Drop the adjustment by defining :
> > 
> >       Whence::TryFormat = V4L2_SUBDEV_FORMAT_TRY;
> >       Whence::ActiveFormat = V4L2_SUBDEV_FORMAT_ACTIVE;
> > 
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

This makes sense to me too!


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> 
> > ---
> >  include/libcamera/internal/v4l2_subdevice.h | 6 ++++--
> >  src/libcamera/v4l2_subdevice.cpp            | 6 ++----
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> > index 6fda52ada41b..2db392d5e37a 100644
> > --- a/include/libcamera/internal/v4l2_subdevice.h
> > +++ b/include/libcamera/internal/v4l2_subdevice.h
> > @@ -13,6 +13,8 @@
> >  #include <string>
> >  #include <vector>
> >  
> > +#include <linux/v4l2-subdev.h>
> > +
> >  #include <libcamera/base/class.h>
> >  #include <libcamera/base/log.h>
> >  
> > @@ -44,8 +46,8 @@ public:
> >       using Formats = std::map<unsigned int, std::vector<SizeRange>>;
> >  
> >       enum Whence {
> > -             ActiveFormat,
> > -             TryFormat,
> > +             TryFormat = V4L2_SUBDEV_FORMAT_TRY,
> > +             ActiveFormat = V4L2_SUBDEV_FORMAT_ACTIVE,
> >       };
> >  
> >       explicit V4L2Subdevice(const MediaEntity *entity);
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index 98a3911aa7df..37960b76a044 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -416,8 +416,7 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> >                            Whence whence)
> >  {
> >       struct v4l2_subdev_format subdevFmt = {};
> > -     subdevFmt.which = whence == ActiveFormat ? V4L2_SUBDEV_FORMAT_ACTIVE
> > -                     : V4L2_SUBDEV_FORMAT_TRY;
> > +     subdevFmt.which = whence;
> >       subdevFmt.pad = pad;
> >  
> >       int ret = ioctl(VIDIOC_SUBDEV_G_FMT, &subdevFmt);
> > @@ -452,8 +451,7 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> >                            Whence whence)
> >  {
> >       struct v4l2_subdev_format subdevFmt = {};
> > -     subdevFmt.which = whence == ActiveFormat ? V4L2_SUBDEV_FORMAT_ACTIVE
> > -                     : V4L2_SUBDEV_FORMAT_TRY;
> > +     subdevFmt.which = whence;
> >       subdevFmt.pad = pad;
> >       subdevFmt.format.width = format->size.width;
> >       subdevFmt.format.height = format->size.height;
> 
> -- 
> Regards,
> 
> Laurent Pinchart