Repository navigation
module-adapter: allow different statuses - #7052
Conversation
There was a problem hiding this comment.
@lyakh this is a special case for the mixin/mixout alone right or is it widely applicable for any connected sink pipeline because of the order in which trigger pipelines with IPC4?
There was a problem hiding this comment.
@ranj063 I'm not sure. Definitely it's only possibly with module-adapter components. In our "popular" topologies I only see mixin-out, copier and volume being on the border between two pipelines. Copier isn't using module-adapter, but we could check some volume pipelines. I so far haven't figured out where in volume consumed and produced bytes are updated, will try to figure that out tomorrow. Then I'll add some debugging there and check.
There was a problem hiding this comment.
@ranj063 I tested with a capture pipeline like DMIC -> gain.A -> copier.A -> gain.B -> PCM and I don't see any difference with or without this PR so at least it isn't breaking that use-case either
There was a problem hiding this comment.
@ranj063 I know why there was no difference. The difference is that although in one case module_process() is called from module_adapter_simple_copy() with num_input_buffers of num_output_buffers equal to 0, both those parameters are ignored by volume_process()... If I add there something like
if (!num_output_buffers || !num_input_buffers)
return 0;
then of course the behaviour changes. But still both possibilities seem to work. Maybe it's because with capture usually you don't have to send data at a constant rate. And also the effect of this PR seems to depend strongly on individual processing modules - how they use those parameters.
There was a problem hiding this comment.
Ack, I don't think this can work if the adapter makes this call. I think it should be component that handles this logic, and indeed, if it has data in its buffers, it is ACTIVE and there's room in outputs, processing should happen.
|
The only CI failure is a timed-out suspend-resume test https://sof-ci.01.org/sofpr/PR7052/build3776/devicetest/index.html?model=TGLU_RVP_NOCODEC_IPC4ZPH&testcase=check-suspend-resume-5 |
@lyakh the internal CI tests have failed too |
@ranj063 I cannot open the results, does it work for you? |
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
I think this is the correct thing to do.
There was a problem hiding this comment.
Ack, I don't think this can work if the adapter makes this call. I think it should be component that handles this logic, and indeed, if it has data in its buffers, it is ACTIVE and there's room in outputs, processing should happen.
The following problem is observed when testing pause functionality: in a case of a playback stream host -> mixin -> mixout -> DAI where host and mixin belong to the pipeline A and mixout and DAI belong to pipeline B, pipeline B is scheduled before A. So at start we get: scheduler start B: no data A: read 1 period of data scheduler sleep (1 period of data pending in pipeline A) scheduler start B: send 1 period of data out A: read 1 period of data scheduler sleep (1 period of data pending in pipeline A) PAUSE RELEASE scheduler start B: old data before sleep isn't sent because A is PAUSED A: read 1 period of data scheduler sleep (2 periods of data pending in pipeline A) ... With repeated pause-release cycles this leads to buffer overrun. To fix this allow sending out data by pipeline B when A is still paused. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
ok, so this now aligns the buffer period state at start and after pause.
|
CI fully passing. @ranj063 will merge, we can deal with speed bumps after we see daily test. |
The following problem is observed when testing pause functionality:
in a case of a playback stream host -> mixin -> mixout -> DAI where host and mixin belong to the pipeline A and mixout and DAI belong to pipeline B, pipeline B is scheduled before A. So at start we get: scheduler start
B: no data
A: read 1 period of data
scheduler sleep
(1 period of data pending in pipeline A)
scheduler start
B: send 1 period of data out
A: read 1 period of data
scheduler sleep
(1 period of data pending in pipeline A)
PAUSE
RELEASE
scheduler start
B: old data before sleep isn't sent because A is PAUSED A: read 1 period of data
scheduler sleep
(2 periods of data pending in pipeline A)
...
With repeated pause-release cycles this leads to buffer overrun. To fix this allow sending out data by pipeline B when A is still paused.