[libcamera-devel] [PATCH v2 2/2] libcamera: v4l2_device: Add stubs to set/get format

[Jacopo Mondi]

Hi Niklas,

On Thu, Jan 24, 2019 at 07:51:32PM +0100, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2019-01-24 12:30:00 +0100, Jacopo Mondi wrote:
> > Add function stubs for set/get format to V4L2Device.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/libcamera/include/v4l2_device.h |  9 ++++++
> >  src/libcamera/v4l2_device.cpp       | 49 +++++++++++++++++++++++++++++
> >  2 files changed, 58 insertions(+)
> >
> > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> > index 413bb7f..2da30d1 100644
> > --- a/src/libcamera/include/v4l2_device.h
> > +++ b/src/libcamera/include/v4l2_device.h
> > @@ -72,10 +72,19 @@ public:
> >  	const char *deviceName() const { return caps_.card(); }
> >  	const char *busName() const { return caps_.bus_info(); }
> >
> > +	int format();
> > +	int setFormat();
> > +
> >  private:
> >  	std::string deviceNode_;
> >  	int fd_;
> >  	V4L2Capability caps_;
> > +
> > +	int getFormatSingleplane();
>
> I think this should be formatSingleplane() to match the naming
> convention.

I see. Being this a private API, I considerd this was not an issue.
But you're right, consistency first.

>
> I like the structure of this patch and I think this is the way to go. I
> do think we should hold off on merging this until we can replace the
> stubs with code that do some work. Furthermore I think you can merge 1/2
> if this series, if there are no other review comments on that in the
> near future.
>

I agree this is not useful if not as a place holder without any actual
code. If I implement the get/set format, I'll resend. I'm merging
[1/2] if no other comment arrives though.

Thanks
   j

> > +	int setFormatSingleplane();
> > +
> > +	int getFormatMultiplane();
> > +	int setFormatMultiplane();
> >  };
> >
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index 9cb504d..2ddb800 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -213,4 +213,53 @@ void V4L2Device::close()
> >   * \return The string containing the device location
> >   */
> >
> > +/**
> > + * \brief Retrieve the image format set on the V4L2 device
> > + *
> > + * TODO: define how the image format is encoded
> > + * FIXME: this function is a stub at the moment
> > + *
> > + * \return 0 for success, a negative error code otherwise
> > + */
> > +int V4L2Device::format()
> > +{
> > +	return caps_.isMultiplanar() ? getFormatMultiplane() :
> > +				       getFormatSingleplane();
> > +}
> > +
> > +/**
> > + * \brief Program an image format on the V4L2 device
> > + *
> > + * TODO: define how the image format is encoded
> > + * FIXME: this function is a stub at the moment
> > + *
> > + * \return 0 for success, a negative error code otherwise
> > + */
> > +int V4L2Device::setFormat()
> > +{
> > +	return caps_.isMultiplanar() ? setFormatMultiplane() :
> > +				       setFormatSingleplane();
> > +}
> > +
> > +/* FIXME: these functions are stubs at the moment. */
> > +int V4L2Device::getFormatSingleplane()
> > +{
> > +	return 0;
> > +}
> > +
> > +int V4L2Device::setFormatSingleplane()
> > +{
> > +	return 0;
> > +}
> > +
> > +int V4L2Device::getFormatMultiplane()
> > +{
> > +	return 0;
> > +}
> > +
> > +int V4L2Device::setFormatMultiplane()
> > +{
> > +	return 0;
> > +}
> > +
> >  } /* namespace libcamera */
> > --
> > 2.20.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund
-------------- 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/20190124/ef5e8cb8/attachment.sig>