[libcamera-devel] [PATCH 1/3] apps: cam: kms_sink: Do not process requests after stop()
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Apr 24 10:25:30 CEST 2023
Hi Umang,
On Mon, Apr 24, 2023 at 01:16:41PM +0530, Umang Jain wrote:
> On 4/24/23 6:35 AM, Laurent Pinchart wrote:
> > On Mon, Apr 24, 2023 at 02:09:29AM +0530, Umang Jain via libcamera-devel wrote:
> >> KMSSink might process completed page flip requests from DRM
> >> after stop() has been called. This is not right hence connect the
> >> Device::requestCompleted signal on start() and disconnect it on stop().
> >
> > I wonder if this doesn't hide another issue. The stop() function queues
> > a synchronous request to disable the display pipeline. This should flush
> > all the other in-flight requests. If another request completes after
> > that, it may indicate that we queue it after calling stop(), which I'd
> > rather fix than hiding the problem.
>
> Ok - partially agreeing with you here.
>
> I agree the application should not queue requests if the sink has
> stopped. But if they do (erratic behaviour by the app), KMSSink should
> return false in processRequest() instead of actually processing it. It
> can be implemented with state (isStopped_) boolean check or similar.
I'm fine with catching the issue in KMSSink as well as fixing the
caller.
> >> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> >> ---
> >> src/apps/cam/kms_sink.cpp | 5 ++++-
> >> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/apps/cam/kms_sink.cpp b/src/apps/cam/kms_sink.cpp
> >> index 353209cd..a508977d 100644
> >> --- a/src/apps/cam/kms_sink.cpp
> >> +++ b/src/apps/cam/kms_sink.cpp
> >> @@ -63,7 +63,6 @@ KMSSink::KMSSink(const std::string &connectorName)
> >> return;
> >> }
> >>
> >> - dev_.requestComplete.connect(this, &KMSSink::requestComplete);
> >> }
> >>
> >> void KMSSink::mapBuffer(libcamera::FrameBuffer *buffer)
> >> @@ -328,11 +327,15 @@ int KMSSink::start()
> >> return ret;
> >> }
> >>
> >> + dev_.requestComplete.connect(this, &KMSSink::requestComplete);
> >> +
> >> return 0;
> >> }
> >>
> >> int KMSSink::stop()
> >> {
> >> + dev_.requestComplete.disconnect();
> >> +
> >> /* Display pipeline. */
> >> DRM::AtomicRequest request(&dev_);
> >>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list