| Message ID | 20210131224702.8838-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Delegated to: | Laurent Pinchart |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Mon, Feb 01, 2021 at 12:46:44AM +0200, Laurent Pinchart wrote: > The stride (and frame size) calculation has been moved from configure > time to configuration validate time by commit 89fb1efac240 ("libcamera: simple: > Fill stride and frameSize at config validation"). This change has > however left one stray setting of the stride when configuring the > converter. Fix it. > > While at it, turn the SimpleConverter::configure() output configuration > argument to a const reference to emphasize it can't be null and isn't > modified by the function, and rename it from cfg to outputCfg to make > its purpose clearer. > > Fixes: 89fb1efac240 ("libcamera: simple: Fill stride and frameSize at config validation") > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/libcamera/pipeline/simple/converter.cpp | 10 ++++------ > src/libcamera/pipeline/simple/converter.h | 2 +- > src/libcamera/pipeline/simple/simple.cpp | 2 +- > 3 files changed, 6 insertions(+), 8 deletions(-) > > diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp > index a6a8e139cb3e..87d15c781ed8 100644 > --- a/src/libcamera/pipeline/simple/converter.cpp > +++ b/src/libcamera/pipeline/simple/converter.cpp > @@ -134,7 +134,7 @@ SizeRange SimpleConverter::sizes(const Size &input) > } > > int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize, > - StreamConfiguration *cfg) > + const StreamConfiguration &outputCfg) > { > V4L2DeviceFormat format; > int ret; > @@ -157,10 +157,10 @@ int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize, > } > > /* Set the pixel format and size on the output. */ > - videoFormat = m2m_->capture()->toV4L2PixelFormat(cfg->pixelFormat); > + videoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat); > format = {}; > format.fourcc = videoFormat; > - format.size = cfg->size; > + format.size = outputCfg.size; > > ret = m2m_->capture()->setFormat(&format); > if (ret < 0) { > @@ -169,14 +169,12 @@ int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize, > return ret; > } > > - if (format.fourcc != videoFormat || format.size != cfg->size) { > + if (format.fourcc != videoFormat || format.size != outputCfg.size) { > LOG(SimplePipeline, Error) > << "Output format not supported"; > return -EINVAL; > } > > - cfg->stride = format.planes[0].bpl; > - > return 0; > } > > diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h > index a3c4d899cfc8..06d66f8caba7 100644 > --- a/src/libcamera/pipeline/simple/converter.h > +++ b/src/libcamera/pipeline/simple/converter.h > @@ -37,7 +37,7 @@ public: > SizeRange sizes(const Size &input); > > int configure(PixelFormat inputFormat, const Size &inputSize, > - StreamConfiguration *cfg); > + const StreamConfiguration &outputCfg); > int exportBuffers(unsigned int count, > std::vector<std::unique_ptr<FrameBuffer>> *buffers); > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index b7aa3d034568..a97b8442d9a4 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -604,7 +604,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) > > if (useConverter_) { > ret = converter_->configure(pipeConfig.pixelFormat, > - pipeConfig.captureSize, &cfg); > + pipeConfig.captureSize, cfg); > if (ret < 0) { > LOG(SimplePipeline, Error) > << "Unable to configure converter"; > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
On 31/01/2021 22:46, Laurent Pinchart wrote: > The stride (and frame size) calculation has been moved from configure > time to configuration validate time by commit 89fb1efac240 ("libcamera: simple: > Fill stride and frameSize at config validation"). This change has > however left one stray setting of the stride when configuring the > converter. Fix it. > > While at it, turn the SimpleConverter::configure() output configuration > argument to a const reference to emphasize it can't be null and isn't > modified by the function, and rename it from cfg to outputCfg to make > its purpose clearer. > > Fixes: 89fb1efac240 ("libcamera: simple: Fill stride and frameSize at config validation") > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/pipeline/simple/converter.cpp | 10 ++++------ > src/libcamera/pipeline/simple/converter.h | 2 +- > src/libcamera/pipeline/simple/simple.cpp | 2 +- > 3 files changed, 6 insertions(+), 8 deletions(-) > > diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp > index a6a8e139cb3e..87d15c781ed8 100644 > --- a/src/libcamera/pipeline/simple/converter.cpp > +++ b/src/libcamera/pipeline/simple/converter.cpp > @@ -134,7 +134,7 @@ SizeRange SimpleConverter::sizes(const Size &input) > } > > int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize, > - StreamConfiguration *cfg) > + const StreamConfiguration &outputCfg) > { > V4L2DeviceFormat format; > int ret; > @@ -157,10 +157,10 @@ int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize, > } > > /* Set the pixel format and size on the output. */ > - videoFormat = m2m_->capture()->toV4L2PixelFormat(cfg->pixelFormat); > + videoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat); > format = {}; > format.fourcc = videoFormat; > - format.size = cfg->size; > + format.size = outputCfg.size; > > ret = m2m_->capture()->setFormat(&format); > if (ret < 0) { > @@ -169,14 +169,12 @@ int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize, > return ret; > } > > - if (format.fourcc != videoFormat || format.size != cfg->size) { > + if (format.fourcc != videoFormat || format.size != outputCfg.size) { > LOG(SimplePipeline, Error) > << "Output format not supported"; > return -EINVAL; > } > > - cfg->stride = format.planes[0].bpl; > - > return 0; > } > > diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h > index a3c4d899cfc8..06d66f8caba7 100644 > --- a/src/libcamera/pipeline/simple/converter.h > +++ b/src/libcamera/pipeline/simple/converter.h > @@ -37,7 +37,7 @@ public: > SizeRange sizes(const Size &input); > > int configure(PixelFormat inputFormat, const Size &inputSize, > - StreamConfiguration *cfg); > + const StreamConfiguration &outputCfg); > int exportBuffers(unsigned int count, > std::vector<std::unique_ptr<FrameBuffer>> *buffers); > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index b7aa3d034568..a97b8442d9a4 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -604,7 +604,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) > > if (useConverter_) { > ret = converter_->configure(pipeConfig.pixelFormat, > - pipeConfig.captureSize, &cfg); > + pipeConfig.captureSize, cfg); > if (ret < 0) { > LOG(SimplePipeline, Error) > << "Unable to configure converter"; >
diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp index a6a8e139cb3e..87d15c781ed8 100644 --- a/src/libcamera/pipeline/simple/converter.cpp +++ b/src/libcamera/pipeline/simple/converter.cpp @@ -134,7 +134,7 @@ SizeRange SimpleConverter::sizes(const Size &input) } int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize, - StreamConfiguration *cfg) + const StreamConfiguration &outputCfg) { V4L2DeviceFormat format; int ret; @@ -157,10 +157,10 @@ int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize, } /* Set the pixel format and size on the output. */ - videoFormat = m2m_->capture()->toV4L2PixelFormat(cfg->pixelFormat); + videoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat); format = {}; format.fourcc = videoFormat; - format.size = cfg->size; + format.size = outputCfg.size; ret = m2m_->capture()->setFormat(&format); if (ret < 0) { @@ -169,14 +169,12 @@ int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize, return ret; } - if (format.fourcc != videoFormat || format.size != cfg->size) { + if (format.fourcc != videoFormat || format.size != outputCfg.size) { LOG(SimplePipeline, Error) << "Output format not supported"; return -EINVAL; } - cfg->stride = format.planes[0].bpl; - return 0; } diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h index a3c4d899cfc8..06d66f8caba7 100644 --- a/src/libcamera/pipeline/simple/converter.h +++ b/src/libcamera/pipeline/simple/converter.h @@ -37,7 +37,7 @@ public: SizeRange sizes(const Size &input); int configure(PixelFormat inputFormat, const Size &inputSize, - StreamConfiguration *cfg); + const StreamConfiguration &outputCfg); int exportBuffers(unsigned int count, std::vector<std::unique_ptr<FrameBuffer>> *buffers); diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index b7aa3d034568..a97b8442d9a4 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -604,7 +604,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) if (useConverter_) { ret = converter_->configure(pipeConfig.pixelFormat, - pipeConfig.captureSize, &cfg); + pipeConfig.captureSize, cfg); if (ret < 0) { LOG(SimplePipeline, Error) << "Unable to configure converter";
The stride (and frame size) calculation has been moved from configure time to configuration validate time by commit 89fb1efac240 ("libcamera: simple: Fill stride and frameSize at config validation"). This change has however left one stray setting of the stride when configuring the converter. Fix it. While at it, turn the SimpleConverter::configure() output configuration argument to a const reference to emphasize it can't be null and isn't modified by the function, and rename it from cfg to outputCfg to make its purpose clearer. Fixes: 89fb1efac240 ("libcamera: simple: Fill stride and frameSize at config validation") Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/pipeline/simple/converter.cpp | 10 ++++------ src/libcamera/pipeline/simple/converter.h | 2 +- src/libcamera/pipeline/simple/simple.cpp | 2 +- 3 files changed, 6 insertions(+), 8 deletions(-)