[libcamera-devel,v4,08/21] v4l2: v4l2_camera_proxy: Use stream config in tryFormat
diff mbox series

Message ID 20200708134417.67747-9-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • Clean up formats in v4l2-compat and pipeline handlers
Related show

Commit Message

Paul Elder July 8, 2020, 1:44 p.m. UTC 
For handling try_fmt, the values should be filled in by validating the
stream configuration, and not by recalculating them or manually checking
against the cached list of formats and sizes. Use
V4L2Camera::validateConfiguration to obtain size, format, stride, and
frameSize values. Also, implement this function in V4L2Camera.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
Changes in v4:
- clean up code a bit
- squash the implementation and usage

New in v3
---
 src/v4l2/v4l2_camera.cpp       | 20 ++++++++++++++++++++
 src/v4l2/v4l2_camera.h         |  3 +++
 src/v4l2/v4l2_camera_proxy.cpp | 23 +++++++++--------------
 3 files changed, 32 insertions(+), 14 deletions(-)

Comments

Laurent Pinchart July 8, 2020, 2:59 p.m. UTC | #1 
Hi Paul,

Thank you for the patch.

On Wed, Jul 08, 2020 at 10:44:04PM +0900, Paul Elder wrote:
> For handling try_fmt, the values should be filled in by validating the
> stream configuration, and not by recalculating them or manually checking
> against the cached list of formats and sizes. Use
> V4L2Camera::validateConfiguration to obtain size, format, stride, and
> frameSize values. Also, implement this function in V4L2Camera.

Maybe "Add a new V4L2Camera::validateConfiguration() function to
validate a configuration, and use it to obtain ..." ?

As the pipeline handlers don't fill stride and frame size consistently
at this point, this may cause a bit of a regression. I would shuffle the
patches in order to 

- extend PixelFormatInfo
- add stride and frameSize to StreamConfiguration
- update the pipeline handlers
- use all this in the V4L2 compat layer

This should hopefully not cause any conflict.

> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> Changes in v4:
> - clean up code a bit
> - squash the implementation and usage
> 
> New in v3
> ---
>  src/v4l2/v4l2_camera.cpp       | 20 ++++++++++++++++++++
>  src/v4l2/v4l2_camera.h         |  3 +++
>  src/v4l2/v4l2_camera_proxy.cpp | 23 +++++++++--------------
>  3 files changed, 32 insertions(+), 14 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index ffc1230..9f81c97 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -138,6 +138,26 @@ int V4L2Camera::configure(StreamConfiguration *streamConfigOut,
>  	return 0;
>  }
>  
> +int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat,
> +				      const Size &size,
> +				      StreamConfiguration *streamConfigOut)
> +{
> +	std::unique_ptr<CameraConfiguration> config =
> +		camera_->generateConfiguration({ StreamRole::Viewfinder });
> +	StreamConfiguration &cfg = config->at(0);
> +	cfg.size = size;
> +	cfg.pixelFormat = pixelFormat;
> +	cfg.bufferCount = 1;
> +
> +	CameraConfiguration::Status validation = config->validate();
> +	if (validation == CameraConfiguration::Invalid)
> +		return -EINVAL;
> +
> +	*streamConfigOut = cfg;
> +
> +	return 0;
> +}
> +
>  int V4L2Camera::allocBuffers(unsigned int count)
>  {
>  	Stream *stream = *camera_->streams().begin();
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index 515e906..be6c4e1 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -47,6 +47,9 @@ public:
>  	int configure(StreamConfiguration *streamConfigOut,
>  		      const Size &size, const PixelFormat &pixelformat,
>  		      unsigned int bufferCount);
> +	int validateConfiguration(const PixelFormat &pixelformat,
> +				  const Size &size,
> +				  StreamConfiguration *streamConfigOut);
>  
>  	int allocBuffers(unsigned int count);
>  	void freeBuffers();
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 305ede0..5c92ff7 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -300,24 +300,19 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)
>  {
>  	V4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat);
>  	PixelFormat format = PixelFormatInfo::info(v4l2Format).format;
> -	const std::vector<PixelFormat> &formats =
> -		streamConfig_.formats().pixelformats();
> -	if (std::find(formats.begin(), formats.end(), format) == formats.end())
> -		format = streamConfig_.formats().pixelformats()[0];
> -
>  	Size size(arg->fmt.pix.width, arg->fmt.pix.height);
> -	const std::vector<Size> &sizes = streamConfig_.formats().sizes(format);
> -	if (std::find(sizes.begin(), sizes.end(), size) == sizes.end())
> -		size = streamConfig_.formats().sizes(format)[0];
>  
> -	const PixelFormatInfo &formatInfo = PixelFormatInfo::info(format);
> +	StreamConfiguration config;
> +	vcam_->validateConfiguration(format, size, &config);
> +
> +	const PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat);
>  
> -	arg->fmt.pix.width        = size.width;
> -	arg->fmt.pix.height       = size.height;
> -	arg->fmt.pix.pixelformat  = formatInfo.v4l2Format;
> +	arg->fmt.pix.width        = config.size.width;
> +	arg->fmt.pix.height       = config.size.height;
> +	arg->fmt.pix.pixelformat  = info.v4l2Format;
>  	arg->fmt.pix.field        = V4L2_FIELD_NONE;
> -	arg->fmt.pix.bytesperline = formatInfo.stride(size.width, 0);
> -	arg->fmt.pix.sizeimage    = formatInfo.frameSize(size);
> +	arg->fmt.pix.bytesperline = config.stride;
> +	arg->fmt.pix.sizeimage    = config.frameSize;
>  	arg->fmt.pix.colorspace   = V4L2_COLORSPACE_SRGB;
>  	arg->fmt.pix.priv         = V4L2_PIX_FMT_PRIV_MAGIC;
>  	arg->fmt.pix.ycbcr_enc    = V4L2_YCBCR_ENC_DEFAULT;
Jacopo Mondi July 8, 2020, 9:20 p.m. UTC | #2 
Hi Paul,

