[libcamera-devel,06/13] libcamera: v4l2_subdevice: Change V4L2Subdevice::Whence
diff mbox series

Message ID 20220801000543.3501-7-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: pipeline: simple: Support the NXP i.MX8 ISI
Related show

Commit Message

Laurent Pinchart Aug. 1, 2022, 12:05 a.m. UTC 
From: Jacopo Mondi <jacopo@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@jmondi.org>
---
 include/libcamera/internal/v4l2_subdevice.h | 6 ++++--
 src/libcamera/v4l2_subdevice.cpp            | 6 ++----
 2 files changed, 6 insertions(+), 6 deletions(-)

Comments

Laurent Pinchart Aug. 2, 2022, 10:35 a.m. UTC | #1 
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@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@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@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;
Kieran Bingham Aug. 3, 2022, 11:55 a.m. UTC | #2 
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@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@jmondi.org>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

This makes sense to me too!


Reviewed-by: Kieran Bingham <kieran.bingham@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

Patch
diff mbox series 

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;