| Message ID | 20241011184600.17118-1-robert.mader@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Robert, thank you for the patch. Robert Mader <robert.mader@collabora.com> writes: > In order to avoid having to adjust the size further down below which > again can break user assumptions. Notably, without this the capture size > of 1920x1080 gets adjusted to 1912x1080 when used with the swISP using a > bayer pattern width of 4, breaking users like Gstreamer down the line. > > Closes https://bugs.libcamera.org/show_bug.cgi?id=236 > > Signed-off-by: Robert Mader <robert.mader@collabora.com> > > --- > > I'm not really sure if this is the correct approach, but sending it out > already for feedback. So far this gives me promissing results on tested > devices. > --- > src/libcamera/pipeline/simple/simple.cpp | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 3ddce71d..2d185b90 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1048,7 +1048,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > const Size &size = pipeConfig->captureSize; > > if (size.width >= maxStreamSize.width && > - size.height >= maxStreamSize.height) { > + size.height >= maxStreamSize.height && > + pipeConfig->outputSizes.contains(maxStreamSize)) { Some more explanation is needed. The original code simply selects the smallest size that can cover the output. Now with this change, what if `size' is sufficiently high but outputSizes doesn't contain exactly maxStreamSize but it contains some larger value? And what happens if software ISP is not enabled for the pipeline? > if (!pipeConfig_ || size < pipeConfig_->captureSize) > pipeConfig_ = pipeConfig; > }
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 3ddce71d..2d185b90 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -1048,7 +1048,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() const Size &size = pipeConfig->captureSize; if (size.width >= maxStreamSize.width && - size.height >= maxStreamSize.height) { + size.height >= maxStreamSize.height && + pipeConfig->outputSizes.contains(maxStreamSize)) { if (!pipeConfig_ || size < pipeConfig_->captureSize) pipeConfig_ = pipeConfig; }
In order to avoid having to adjust the size further down below which again can break user assumptions. Notably, without this the capture size of 1920x1080 gets adjusted to 1912x1080 when used with the swISP using a bayer pattern width of 4, breaking users like Gstreamer down the line. Closes https://bugs.libcamera.org/show_bug.cgi?id=236 Signed-off-by: Robert Mader <robert.mader@collabora.com> --- I'm not really sure if this is the correct approach, but sending it out already for feedback. So far this gives me promissing results on tested devices. --- src/libcamera/pipeline/simple/simple.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)