On Wed, Jul 08, 2020 at 10:44:04PM +0900, Paul Elder wrote:
> For handling try_fmt, the values should be filled in by validating the
> stream configuration, and not by recalculating them or manually checking
> against the cached list of formats and sizes. Use
> V4L2Camera::validateConfiguration to obtain size, format, stride, and
> frameSize values. Also, implement this function in V4L2Camera.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
>
> ---
> Changes in v4:
> - clean up code a bit
> - squash the implementation and usage
>
> New in v3
> ---
>  src/v4l2/v4l2_camera.cpp       | 20 ++++++++++++++++++++
>  src/v4l2/v4l2_camera.h         |  3 +++
>  src/v4l2/v4l2_camera_proxy.cpp | 23 +++++++++--------------
>  3 files changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index ffc1230..9f81c97 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -138,6 +138,26 @@ int V4L2Camera::configure(StreamConfiguration *streamConfigOut,
>  	return 0;
>  }
>
> +int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat,

I'm not familiar with this part of the code, nor I have a full
understanding of the series yet, so maybe there's alreay a
generateConfiguration() or has been introduced, but this function
seems to that does not validate a configuration with filled fields,
but rather generates a valid one.

If its purpose is to validate a configuration it should then take a
const StreamConfiguration & and return its status, or an error code.

If it should instead generate a valid StreamConfiguration I would
rename that and make it return a StreamConfiguration

> +				      const Size &size,
> +				      StreamConfiguration *streamConfigOut)
> +{
> +	std::unique_ptr<CameraConfiguration> config =
> +		camera_->generateConfiguration({ StreamRole::Viewfinder });
> +	StreamConfiguration &cfg = config->at(0);
> +	cfg.size = size;
> +	cfg.pixelFormat = pixelFormat;
> +	cfg.bufferCount = 1;
> +
> +	CameraConfiguration::Status validation = config->validate();
> +	if (validation == CameraConfiguration::Invalid)
> +		return -EINVAL;
> +
> +	*streamConfigOut = cfg;
> +
> +	return 0;
> +}
> +
>  int V4L2Camera::allocBuffers(unsigned int count)
>  {
>  	Stream *stream = *camera_->streams().begin();
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index 515e906..be6c4e1 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -47,6 +47,9 @@ public:
>  	int configure(StreamConfiguration *streamConfigOut,
>  		      const Size &size, const PixelFormat &pixelformat,
>  		      unsigned int bufferCount);
> +	int validateConfiguration(const PixelFormat &pixelformat,
> +				  const Size &size,
> +				  StreamConfiguration *streamConfigOut);
>
>  	int allocBuffers(unsigned int count);
>  	void freeBuffers();
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 305ede0..5c92ff7 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -300,24 +300,19 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)
>  {
>  	V4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat);
>  	PixelFormat format = PixelFormatInfo::info(v4l2Format).format;
> -	const std::vector<PixelFormat> &formats =
> -		streamConfig_.formats().pixelformats();
> -	if (std::find(formats.begin(), formats.end(), format) == formats.end())
> -		format = streamConfig_.formats().pixelformats()[0];
> -
>  	Size size(arg->fmt.pix.width, arg->fmt.pix.height);
> -	const std::vector<Size> &sizes = streamConfig_.formats().sizes(format);
> -	if (std::find(sizes.begin(), sizes.end(), size) == sizes.end())
> -		size = streamConfig_.formats().sizes(format)[0];
>
> -	const PixelFormatInfo &formatInfo = PixelFormatInfo::info(format);
> +	StreamConfiguration config;
> +	vcam_->validateConfiguration(format, size, &config);

