[libcamera-devel] [PATCH 1/3] apps: cam: kms_sink: Do not process requests after stop()

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

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.

> 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