<div dir="ltr"><div dir="ltr">Hi Umang,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Aug 2, 2022 at 3:53 PM Umang Jain <<a href="mailto:umang.jain@ideasonboard.com" target="_blank">umang.jain@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Harvey,<br>
<br>
On 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:<br>
> When StillCapture and other non-raw configurations are requested,<br>
> assigns |outCaptureStream| instead of |outStream| to the StillCapture<br>
> configuration.<br>
><br>
> Signed-off-by: Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
> ---<br>
> src/libcamera/pipeline/ipu3/ipu3.cpp | 50 +++++++++++++++++++++++-----<br>
> 1 file changed, 42 insertions(+), 8 deletions(-)<br>
><br>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp<br>
> index ec9d14d1..e26c2736 100644<br>
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp<br>
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp<br>
> @@ -254,8 +254,10 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()<br>
> * \todo Clarify the IF and BDS margins requirements.<br>
> */<br>
> unsigned int rawCount = 0;<br>
> - unsigned int yuvCount = 0;<br>
> + unsigned int videoCount = 0;<br>
> + unsigned int stillCount = 0;<br>
> Size maxYuvSize;<br>
> + Size maxVideoSize;<br>
> Size rawSize;<br>
> <br>
> for (const StreamConfiguration &cfg : config_) {<br>
> @@ -264,16 +266,29 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()<br>
> if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {<br>
> rawCount++;<br>
> rawSize.expandTo(cfg.size);<br>
> + } else if (cfg.streamRole == StreamRole::StillCapture) {<br>
> + if (stillCount != 0)<br>
> + return Invalid;<br>
> +<br>
> + stillCount++;<br>
> + maxYuvSize.expandTo(cfg.size);<br>
> } else {<br>
> - yuvCount++;<br>
> + videoCount++;<br>
> maxYuvSize.expandTo(cfg.size);<br>
> + maxVideoSize.expandTo(cfg.size);<br>
> }<br>
> }<br>
> <br>
> - if (rawCount > 1 || yuvCount > 2) {<br>
> + if (videoCount == 0 && stillCount == 1) {<br>
> + stillCount = 0;<br>
> + videoCount = 1;<br>
> + maxVideoSize.expandTo(maxYuvSize);<br>
> + }<br>
<br>
<br>
Maybe you can explain this if block with a brief comment on the case why <br>
we invert stillCount and videoCount.<br>
<br></blockquote><div><br></div><div>Added a simple one. Please check.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +<br>
> + if (rawCount > 1 || videoCount > 2) {<br>
> LOG(IPU3, Debug) << "Camera configuration not supported";<br>
> return Invalid;<br>
> - } else if (rawCount && !yuvCount) {<br>
> + } else if (rawCount && !stillCount && !videoCount) {<br>
> /*<br>
> * Disallow raw-only camera configuration. Currently, ImgU does<br>
> * not get configured for raw-only streams and has early return<br>
> @@ -316,6 +331,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()<br>
> ImgUDevice::Pipe pipe{};<br>
> pipe.input = cio2Configuration_.size;<br>
> <br>
> + ImgUDevice::Pipe pipe1{};<br>
> + pipe1.input = cio2Configuration_.size;<br>
> +<br>
> /*<br>
> * Adjust the configurations if needed and assign streams while<br>
> * iterating them.<br>
> @@ -380,18 +398,34 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()<br>
> cfg->stride = info.stride(cfg->size.width, 0, 1);<br>
> cfg->frameSize = info.frameSize(cfg->size, 1);<br>
> <br>
> + if (stillCount == 1 && cfg->streamRole == StreamRole::StillCapture) {<br>
<br>
<br>
Maybe check both is redundant? Can stillCount be converted to <br>
ASSERT(stillCount != 0) inside the block?<br>
<br></blockquote><div><br></div><div>Sure. Use |ASSERT(stillCount == 1)| instead, as it only supports one StillCapture stream.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> + LOG(IPU3, Debug) << "Assigned "<br>
> + << cfg->toString()<br>
> + << " to the imgu1 main output";<br>
> + cfg->setStream(const_cast<Stream *>(&data_->outCaptureStream_));<br>
> +<br>
> + pipe1.main = cfg->size;<br>
> + pipe1.viewfinder = pipe1.main;<br>
> +<br>
> + pipeConfig1_ = data_->imgu1_->calculatePipeConfig(&pipe1);<br>
> + if (pipeConfig1_.isNull()) {<br>
> + LOG(IPU3, Error) << "Failed to calculate pipe configuration: "<br>
> + << "unsupported resolutions.";<br>
> + return Invalid;<br>
> + }<br>
> /*<br>
> * Use the main output stream in case only one stream is<br>
> * requested or if the current configuration is the one<br>
> * with the maximum YUV output size.<br>
> */<br>
> - if (mainOutputAvailable &&<br>
> - (originalCfg.size == maxYuvSize || yuvCount == 1)) {<br>
> + } else if (mainOutputAvailable &&<br>
> + (originalCfg.size == maxVideoSize ||<br>
> + videoCount == 1)) {<br>
> cfg->setStream(const_cast<Stream *>(&data_->outStream_));<br>
> mainOutputAvailable = false;<br>
> <br>
> pipe.main = cfg->size;<br>
> - if (yuvCount == 1)<br>
> + if (videoCount == 1)<br>
> pipe.viewfinder = pipe.main;<br>
> <br>
> LOG(IPU3, Debug) << "Assigned " << cfg->toString()<br>
> @@ -415,7 +449,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()<br>
> }<br>
> <br>
> /* Only compute the ImgU configuration if a YUV stream has been requested. */<br>
> - if (yuvCount) {<br>
> + if (videoCount) {<br>
> pipeConfig0_ = data_->imgu0_->calculatePipeConfig(&pipe);<br>
> if (pipeConfig0_.isNull()) {<br>
> LOG(IPU3, Error) << "Failed to calculate pipe configuration: "<br>
</blockquote></div></div>