| Message ID | 20220707150310.3645858-3-paul.elder@ideasonboard.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Fri, Jul 08, 2022 at 12:03:08AM +0900, Paul Elder via libcamera-devel wrote: > Some formats, such as YVU422 (but *not* YUV422), YUV444, and YVU444, > have only a multiplanar v4l2 format and no singleplanar format. When > using V4L2PixelFormat::fromPixelFormat() to convert a libcamera format > to V4L2PixelFormat, the default is to fetch the singleplanar format, and > for those three formats an invalid format will be returned. The caller > shouldn't be expected to check first if it should request the > singleplanar or multiplanar version (that's the whole point of > V4L2PixelFormat::fromPixelFormat()), so the solution that this patch > implements is if fetching singleplanar/multiplanar fails, try fetching > the other one. > > The side effect is that as most formats have only singleplanar v4l2 > format and no multiplanar format, if a multiplanar format is request for > these formats then the singleplanar one will be automatically returned. > Since currently all callers of V4L2PixelFormat::fromPixelFormat() use > the default singleplanar call, it is reasoned that this is not an issue. Jacopo is planning to rework this to add JPEG support in addition to MJPEG, with a mechanism that should address your problem in a better way (taking into account the formats actually supported by the device). We could merge this as a quick workaround, but if there's no urgency, I'd wait for Jacopo's patch. > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/libcamera/v4l2_pixelformat.cpp | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp > index 58fc4e9d..0d2bc350 100644 > --- a/src/libcamera/v4l2_pixelformat.cpp > +++ b/src/libcamera/v4l2_pixelformat.cpp > @@ -321,7 +321,14 @@ V4L2PixelFormat V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat, > if (!info.isValid()) > return V4L2PixelFormat(); > > - return multiplanar ? info.v4l2Formats.multi : info.v4l2Formats.single; > + V4L2PixelFormat ret = multiplanar ? info.v4l2Formats.multi > + : info.v4l2Formats.single; > + /* Try the other of multi/single if the proper one doesn't exist */ > + if (!ret.isValid()) > + ret = multiplanar ? info.v4l2Formats.single > + : info.v4l2Formats.multi; > + > + return ret; > } > > /**
On Fri, Jul 08, 2022 at 12:20:14AM +0300, Laurent Pinchart wrote: > Hi Paul, > > Thank you for the patch. > > On Fri, Jul 08, 2022 at 12:03:08AM +0900, Paul Elder via libcamera-devel wrote: > > Some formats, such as YVU422 (but *not* YUV422), YUV444, and YVU444, > > have only a multiplanar v4l2 format and no singleplanar format. When > > using V4L2PixelFormat::fromPixelFormat() to convert a libcamera format > > to V4L2PixelFormat, the default is to fetch the singleplanar format, and > > for those three formats an invalid format will be returned. The caller > > shouldn't be expected to check first if it should request the > > singleplanar or multiplanar version (that's the whole point of > > V4L2PixelFormat::fromPixelFormat()), so the solution that this patch > > implements is if fetching singleplanar/multiplanar fails, try fetching > > the other one. > > > > The side effect is that as most formats have only singleplanar v4l2 > > format and no multiplanar format, if a multiplanar format is request for > > these formats then the singleplanar one will be automatically returned. > > Since currently all callers of V4L2PixelFormat::fromPixelFormat() use > > the default singleplanar call, it is reasoned that this is not an issue. > > Jacopo is planning to rework this to add JPEG support in addition to > MJPEG, with a mechanism that should address your problem in a better way > (taking into account the formats actually supported by the device). We > could merge this as a quick workaround, but if there's no urgency, I'd > wait for Jacopo's patch. Yeah I guess we could wait then. I'll change the next patch to just YUV422 then. Paul > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > src/libcamera/v4l2_pixelformat.cpp | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp > > index 58fc4e9d..0d2bc350 100644 > > --- a/src/libcamera/v4l2_pixelformat.cpp > > +++ b/src/libcamera/v4l2_pixelformat.cpp > > @@ -321,7 +321,14 @@ V4L2PixelFormat V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat, > > if (!info.isValid()) > > return V4L2PixelFormat(); > > > > - return multiplanar ? info.v4l2Formats.multi : info.v4l2Formats.single; > > + V4L2PixelFormat ret = multiplanar ? info.v4l2Formats.multi > > + : info.v4l2Formats.single; > > + /* Try the other of multi/single if the proper one doesn't exist */ > > + if (!ret.isValid()) > > + ret = multiplanar ? info.v4l2Formats.single > > + : info.v4l2Formats.multi; > > + > > + return ret; > > } > > > > /**
diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp index 58fc4e9d..0d2bc350 100644 --- a/src/libcamera/v4l2_pixelformat.cpp +++ b/src/libcamera/v4l2_pixelformat.cpp @@ -321,7 +321,14 @@ V4L2PixelFormat V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat, if (!info.isValid()) return V4L2PixelFormat(); - return multiplanar ? info.v4l2Formats.multi : info.v4l2Formats.single; + V4L2PixelFormat ret = multiplanar ? info.v4l2Formats.multi + : info.v4l2Formats.single; + /* Try the other of multi/single if the proper one doesn't exist */ + if (!ret.isValid()) + ret = multiplanar ? info.v4l2Formats.single + : info.v4l2Formats.multi; + + return ret; } /**
Some formats, such as YVU422 (but *not* YUV422), YUV444, and YVU444, have only a multiplanar v4l2 format and no singleplanar format. When using V4L2PixelFormat::fromPixelFormat() to convert a libcamera format to V4L2PixelFormat, the default is to fetch the singleplanar format, and for those three formats an invalid format will be returned. The caller shouldn't be expected to check first if it should request the singleplanar or multiplanar version (that's the whole point of V4L2PixelFormat::fromPixelFormat()), so the solution that this patch implements is if fetching singleplanar/multiplanar fails, try fetching the other one. The side effect is that as most formats have only singleplanar v4l2 format and no multiplanar format, if a multiplanar format is request for these formats then the singleplanar one will be automatically returned. Since currently all callers of V4L2PixelFormat::fromPixelFormat() use the default singleplanar call, it is reasoned that this is not an issue. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- src/libcamera/v4l2_pixelformat.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)