<div dir="ltr"><div dir="ltr">Hi,<br><br>Unfortunately this patch has introduced a tiny regression. It turns out that<br>calling stream on for an ISP node takes some hundreds of milliseconds to complete.<br>Most of this time is communicating and setup within the firmware layer. In<br>video encode use cases, this will cause a frame drop if we do not have enough<br>buffers to smooth out the glitch.<br><br>Not wanting to spend too much time and effort optimising this in the firmware<br>(particularly because I will likely break something else!), I will remove this<br>patch from the series, and we accept that we have to allocate a single buffer if<br>we have to drop frames for AE convergence. This is not disastrous, as typically<br>this will be a viewfinder size buffer.<br><br>I will post an updated series shortly.<br><br>Thanks,<br>Naush<br><div></div></div><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 6 Dec 2022 at 13:55, Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.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">If the pipeline handler is required to drop startup frames by the IPA, do not<br>
call StreamOn on the ISP Output0 and Output1 device nodes from<br>
PipelineHandlerRPi::start(). This stops the ISP from generating output on those<br>
channels giving improved latency, and more crucially does not require internal<br>
buffers to be allocated to deal with this condition.<br>
<br>
Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
---<br>
.../pipeline/raspberrypi/raspberrypi.cpp | 36 ++++++++++++++++---<br>
1 file changed, 32 insertions(+), 4 deletions(-)<br>
<br>
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
index 9a316eda6fea..302279ac5e01 100644<br>
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
@@ -1088,8 +1088,18 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)<br>
<br>
data->state_ = RPiCameraData::State::Idle;<br>
<br>
- /* Start all streams. */<br>
+ /*<br>
+ * Start all streams with the exception of ISP Output0 and Output1 if<br>
+ * we are dropping some start-up frames. This saves a tiny bit of latency<br>
+ * and avoids the need for allocating an Output0 buffer only to handle<br>
+ * startup drop frame conditions.<br>
+ */<br>
for (auto const stream : data->streams_) {<br>
+ if (data->dropFrameCount_ &&<br>
+ (stream == &data->isp_[Isp::Output0] ||<br>
+ stream == &data->isp_[Isp::Output1]))<br>
+ continue;<br>
+<br>
ret = stream->dev()->streamOn();<br>
if (ret) {<br>
stop(camera);<br>
@@ -1425,6 +1435,14 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)<br>
if (ret < 0)<br>
return ret;<br>
} else {<br>
+ /*<br>
+ * We haven't enabled streaming yet for Output0 and Output1<br>
+ * for startup frame drops, so don't queue any buffers.<br>
+ */<br>
+ if (stream == &data->isp_[Isp::Output0] ||<br>
+ stream == &data->isp_[Isp::Output1])<br>
+ continue;<br>
+<br>
/*<br>
* For external streams, we must queue up a set of internal<br>
* buffers to handle the number of drop frames requested by<br>
@@ -2158,16 +2176,26 @@ void RPiCameraData::checkRequestCompleted()<br>
}<br>
<br>
/*<br>
- * Make sure we have three outputs completed in the case of a dropped<br>
- * frame.<br>
+ * Only the ISP statistics output is generated when we are dropping<br>
+ * frames on startup.<br>
*/<br>
if (state_ == State::IpaComplete &&<br>
- ((ispOutputCount_ == 3 && dropFrameCount_) || requestCompleted)) {<br>
+ ((ispOutputCount_ == 1 && dropFrameCount_) || requestCompleted)) {<br>
state_ = State::Idle;<br>
if (dropFrameCount_) {<br>
dropFrameCount_--;<br>
LOG(RPI, Debug) << "Dropping frame at the request of the IPA ("<br>
<< dropFrameCount_ << " left)";<br>
+<br>
+ /*<br>
+ * If we have finished dropping startup frames, start<br>
+ * streaming on the ISP Output0 and Output1 nodes for<br>
+ * normal operation.<br>
+ */<br>
+ if (!dropFrameCount_) {<br>
+ isp_[Isp::Output0].dev()->streamOn();<br>
+ isp_[Isp::Output1].dev()->streamOn();<br>
+ }<br>
}<br>
}<br>
}<br>
-- <br>
2.25.1<br>
<br>
</blockquote></div></div>