Message ID | 20200704133140.1738660-8-paul.elder@ideasonboard.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Paul, Thank you for the patch. On Sat, Jul 04, 2020 at 10:31:25PM +0900, Paul Elder wrote: > In V4L2CameraProxy, we need a way to validate formats and sizes, to > implement try_fmt. Instead of manually checking it against the cached > list of formats and sizes, add V4L2Camera::validateConfiguration that we > can use to implement try_fmt. I'd squash this with patch 10/22, as it's more difficult to review the function without seeing how it's used. If the function and/or the user where large it may make sense to split them in separate patches, but here a single patch is manageable. > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > New in v3 > --- > src/v4l2/v4l2_camera.cpp | 29 +++++++++++++++++++++++++++++ > src/v4l2/v4l2_camera.h | 3 +++ > 2 files changed, 32 insertions(+) > > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp > index ffc1230..326e1c2 100644 > --- a/src/v4l2/v4l2_camera.cpp > +++ b/src/v4l2/v4l2_camera.cpp > @@ -138,6 +138,35 @@ int V4L2Camera::configure(StreamConfiguration *streamConfigOut, > return 0; > } > > +int V4L2Camera::validateConfiguration(StreamConfiguration *streamConfigOut, > + const Size &size, > + const PixelFormat &pixelFormat) We usually put output parameters last, and pixel format before size (probably because we consider that the size qualifies the pixel format). > +{ > + if (!streamConfigOut) > + return -EINVAL; That can't happen, it's a private function, and the caller always passed a non-null pointer. > + > + std::unique_ptr<CameraConfiguration> config = > + camera_->generateConfiguration({ StreamRole::Viewfinder }); > + StreamConfiguration &cfg = config->at(0); > + cfg.size.width = size.width; > + cfg.size.height = size.height; cfg.size = size; > + cfg.pixelFormat = pixelFormat; > + cfg.bufferCount = 1; > + > + CameraConfiguration::Status validation = config->validate(); > + if (validation == CameraConfiguration::Invalid) > + return -EINVAL; > + > + streamConfigOut->pixelFormat = cfg.pixelFormat; > + streamConfigOut->size.width = cfg.size.width; > + streamConfigOut->size.height = cfg.size.height; streamConfigOut->size = cfg.size; > + streamConfigOut->bufferCount = cfg.bufferCount; > + streamConfigOut->stride = cfg.stride; > + streamConfigOut->frameSize = cfg.frameSize; Or just *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..15a9363 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(StreamConfiguration *streamConfigOut, > + const Size &size, > + const PixelFormat &pixelformat); > > int allocBuffers(unsigned int count); > void freeBuffers();
diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index ffc1230..326e1c2 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -138,6 +138,35 @@ int V4L2Camera::configure(StreamConfiguration *streamConfigOut, return 0; } +int V4L2Camera::validateConfiguration(StreamConfiguration *streamConfigOut, + const Size &size, + const PixelFormat &pixelFormat) +{ + if (!streamConfigOut) + return -EINVAL; + + std::unique_ptr<CameraConfiguration> config = + camera_->generateConfiguration({ StreamRole::Viewfinder }); + StreamConfiguration &cfg = config->at(0); + cfg.size.width = size.width; + cfg.size.height = size.height; + cfg.pixelFormat = pixelFormat; + cfg.bufferCount = 1; + + CameraConfiguration::Status validation = config->validate(); + if (validation == CameraConfiguration::Invalid) + return -EINVAL; + + streamConfigOut->pixelFormat = cfg.pixelFormat; + streamConfigOut->size.width = cfg.size.width; + streamConfigOut->size.height = cfg.size.height; + streamConfigOut->bufferCount = cfg.bufferCount; + streamConfigOut->stride = cfg.stride; + streamConfigOut->frameSize = cfg.frameSize; + + 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..15a9363 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(StreamConfiguration *streamConfigOut, + const Size &size, + const PixelFormat &pixelformat); int allocBuffers(unsigned int count); void freeBuffers();
In V4L2CameraProxy, we need a way to validate formats and sizes, to implement try_fmt. Instead of manually checking it against the cached list of formats and sizes, add V4L2Camera::validateConfiguration that we can use to implement try_fmt. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- New in v3 --- src/v4l2/v4l2_camera.cpp | 29 +++++++++++++++++++++++++++++ src/v4l2/v4l2_camera.h | 3 +++ 2 files changed, 32 insertions(+)