Shouldn't you check if it's valid ? maybe it's done later on.

> +
> +	const PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat);
>
> -	arg->fmt.pix.width        = size.width;
> -	arg->fmt.pix.height       = size.height;
> -	arg->fmt.pix.pixelformat  = formatInfo.v4l2Format;
> +	arg->fmt.pix.width        = config.size.width;
> +	arg->fmt.pix.height       = config.size.height;
> +	arg->fmt.pix.pixelformat  = info.v4l2Format;
>  	arg->fmt.pix.field        = V4L2_FIELD_NONE;
> -	arg->fmt.pix.bytesperline = formatInfo.stride(size.width, 0);
> -	arg->fmt.pix.sizeimage    = formatInfo.frameSize(size);
> +	arg->fmt.pix.bytesperline = config.stride;
> +	arg->fmt.pix.sizeimage    = config.frameSize;
>  	arg->fmt.pix.colorspace   = V4L2_COLORSPACE_SRGB;
>  	arg->fmt.pix.priv         = V4L2_PIX_FMT_PRIV_MAGIC;
>  	arg->fmt.pix.ycbcr_enc    = V4L2_YCBCR_ENC_DEFAULT;
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Paul Elder July 9, 2020, 8:11 a.m. UTC | #3 
Hi Jacopo,

Thank you for the review.

On Wed, Jul 08, 2020 at 11:20:10PM +0200, Jacopo Mondi wrote:
> Hi Paul,
> 
> On Wed, Jul 08, 2020 at 10:44:04PM +0900, Paul Elder wrote:
> > For handling try_fmt, the values should be filled in by validating the
> > stream configuration, and not by recalculating them or manually checking
> > against the cached list of formats and sizes. Use
> > V4L2Camera::validateConfiguration to obtain size, format, stride, and
> > frameSize values. Also, implement this function in V4L2Camera.
> >
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> >
> > ---
> > Changes in v4:
> > - clean up code a bit
> > - squash the implementation and usage
> >
> > New in v3
> > ---
> >  src/v4l2/v4l2_camera.cpp       | 20 ++++++++++++++++++++
> >  src/v4l2/v4l2_camera.h         |  3 +++
> >  src/v4l2/v4l2_camera_proxy.cpp | 23 +++++++++--------------
> >  3 files changed, 32 insertions(+), 14 deletions(-)
> >
> > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> > index ffc1230..9f81c97 100644
> > --- a/src/v4l2/v4l2_camera.cpp
> > +++ b/src/v4l2/v4l2_camera.cpp
> > @@ -138,6 +138,26 @@ int V4L2Camera::configure(StreamConfiguration *streamConfigOut,
> >  	return 0;
> >  }
> >
> > +int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat,
> 
> I'm not familiar with this part of the code, nor I have a full
> understanding of the series yet, so maybe there's alreay a
> generateConfiguration() or has been introduced, but this function
> seems to that does not validate a configuration with filled fields,
> but rather generates a valid one.

When the V4L2Camera is open()ed, it generates a configuration, to save
the current configuration of the V4L2Camera/Camera. After we supported
multiple open, the V4L2Camera is only open()ed once, and V4L2CameraProxy
keeps references for different file open()s, and V4L2CompatManager keeps
references for dup()s. Of course, if all files and dupped fds are
closed then the V4L2Camera is also close()d, and the next open() will
also open() the V4L2Camera.

