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

368 diff mbox series

Add function stubs for set/get format to V4L2Device.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/include/v4l2_device.h |  9 ++++++
 src/libcamera/v4l2_device.cpp       | 49 +++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

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@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 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.

> +	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@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

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@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@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund