<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div><div>Thank you for your feedback.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 22 Mar 2022 at 20:16, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@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 Naush,<br>
<br>
Thank you for the patch.<br>
<br>
On Tue, Mar 22, 2022 at 09:22:55AM +0000, Naushir Patuck via libcamera-devel wrote:<br>
> If the device is in the process of being stopped (i.e. Stopping state), any<br>
> call to queueBuffer() must fail. This is to ensure the integrity of the buffer<br>
> queue, as it gets cleared at the end of streamOff.<br>
> <br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
> src/libcamera/v4l2_videodevice.cpp | 8 ++++++++<br>
> 1 file changed, 8 insertions(+)<br>
> <br>
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp<br>
> index 9cea6a608660..1516be129d73 100644<br>
> --- a/src/libcamera/v4l2_videodevice.cpp<br>
> +++ b/src/libcamera/v4l2_videodevice.cpp<br>
> @@ -1491,6 +1491,9 @@ int V4L2VideoDevice::releaseBuffers()<br>
> * The best available V4L2 buffer is picked for \a buffer using the V4L2 buffer<br>
> * cache.<br>
> *<br>
> + * Note that \a queueBuffer will fail if the device is in the process of being<br>
<br>
If you write it queueBuffer() doxygen should recognize the function call<br>
and generate proper links, you can drop the \a. Same below.<br>
<br>
> + * stopped from a streaming state through the \a streamOff function.<br>
> + *<br>
> * \return 0 on success or a negative error code otherwise<br>
> */<br>
> int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)<br>
> @@ -1499,6 +1502,11 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)<br>
> struct v4l2_buffer buf = {};<br>
> int ret;<br>
> <br>
> + if (state_ == State::Stopping) {<br>
> + LOG(V4L2, Error) << "Device is in a stopping state.";<br>
<br>
Does this currently happen ? I assume it does, causing the unit test<br>
failures in v3 of the series. Is it a bug on the application side, or a<br>
problem elsewhere ? Are the cam, qcam and simple-cam applications<br>
affected ? We should not have any error message printed under normal<br>
operation conditions, so I'd like to at least know what needs to be<br>
fixed.<br></blockquote><div><br></div><div>This does occur, but only in the v4l2_m2mdevice test (that I've found). Applications</div><div>are not directly affected, as this is on the interface between the pipeline handlers</div><div>and the v4l2 device. The RPi pipeline handler does not re-queue buffers into the</div><div>device when we are stopping. I've had a brief look at the ipu3 and rkisp ones, and</div><div>I don't think they do either, but it would be good for someone else to confirm.</div><div><br></div><div>Kieran and I briefly talked about what to do with the v4l2_m2mdevice test, it would</div><div>be an easy fix to correct the behavior and get rid of the above log messages popping</div><div>out with this change.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
> + return -EPERM;<br>
<br>
I'm tempted to use -ESHUTDOWN as a more precise alternative. Any opinion<br>
from anyone ?<br></blockquote><div><br></div><div>I am fine with ESHUTDOWN if others think it is more appropriate.</div><div> </div><div><div>Regards,</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>
> + }<br>
> +<br>
> /*<br>
> * Pipeline handlers should not requeue buffers after releasing the<br>
> * buffers on the device. Any occurence of this error should be fixed<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>