> If its purpose is to validate a configuration it should then take a
> const StreamConfiguration & and return its status, or an error code.
> 
> If it should instead generate a valid StreamConfiguration I would
> rename that and make it return a StreamConfiguration

The purpose of this function is, basically, to implement try_fmt. That's
why it takes a size and a pixelformat as input, and uses those to create
a configuration, and simply validates it.

Perhaps the function isn't named very well? The caller still needs to
provide and output StreamConfiguration though, since it needs to extract
the validated size and pixelformat, as well as the stride and frameSize,
so the StreamConfiguration can't be completely abstracted away from the
caller. In any case, the V4L2CameraProxy deals with StreamConfiguration
elsewhere too, so there isn't much motivation to partition them.

> > +				      const Size &size,
> > +				      StreamConfiguration *streamConfigOut)
> > +{
> > +	std::unique_ptr<CameraConfiguration> config =
> > +		camera_->generateConfiguration({ StreamRole::Viewfinder });
> > +	StreamConfiguration &cfg = config->at(0);
> > +	cfg.size = size;
> > +	cfg.pixelFormat = pixelFormat;
> > +	cfg.bufferCount = 1;
> > +
> > +	CameraConfiguration::Status validation = config->validate();
> > +	if (validation == CameraConfiguration::Invalid)
> > +		return -EINVAL;
> > +
> > +	*streamConfigOut = cfg;
> > +
> > +	return 0;
> > +}
> > +
> >  int V4L2Camera::allocBuffers(unsigned int count)
> >  {
> >  	Stream *stream = *camera_->streams().begin();
> > diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> > index 515e906..be6c4e1 100644
> > --- a/src/v4l2/v4l2_camera.h
> > +++ b/src/v4l2/v4l2_camera.h
> > @@ -47,6 +47,9 @@ public:
> >  	int configure(StreamConfiguration *streamConfigOut,
> >  		      const Size &size, const PixelFormat &pixelformat,
> >  		      unsigned int bufferCount);
> > +	int validateConfiguration(const PixelFormat &pixelformat,
> > +				  const Size &size,
> > +				  StreamConfiguration *streamConfigOut);
> >
> >  	int allocBuffers(unsigned int count);
> >  	void freeBuffers();
> > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> > index 305ede0..5c92ff7 100644
> > --- a/src/v4l2/v4l2_camera_proxy.cpp
> > +++ b/src/v4l2/v4l2_camera_proxy.cpp
> > @@ -300,24 +300,19 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)
> >  {
> >  	V4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat);
> >  	PixelFormat format = PixelFormatInfo::info(v4l2Format).format;
> > -	const std::vector<PixelFormat> &formats =
> > -		streamConfig_.formats().pixelformats();
> > -	if (std::find(formats.begin(), formats.end(), format) == formats.end())
> > -		format = streamConfig_.formats().pixelformats()[0];
> > -
> >  	Size size(arg->fmt.pix.width, arg->fmt.pix.height);
> > -	const std::vector<Size> &sizes = streamConfig_.formats().sizes(format);
> > -	if (std::find(sizes.begin(), sizes.end(), size) == sizes.end())
> > -		size = streamConfig_.formats().sizes(format)[0];
> >
> > -	const PixelFormatInfo &formatInfo = PixelFormatInfo::info(format);
> > +	StreamConfiguration config;
> > +	vcam_->validateConfiguration(format, size, &config);
> 
> Shouldn't you check if it's valid ? maybe it's done later on.

Hmmmmm.... yeah I think I should. Invalid means it failed to adjust, so
in that case it's fair for try_fmt to return -EINVAL, right.

