<div dir="ltr"><div dir="ltr">Hi David,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 17 Mar 2022 at 14:32, David Plowman <<a href="mailto:david.plowman@raspberrypi.com">david.plowman@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">Hi Naush<br>
<br>
Thanks for the patch!<br>
<br>
Perhaps I'm coming a bit late to this particular discussion, but this<br>
makes me a bit nervous. I'm worried about deallocations/allocations<br>
happening at unpredictable moments relative to other things in the<br>
system. Could there be a risk of creating more intermittent and<br>
hard-to-reproduce failures?<br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
One example is the Pi 3 where I think we know that CMA fragmentation<br>
is already a problem. I wonder if I would rather be able to control<br>
when/whether buffers get freed? But it does make the API more involved<br>
and leaves an application with more to think about.<br></blockquote><div><br></div><div><div>Yes, this is a distinct possibility.  I'm fairly confident this will not be an issue with libcamera-apps,</div><div>as the timeout will never hit.  However, picamera2 can certainly invoke the timeout and</div><div>cause issues.  Hmmm.... not sure which way to go here.... perhaps an application flag</div><div>would indeed be better.</div><div> </div><div>Naush</div><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>
Possibly I'm just worrying too much?<br>
<br>
Thanks!<br>
David<br>
<br>
On Thu, 17 Mar 2022 at 14:08, Naushir Patuck via libcamera-devel<br>
<<a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a>> wrote:<br>
><br>
> Start a 5 second timer when stopDevice() is called, and if it times out,<br>
> free all internally allocated buffers. The timer is cleared when start() is<br>
> subsequently called.<br>
><br>
> This avoids the pipeline handler from holding onto internal buffers for long<br>
> periods of time due to application inactivity.<br>
><br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
>  .../pipeline/raspberrypi/raspberrypi.cpp        | 17 +++++++++++++++++<br>
>  1 file changed, 17 insertions(+)<br>
><br>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> index d3212b193ced..32b282b4bdbd 100644<br>
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> @@ -15,6 +15,7 @@<br>
>  #include <utility><br>
><br>
>  #include <libcamera/base/shared_fd.h><br>
> +#include <libcamera/base/timer.h><br>
>  #include <libcamera/base/utils.h><br>
><br>
>  #include <libcamera/camera.h><br>
> @@ -45,6 +46,8 @@<br>
>  #include "dma_heaps.h"<br>
>  #include "rpi_stream.h"<br>
><br>
> +using namespace std::literals::chrono_literals;<br>
> +<br>
>  namespace libcamera {<br>
><br>
>  LOG_DEFINE_CATEGORY(RPI)<br>
> @@ -288,6 +291,9 @@ public:<br>
>         /* Has this camera been reconfigured? */<br>
>         bool reallocate_;<br>
><br>
> +       /* Timer to free internal buffers once stopDevice() has been called. */<br>
> +       Timer stopTimer_;<br>
> +<br>
>  private:<br>
>         void checkRequestCompleted();<br>
>         void fillRequestMetadata(const ControlList &bufferControls,<br>
> @@ -991,6 +997,10 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)<br>
>         RPiCameraData *data = cameraData(camera);<br>
>         int ret;<br>
><br>
> +       /* Disable the stop timeout waiting to release buffers after a stop(). */<br>
> +       data->stopTimer_.stop();<br>
> +       data->stopTimer_.timeout.disconnect();<br>
> +<br>
>         for (auto const stream : data->streams_)<br>
>                 stream->resetBuffers();<br>
><br>
> @@ -1071,6 +1081,13 @@ void PipelineHandlerRPi::stopDevice(Camera *camera)<br>
><br>
>         /* Stop the IPA. */<br>
>         data->ipa_->stop();<br>
> +<br>
> +       /*<br>
> +        * Start a 5 second timer once the camera is stopped, and on a timeout,<br>
> +        * release all internally allocated buffers.<br>
> +        */<br>
> +       data->stopTimer_.timeout.connect(data, &RPiCameraData::freeBuffers);<br>
> +       data->stopTimer_.start(5s);<br>
>  }<br>
><br>
>  int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request)<br>
> --<br>
> 2.25.1<br>
><br>
</blockquote></div></div>