[libcamera-devel] [PATCH 1/5] libcamera: v4l2_subdevice: Make crop/compose rectangle a reference
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Feb 22 00:31:54 CET 2019
On Thu, Feb 21, 2019 at 04:54:05PM +0100, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your patch.
>
> On 2019-02-20 14:17:53 +0100, Jacopo Mondi wrote:
> > As the crop/compose rectangle provided to setCrop and setCompose methods
> > is never modified, make it a const reference.
>
> I think this is the wrong thing to do. Instead I think you should change
> this patch to update the rectangle after VIDIOC_SUBDEV_S_SELECTION.
>
> The kernel can update the requested selection rectangle and the caller
> should be made aware of this. Look at V4L2Subdevice::setFormat() which
> have a similar pattern but for VIDIOC_SUBDEV_S_FMT.
Agreed, I was about to say the same.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > src/libcamera/include/v4l2_subdevice.h | 6 +++---
> > src/libcamera/v4l2_subdevice.cpp | 18 +++++++++---------
> > 2 files changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> > index 40becd0ca99b..18000d6ed06b 100644
> > --- a/src/libcamera/include/v4l2_subdevice.h
> > +++ b/src/libcamera/include/v4l2_subdevice.h
> > @@ -44,8 +44,8 @@ public:
> > std::string deviceNode() const { return deviceNode_; }
> > std::string deviceName() const { return deviceName_; }
> >
> > - int setCrop(unsigned int pad, Rectangle *rect);
> > - int setCompose(unsigned int pad, Rectangle *rect);
> > + int setCrop(unsigned int pad, const Rectangle &rect);
> > + int setCompose(unsigned int pad, const Rectangle &rect);
> >
> > int enumFormat(unsigned int pad, V4L2SubdeviceFormatEnum *formatEnum);
> > int getFormat(unsigned int pad, V4L2SubdeviceFormat *format);
> > @@ -53,7 +53,7 @@ public:
> >
> > private:
> > int setSelection(unsigned int pad, unsigned int target,
> > - Rectangle *rect);
> > + const Rectangle &rect);
> >
> > std::string deviceNode_;
> > std::string deviceName_;
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index 4411ffa51460..2e73edee68c9 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -203,11 +203,11 @@ void V4L2Subdevice::close()
> > /**
> > * \brief Set a crop rectangle on one of the V4L2 subdevice pads
> > * \param[in] pad The 0-indexed pad number the rectangle is to be applied to
> > - * \param[inout] rect The rectangle describing crop target area
> > + * \param[in] rect The rectangle describing crop target area
> > *
> > * \return 0 on success, or a negative error code otherwise
> > */
> > -int V4L2Subdevice::setCrop(unsigned int pad, Rectangle *rect)
> > +int V4L2Subdevice::setCrop(unsigned int pad, const Rectangle &rect)
> > {
> > return setSelection(pad, V4L2_SEL_TGT_CROP, rect);
> > }
> > @@ -215,11 +215,11 @@ int V4L2Subdevice::setCrop(unsigned int pad, Rectangle *rect)
> > /**
> > * \brief Set a compose rectangle on one of the V4L2 subdevice pads
> > * \param[in] pad The 0-indexed pad number the rectangle is to be applied to
> > - * \param[inout] rect The rectangle describing the compose target area
> > + * \param[in] rect The rectangle describing the compose target area
> > *
> > * \return 0 on success, or a negative error code otherwise
> > */
> > -int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)
> > +int V4L2Subdevice::setCompose(unsigned int pad, const Rectangle &rect)
> > {
> > return setSelection(pad, V4L2_SEL_TGT_COMPOSE, rect);
> > }
> > @@ -333,7 +333,7 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format)
> > }
> >
> > int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> > - Rectangle *rect)
> > + const Rectangle &rect)
> > {
> > struct v4l2_subdev_selection sel = {};
> >
> > @@ -342,10 +342,10 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> > sel.target = target;
> > sel.flags = 0;
> >
> > - sel.r.left = rect->y;
> > - sel.r.top = rect->x;
> > - sel.r.width = rect->w;
> > - sel.r.height = rect->h;
> > + sel.r.left = rect.y;
> > + sel.r.top = rect.x;
> > + sel.r.width = rect.w;
> > + sel.r.height = rect.h;
> >
> > int ret = ioctl(fd_, VIDIOC_SUBDEV_S_SELECTION, &sel);
> > if (ret < 0) {
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list