> > +
> > +	const PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat);
> >
> > -	arg->fmt.pix.width        = size.width;
> > -	arg->fmt.pix.height       = size.height;
> > -	arg->fmt.pix.pixelformat  = formatInfo.v4l2Format;
> > +	arg->fmt.pix.width        = config.size.width;
> > +	arg->fmt.pix.height       = config.size.height;
> > +	arg->fmt.pix.pixelformat  = info.v4l2Format;
> >  	arg->fmt.pix.field        = V4L2_FIELD_NONE;
> > -	arg->fmt.pix.bytesperline = formatInfo.stride(size.width, 0);
> > -	arg->fmt.pix.sizeimage    = formatInfo.frameSize(size);
> > +	arg->fmt.pix.bytesperline = config.stride;
> > +	arg->fmt.pix.sizeimage    = config.frameSize;
> >  	arg->fmt.pix.colorspace   = V4L2_COLORSPACE_SRGB;
> >  	arg->fmt.pix.priv         = V4L2_PIX_FMT_PRIV_MAGIC;
> >  	arg->fmt.pix.ycbcr_enc    = V4L2_YCBCR_ENC_DEFAULT;


Thanks,

Paul

Patch
diff mbox series 

diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
index ffc1230..9f81c97 100644
--- a/src/v4l2/v4l2_camera.cpp
+++ b/src/v4l2/v4l2_camera.cpp
@@ -138,6 +138,26 @@  int V4L2Camera::configure(StreamConfiguration *streamConfigOut,
 	return 0;
 }
 
+int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat,
+				      const Size &size,
+				      StreamConfiguration *streamConfigOut)
+{
+	std::unique_ptr<CameraConfiguration> config =
+		camera_->generateConfiguration({ StreamRole::Viewfinder });
+	StreamConfiguration &cfg = config->at(0);
+	cfg.size = size;
+	cfg.pixelFormat = pixelFormat;
+	cfg.bufferCount = 1;
+
+	CameraConfiguration::Status validation = config->validate();
+	if (validation == CameraConfiguration::Invalid)
+		return -EINVAL;
+
+	*streamConfigOut = cfg;
+
+	return 0;
+}
+
 int V4L2Camera::allocBuffers(unsigned int count)
 {
 	Stream *stream = *camera_->streams().begin();
diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
index 515e906..be6c4e1 100644
--- a/src/v4l2/v4l2_camera.h
+++ b/src/v4l2/v4l2_camera.h
@@ -47,6 +47,9 @@  public:
 	int configure(StreamConfiguration *streamConfigOut,
 		      const Size &size, const PixelFormat &pixelformat,
 		      unsigned int bufferCount);
+	int validateConfiguration(const PixelFormat &pixelformat,
+				  const Size &size,
+				  StreamConfiguration *streamConfigOut);
 
 	int allocBuffers(unsigned int count);
 	void freeBuffers();
diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
index 305ede0..5c92ff7 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -300,24 +300,19 @@  void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)
 {
 	V4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat);
 	PixelFormat format = PixelFormatInfo::info(v4l2Format).format;
-	const std::vector<PixelFormat> &formats =
-		streamConfig_.formats().pixelformats();
-	if (std::find(formats.begin(), formats.end(), format) == formats.end())
-		format = streamConfig_.formats().pixelformats()[0];
-
 	Size size(arg->fmt.pix.width, arg->fmt.pix.height);
-	const std::vector<Size> &sizes = streamConfig_.formats().sizes(format);
-	if (std::find(sizes.begin(), sizes.end(), size) == sizes.end())
-		size = streamConfig_.formats().sizes(format)[0];
 
-	const PixelFormatInfo &formatInfo = PixelFormatInfo::info(format);
+	StreamConfiguration config;
+	vcam_->validateConfiguration(format, size, &config);
+
+	const PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat);
 
-	arg->fmt.pix.width        = size.width;
-	arg->fmt.pix.height       = size.height;
-	arg->fmt.pix.pixelformat  = formatInfo.v4l2Format;
+	arg->fmt.pix.width        = config.size.width;
+	arg->fmt.pix.height       = config.size.height;
+	arg->fmt.pix.pixelformat  = info.v4l2Format;
 	arg->fmt.pix.field        = V4L2_FIELD_NONE;
-	arg->fmt.pix.bytesperline = formatInfo.stride(size.width, 0);
-	arg->fmt.pix.sizeimage    = formatInfo.frameSize(size);
+	arg->fmt.pix.bytesperline = config.stride;
+	arg->fmt.pix.sizeimage    = config.frameSize;
 	arg->fmt.pix.colorspace   = V4L2_COLORSPACE_SRGB;
 	arg->fmt.pix.priv         = V4L2_PIX_FMT_PRIV_MAGIC;
 	arg->fmt.pix.ycbcr_enc    = V4L2_YCBCR_ENC_DEFAULT;