[libcamera-devel] [PATCH v2] libcamera: ipu3: Use std::max() instead of expandTo() to get the max resolution
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu Aug 25 11:55:53 CEST 2022
Quoting Jacopo Mondi via libcamera-devel (2022-08-17 09:19:29)
> Hi Han-Lin
>
> On Fri, Aug 12, 2022 at 05:01:03PM +0800, Han-Lin Chen via libcamera-devel wrote:
> > Using Size::expandTo() to find the max resolution might generate a non-existent
> > resolution. For example, when application request streams for 1920x1080 and
> > 1600x1200, the max resolution will be wrongly 1920x1200 and fails the
> > configuration.
> >
> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=139
> > Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> > ---
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 22 ++++++++++++----------
> > 1 file changed, 12 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index f65db3c8..47311953 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -247,6 +247,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
> > */
> > unsigned int rawCount = 0;
> > unsigned int yuvCount = 0;
> > + Size rawRequirement;
> > Size maxYuvSize;
> > Size rawSize;
> >
> > @@ -255,10 +256,11 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
> >
> > if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
> > rawCount++;
> > - rawSize.expandTo(cfg.size);
> > + rawSize = std::max(rawSize, cfg.size);
> > } else {
> > yuvCount++;
> > - maxYuvSize.expandTo(cfg.size);
> > + maxYuvSize = std::max(maxYuvSize, cfg.size);
> > + rawRequirement.expandTo(cfg.size);
> > }
> > }
> >
> > @@ -287,17 +289,17 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
> > * The output YUV streams will be limited in size to the maximum frame
> > * size requested for the RAW stream, if present.
> > *
> > - * If no raw stream is requested generate a size as large as the maximum
> > - * requested YUV size aligned to the ImgU constraints and bound by the
> > - * sensor's maximum resolution. See
> > + * If no raw stream is requested generate a size as large as the span
I know it's pre-existing, but could you add a comma here? Without it the
sentence flows too fast in my head.
"If no raw stream is requested, generate a size as ...
I think this is called a "First conditional sentence"
https://www.grammarly.com/blog/conditional-sentences/
> > + * of all requested YUV size aligned to the ImgU constraints and bound
> > + * by the sensor's maximum resolution. See
I'm confused here about the use of 'a span of all requested YUV sizes'
as that feels like it gets confused with a libcamera::Span concept which
I don't think is what it means here.
I would say
"If no raw stream is requested, generate a size from the largest given
YUV stream, aligned to the ImgU constraints and bound by the sensor's
maximum resolution. See:"
> > * https://bugs.libcamera.org/show_bug.cgi?id=32
> > */
> > if (rawSize.isNull())
> > - rawSize = maxYuvSize.expandedTo({ ImgUDevice::kIFMaxCropWidth,
> > - ImgUDevice::kIFMaxCropHeight })
> > - .grownBy({ ImgUDevice::kOutputMarginWidth,
> > - ImgUDevice::kOutputMarginHeight })
> > - .boundedTo(data_->cio2_.sensor()->resolution());
> > + rawSize = rawRequirement.expandedTo({ ImgUDevice::kIFMaxCropWidth,
> > + ImgUDevice::kIFMaxCropHeight })
> > + .grownBy({ ImgUDevice::kOutputMarginWidth,
> > + ImgUDevice::kOutputMarginHeight })
> > + .boundedTo(data_->cio2_.sensor()->resolution());
>
> This seems to fix the issue indeed, and maintain the current behaviour
> where the RAW stream size limits the YUV output size.
>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
With the comment updated in any appropriate way,
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> Thanks
> j
>
> >
> > cio2Configuration_ = data_->cio2_.generateConfiguration(rawSize);
> > if (!cio2Configuration_.pixelFormat.isValid())
> > --
> > 2.37.1.595.g718a3a8f04-goog
> >
More information about the libcamera-devel
mailing list