[libcamera-devel] [PATCH v9 6/8] libcamera: Support passing ColorSpaces to V4L2 subdevices

[Laurent Pinchart]

On Tue, Dec 07, 2021 at 03:29:44PM +0200, Laurent Pinchart wrote:
> Hi David,
> 
> Thank you for the patch.

Also, the subject line should read

libcamera: v4l2_subdevice: Support passing ColorSpaces to V4L2 subdevices

or just

libcamera: v4l2_subdevice: Support passing ColorSpaces

(please update other patches in the series accordingly)

> On Mon, Dec 06, 2021 at 10:50:29AM +0000, David Plowman wrote:
> > The ColorSpace from the StreamConfiguration is now handled
> > appropriately in the V4L2Subdevice.
> > 
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> >  src/libcamera/v4l2_subdevice.cpp | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index 981645e0..f5ec6901 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -411,6 +411,7 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> >  	format->size.width = subdevFmt.format.width;
> >  	format->size.height = subdevFmt.format.height;
> >  	format->mbus_code = subdevFmt.format.code;
> > +	format->colorSpace = toColorSpace(subdevFmt.format);
> >  
> >  	return 0;
> >  }
> > @@ -439,7 +440,13 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> >  	subdevFmt.format.code = format->mbus_code;
> >  	subdevFmt.format.field = V4L2_FIELD_NONE;
> >  
> > -	int ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);
> > +	int ret = fromColorSpace(format->colorSpace, subdevFmt.format);
> > +	if (ret < 0)
> > +		LOG(V4L2, Warning)
> > +			<< "Setting color space unrecognised by V4L2: "
> > +			<< ColorSpace::toString(format->colorSpace);
> 
> Same comment as in 5/8 regarding the message.

I meant 4/8.

> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> > +
> > +	ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);
> >  	if (ret) {
> >  		LOG(V4L2, Error)
> >  			<< "Unable to set format on pad " << pad
> > @@ -450,6 +457,7 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> >  	format->size.width = subdevFmt.format.width;
> >  	format->size.height = subdevFmt.format.height;
> >  	format->mbus_code = subdevFmt.format.code;
> > +	format->colorSpace = toColorSpace(subdevFmt.format);
> >  
> >  	return 0;
> >  }

-- 
Regards,

Laurent Pinchart