[v3,0/6] Fix occasional software ISP assertion error on stop

22856 mbox series

When software ISP is stopped, there can be pending messages in the
message queue that will attempt to call the already stopped IPA,
resulting in an assertion error like this:

  FATAL default soft_ipa_proxy.cpp:456 assertion "state_ == ProxyRunning" failed in processStatsThread()

This patch series fixes the problem and also attempts to prevent other
problems related to message handling when or after software ISP is
stopped.

Changes in v3:
- Missing <deque> include added.
- I/O buffers are popped from the deque’s of queued buffers rather than
  searched there + assertion to check it’s the intended buffer added.
- Returned pending input buffers are canceled.
- The thread.cpp formatting patch dropped.
- The dispatching modifications patch split to several patches as
  suggested by Laurent.
- Thread::removeMessages() documentation improved as suggested by
  Laurent.
- SoftwareIsp::running_ dropped in the patch adding explicit SoftwareIsp
  messages removal.
- Formatting fixes suggested by Laurent.

Changes in v2:
- The receiver check in Thread::dispatchMessages moved to the right
  place.

Milan Zamazal (6):
  libcamera: software_isp: Emit ispStatsReady only if IPA is running
  libcamera: software_isp: Handle queued output buffers on stop
  libcamera: software_isp: Handle queued input buffers on stop
  libcamera: base: thread: Support dispatching for a specific receiver
  libcamera: software_isp: Dispatch messages on stop
  utils: ipc: Only dispatch messages for proxy when stopping thread

 include/libcamera/base/thread.h               |  3 +-
 .../internal/software_isp/software_isp.h      |  3 ++
 src/libcamera/base/thread.cpp                 | 21 ++++++-----
 src/libcamera/software_isp/software_isp.cpp   | 36 +++++++++++++++++--
 .../libcamera_templates/proxy_functions.tmpl  |  2 +-
 5 files changed, 53 insertions(+), 12 deletions(-)

Hi Milan,

On Tue, Feb 25, 2025 at 04:06:06PM +0100, Milan Zamazal wrote:
> When software ISP is stopped, there can be pending messages in the
> message queue that will attempt to call the already stopped IPA,
> resulting in an assertion error like this:
> 
>   FATAL default soft_ipa_proxy.cpp:456 assertion "state_ == ProxyRunning" failed in processStatsThread()
> 
> This patch series fixes the problem and also attempts to prevent other
> problems related to message handling when or after software ISP is
> stopped.

> Changes in v3:
> - Missing <deque> include added.
> - I/O buffers are popped from the deque’s of queued buffers rather than
>   searched there + assertion to check it’s the intended buffer added.
> - Returned pending input buffers are canceled.
> - The thread.cpp formatting patch dropped.
> - The dispatching modifications patch split to several patches as
>   suggested by Laurent.
> - Thread::removeMessages() documentation improved as suggested by
>   Laurent.
> - SoftwareIsp::running_ dropped in the patch adding explicit SoftwareIsp
>   messages removal.
> - Formatting fixes suggested by Laurent.


Tested with:
[PATCH] libcamera: delayed_controls: Inherit from Object class

The assertion is fixed and I do not see any other issues with
the set.

For the series:
Tested-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

Regards
Stanislaw

Hi Stanislaw,

Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> writes:

> Hi Milan,
>
> On Tue, Feb 25, 2025 at 04:06:06PM +0100, Milan Zamazal wrote:
>> When software ISP is stopped, there can be pending messages in the
>> message queue that will attempt to call the already stopped IPA,
>> resulting in an assertion error like this:
>> 
>>   FATAL default soft_ipa_proxy.cpp:456 assertion "state_ == ProxyRunning" failed in processStatsThread()
>> 
>> This patch series fixes the problem and also attempts to prevent other
>> problems related to message handling when or after software ISP is
>> stopped.
>
>> Changes in v3:
>> - Missing <deque> include added.
>> - I/O buffers are popped from the deque’s of queued buffers rather than
>>   searched there + assertion to check it’s the intended buffer added.
>> - Returned pending input buffers are canceled.
>> - The thread.cpp formatting patch dropped.
>> - The dispatching modifications patch split to several patches as
>>   suggested by Laurent.
>> - Thread::removeMessages() documentation improved as suggested by
>>   Laurent.
>> - SoftwareIsp::running_ dropped in the patch adding explicit SoftwareIsp
>>   messages removal.
>> - Formatting fixes suggested by Laurent.
>
>
> Tested with:
> [PATCH] libcamera: delayed_controls: Inherit from Object class
>
> The assertion is fixed and I do not see any other issues with
> the set.
>
> For the series:
> Tested-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

Good news, thank you for testing.

> Regards
> Stanislaw