[libcamera-devel] [PATCH v2 3/6] libcamera: v4l2_device: Add META support in g/s_fmt
Jacopo Mondi
jacopo at jmondi.org
Sun Jun 2 14:26:46 CEST 2019
Hi Laurent,
On Tue, May 28, 2019 at 05:36:01PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Mon, May 27, 2019 at 11:05:56AM +0200, Jacopo Mondi wrote:
> > Add two operations to set and get format on devices implementing the
> > V4L2 Metadata Interface, identified by the META_OUTPUT or META_CAPTURE
> > capabilities.
> >
> > While at it, sort get/setFormat operations alphabetically and unify
> > their style (eg. no empty line before ioctl).
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > src/libcamera/include/v4l2_device.h | 7 +-
> > src/libcamera/v4l2_device.cpp | 108 +++++++++++++++++++++-------
> > 2 files changed, 89 insertions(+), 26 deletions(-)
> >
> > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> > index cecafa151caa..bdecc087fe5a 100644
> > --- a/src/libcamera/include/v4l2_device.h
> > +++ b/src/libcamera/include/v4l2_device.h
> > @@ -150,12 +150,15 @@ protected:
> > std::string logPrefix() const;
> >
> > private:
> > - int getFormatSingleplane(V4L2DeviceFormat *format);
> > - int setFormatSingleplane(V4L2DeviceFormat *format);
> > + int getFormatMeta(V4L2DeviceFormat *format);
> > + int setFormatMeta(V4L2DeviceFormat *format);
> >
> > int getFormatMultiplane(V4L2DeviceFormat *format);
> > int setFormatMultiplane(V4L2DeviceFormat *format);
> >
> > + int getFormatSingleplane(V4L2DeviceFormat *format);
> > + int setFormatSingleplane(V4L2DeviceFormat *format);
> > +
> > int requestBuffers(unsigned int count);
> > int createPlane(Buffer *buffer, unsigned int plane,
> > unsigned int length);
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index e42f6ef0c340..c9251da465af 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -428,8 +428,12 @@ std::string V4L2Device::logPrefix() const
> > */
> > int V4L2Device::getFormat(V4L2DeviceFormat *format)
> > {
> > - return caps_.isMultiplanar() ? getFormatMultiplane(format) :
> > - getFormatSingleplane(format);
> > + if (caps_.isMeta())
> > + return getFormatMeta(format);
> > + else if (caps_.isMultiplanar())
> > + return getFormatMultiplane(format);
> > + else
> > + return getFormatSingleplane(format);
> > }
> >
> > /**
> > @@ -443,14 +447,18 @@ int V4L2Device::getFormat(V4L2DeviceFormat *format)
> > */
> > int V4L2Device::setFormat(V4L2DeviceFormat *format)
> > {
> > - return caps_.isMultiplanar() ? setFormatMultiplane(format) :
> > - setFormatSingleplane(format);
> > + if (caps_.isMeta())
> > + return setFormatMeta(format);
> > + else if (caps_.isMultiplanar())
> > + return setFormatMultiplane(format);
> > + else
> > + return setFormatSingleplane(format);
> > }
> >
> > -int V4L2Device::getFormatSingleplane(V4L2DeviceFormat *format)
> > +int V4L2Device::getFormatMeta(V4L2DeviceFormat *format)
> > {
> > struct v4l2_format v4l2Format = {};
> > - struct v4l2_pix_format *pix = &v4l2Format.fmt.pix;
> > + struct v4l2_meta_format *pix = &v4l2Format.fmt.meta;
> > int ret;
> >
> > v4l2Format.type = bufferType_;
> > @@ -461,29 +469,24 @@ int V4L2Device::getFormatSingleplane(V4L2DeviceFormat *format)
> > return ret;
> > }
> >
> > - format->size.width = pix->width;
> > - format->size.height = pix->height;
> > - format->fourcc = pix->pixelformat;
> > + format->size.width = 0;
> > + format->size.height = 0;
> > + format->fourcc = pix->dataformat;
> > format->planesCount = 1;
> > - format->planes[0].bpl = pix->bytesperline;
> > - format->planes[0].size = pix->sizeimage;
> > + format->planes[0].bpl = pix->buffersize;
> > + format->planes[0].size = pix->buffersize;
> >
> > return 0;
> > }
> >
> > -int V4L2Device::setFormatSingleplane(V4L2DeviceFormat *format)
> > +int V4L2Device::setFormatMeta(V4L2DeviceFormat *format)
> > {
> > struct v4l2_format v4l2Format = {};
> > - struct v4l2_pix_format *pix = &v4l2Format.fmt.pix;
> > + struct v4l2_meta_format *pix = &v4l2Format.fmt.meta;
> > int ret;
> >
> > v4l2Format.type = bufferType_;
> > - pix->width = format->size.width;
> > - pix->height = format->size.height;
> > - pix->pixelformat = format->fourcc;
> > - pix->bytesperline = format->planes[0].bpl;
> > - pix->field = V4L2_FIELD_NONE;
> > -
> > + pix->dataformat = format->fourcc;
>
> Shouldn't you set bytesperline ?
If you mean pix->buffersize, yes I should. It went unnoticed on IPU3 as the
size for both the metadata nodes is ignored and harcoded.
With this fixed and your ack collected, I have now pushed the first 3
patches of the series to master.
Thanks
j
>
> With that fixed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > ret = ioctl(fd_, VIDIOC_S_FMT, &v4l2Format);
> > if (ret) {
> > ret = -errno;
> > @@ -495,12 +498,12 @@ int V4L2Device::setFormatSingleplane(V4L2DeviceFormat *format)
> > * Return to caller the format actually applied on the device,
> > * which might differ from the requested one.
> > */
> > - format->size.width = pix->width;
> > - format->size.height = pix->height;
> > - format->fourcc = pix->pixelformat;
> > + format->size.width = 0;
> > + format->size.height = 0;
> > + format->fourcc = format->fourcc;
> > format->planesCount = 1;
> > - format->planes[0].bpl = pix->bytesperline;
> > - format->planes[0].size = pix->sizeimage;
> > + format->planes[0].bpl = pix->buffersize;
> > + format->planes[0].size = pix->buffersize;
> >
> > return 0;
> > }
> > @@ -573,6 +576,63 @@ int V4L2Device::setFormatMultiplane(V4L2DeviceFormat *format)
> > return 0;
> > }
> >
> > +int V4L2Device::getFormatSingleplane(V4L2DeviceFormat *format)
> > +{
> > + struct v4l2_format v4l2Format = {};
> > + struct v4l2_pix_format *pix = &v4l2Format.fmt.pix;
> > + int ret;
> > +
> > + v4l2Format.type = bufferType_;
> > + ret = ioctl(fd_, VIDIOC_G_FMT, &v4l2Format);
> > + if (ret) {
> > + ret = -errno;
> > + LOG(V4L2, Error) << "Unable to get format: " << strerror(-ret);
> > + return ret;
> > + }
> > +
> > + format->size.width = pix->width;
> > + format->size.height = pix->height;
> > + format->fourcc = pix->pixelformat;
> > + format->planesCount = 1;
> > + format->planes[0].bpl = pix->bytesperline;
> > + format->planes[0].size = pix->sizeimage;
> > +
> > + return 0;
> > +}
> > +
> > +int V4L2Device::setFormatSingleplane(V4L2DeviceFormat *format)
> > +{
> > + struct v4l2_format v4l2Format = {};
> > + struct v4l2_pix_format *pix = &v4l2Format.fmt.pix;
> > + int ret;
> > +
> > + v4l2Format.type = bufferType_;
> > + pix->width = format->size.width;
> > + pix->height = format->size.height;
> > + pix->pixelformat = format->fourcc;
> > + pix->bytesperline = format->planes[0].bpl;
> > + pix->field = V4L2_FIELD_NONE;
> > + ret = ioctl(fd_, VIDIOC_S_FMT, &v4l2Format);
> > + if (ret) {
> > + ret = -errno;
> > + LOG(V4L2, Error) << "Unable to set format: " << strerror(-ret);
> > + return ret;
> > + }
> > +
> > + /*
> > + * Return to caller the format actually applied on the device,
> > + * which might differ from the requested one.
> > + */
> > + format->size.width = pix->width;
> > + format->size.height = pix->height;
> > + format->fourcc = pix->pixelformat;
> > + format->planesCount = 1;
> > + format->planes[0].bpl = pix->bytesperline;
> > + format->planes[0].size = pix->sizeimage;
> > +
> > + return 0;
> > +}
> > +
> > int V4L2Device::requestBuffers(unsigned int count)
> > {
> > struct v4l2_requestbuffers rb = {};
>
> --
> 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/20190602/ea56f9e5/attachment.sig>
More information about the libcamera-devel
mailing list