[libcamera-devel] [PATCH v3 7/9] ipu3: Assign |outCaptureStream| to StillCapture configuration

[Cheng-Hao Yang]

Hi Umang,

On Tue, Aug 2, 2022 at 3:53 PM Umang Jain <umang.jain at ideasonboard.com>
wrote:

> Hi Harvey,
>
> On 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:
> > When StillCapture and other non-raw configurations are requested,
> > assigns |outCaptureStream| instead of |outStream| to the StillCapture
> > configuration.
> >
> > Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> > ---
> >   src/libcamera/pipeline/ipu3/ipu3.cpp | 50 +++++++++++++++++++++++-----
> >   1 file changed, 42 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp
> b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index ec9d14d1..e26c2736 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -254,8 +254,10 @@ CameraConfiguration::Status
> IPU3CameraConfiguration::validate()
> >        * \todo Clarify the IF and BDS margins requirements.
> >        */
> >       unsigned int rawCount = 0;
> > -     unsigned int yuvCount = 0;
> > +     unsigned int videoCount = 0;
> > +     unsigned int stillCount = 0;
> >       Size maxYuvSize;
> > +     Size maxVideoSize;
> >       Size rawSize;
> >
> >       for (const StreamConfiguration &cfg : config_) {
> > @@ -264,16 +266,29 @@ CameraConfiguration::Status
> IPU3CameraConfiguration::validate()
> >               if (info.colourEncoding ==
> PixelFormatInfo::ColourEncodingRAW) {
> >                       rawCount++;
> >                       rawSize.expandTo(cfg.size);
> > +             } else if (cfg.streamRole == StreamRole::StillCapture) {
> > +                     if (stillCount != 0)
> > +                             return Invalid;
> > +
> > +                     stillCount++;
> > +                     maxYuvSize.expandTo(cfg.size);
> >               } else {
> > -                     yuvCount++;
> > +                     videoCount++;
> >                       maxYuvSize.expandTo(cfg.size);
> > +                     maxVideoSize.expandTo(cfg.size);
> >               }
> >       }
> >
> > -     if (rawCount > 1 || yuvCount > 2) {
> > +     if (videoCount == 0 && stillCount == 1) {
> > +             stillCount = 0;
> > +             videoCount = 1;
> > +             maxVideoSize.expandTo(maxYuvSize);
> > +     }
>
>
> Maybe you can explain this if block with a brief comment on the case why
> we invert stillCount and videoCount.
>
>
Added a simple one. Please check.


> > +
> > +     if (rawCount > 1 || videoCount > 2) {
> >               LOG(IPU3, Debug) << "Camera configuration not supported";
> >               return Invalid;
> > -     } else if (rawCount && !yuvCount) {
> > +     } else if (rawCount && !stillCount && !videoCount) {
> >               /*
> >                * Disallow raw-only camera configuration. Currently, ImgU
> does
> >                * not get configured for raw-only streams and has early
> return
> > @@ -316,6 +331,9 @@ CameraConfiguration::Status
> IPU3CameraConfiguration::validate()
> >       ImgUDevice::Pipe pipe{};
> >       pipe.input = cio2Configuration_.size;
> >
> > +     ImgUDevice::Pipe pipe1{};
> > +     pipe1.input = cio2Configuration_.size;
> > +
> >       /*
> >        * Adjust the configurations if needed and assign streams while
> >        * iterating them.
> > @@ -380,18 +398,34 @@ CameraConfiguration::Status
> IPU3CameraConfiguration::validate()
> >                       cfg->stride = info.stride(cfg->size.width, 0, 1);
> >                       cfg->frameSize = info.frameSize(cfg->size, 1);
> >
> > +                     if (stillCount == 1 && cfg->streamRole ==
> StreamRole::StillCapture) {
>
>
> Maybe check both is redundant? Can stillCount be converted to
> ASSERT(stillCount != 0) inside the block?
>
>
Sure. Use |ASSERT(stillCount == 1)| instead, as it only supports one
StillCapture stream.


> > +                             LOG(IPU3, Debug) << "Assigned "
> > +                                              << cfg->toString()
> > +                                              << " to the imgu1 main
> output";
> > +                             cfg->setStream(const_cast<Stream
> *>(&data_->outCaptureStream_));
> > +
> > +                             pipe1.main = cfg->size;
> > +                             pipe1.viewfinder = pipe1.main;
> > +
> > +                             pipeConfig1_ =
> data_->imgu1_->calculatePipeConfig(&pipe1);
> > +                             if (pipeConfig1_.isNull()) {
> > +                                     LOG(IPU3, Error) << "Failed to
> calculate pipe configuration: "
> > +                                                      << "unsupported
> resolutions.";
> > +                                     return Invalid;
> > +                             }
> >                       /*
> >                        * Use the main output stream in case only one
> stream is
> >                        * requested or if the current configuration is
> the one
> >                        * with the maximum YUV output size.
> >                        */
> > -                     if (mainOutputAvailable &&
> > -                         (originalCfg.size == maxYuvSize || yuvCount ==
> 1)) {
> > +                     } else if (mainOutputAvailable &&
> > +                                (originalCfg.size == maxVideoSize ||
> > +                                 videoCount == 1)) {
> >                               cfg->setStream(const_cast<Stream
> *>(&data_->outStream_));
> >                               mainOutputAvailable = false;
> >
> >                               pipe.main = cfg->size;
> > -                             if (yuvCount == 1)
> > +                             if (videoCount == 1)
> >                                       pipe.viewfinder = pipe.main;
> >
> >                               LOG(IPU3, Debug) << "Assigned " <<
> cfg->toString()
> > @@ -415,7 +449,7 @@ CameraConfiguration::Status
> IPU3CameraConfiguration::validate()
> >       }
> >
> >       /* Only compute the ImgU configuration if a YUV stream has been
> requested. */
> > -     if (yuvCount) {
> > +     if (videoCount) {
> >               pipeConfig0_ = data_->imgu0_->calculatePipeConfig(&pipe);
> >               if (pipeConfig0_.isNull()) {
> >                       LOG(IPU3, Error) << "Failed to calculate pipe
> configuration: "
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220802/5d72dced/attachment.htm>