Skip to content

Conversation

@singalsu
Copy link
Contributor

No description provided.

@singalsu singalsu marked this pull request as ready for review April 18, 2023 11:42
The SRC should be a normal LL component, it can process 1 ms blocks
of audio in real-time. The domain_types = "1" is for DP scheduling
that is not enabled currently in Zephyr configuration. It causes a
pipeline prepare fail when a topology contains SRC.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch removes one unnecessary tab from section for SRCINTC.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
@singalsu singalsu force-pushed the config_srcintc_ll branch from 00c8356 to 7cf5786 Compare April 18, 2023 11:49
@singalsu
Copy link
Contributor Author

Update - fixed typos in 1st commit message.

Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add "request for change" mainly to make sure we are aligned on what an LL component should be. If everything can be a LL component, then we don't have an LL solution, do we?

affinity_mask = "0xF"
instance_count = "10"
domain_types = "1"
domain_types = "0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fail to see how an SRC qualifiers for a LL component? It's a heavy duty processing that is used to match rates that are precisely not aligned with the LL processing, so is this not going to introduce variability in the LL domain?

We want to keep the low-latency domain with as few heavy-duty processing as possible, precisely meet strict latency requirements.

Nothing prevents the SRC from being executed in a background task with lower priority and larger buffers.

Lost on this one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart We have no DP support on tgl in SOF mainline, so this either this is put to LL or disabled.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but let's agree that this is not the long-term direction. When DP is enabled, we have to revert this change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart src is not a typical DP.
yes, it is a heavy processing, but with same period as LL
Typical DPs are - voice recognition (>300ms processing period), MPEG decoders (26ms for mp3), etc.
As long as it is on the same core that rest of the pipeline, there's no point at all to use DP for it
DP processing is much heavier in fact (a thread!)

DP may be enabled on TGL, but it has never been tested there

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart We could already put SRC to DP in mtl.toml. Please @singalsu chime in but I was under impression SRC is LL. It can handle audio in LL-sized chunks, has fixed processing time (relation to amount of input data), so I'd imagine it can meet LL needs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart src is not a typical DP. yes, it is a heavy processing, but with same period as LL Typical DPs are - voice recognition (>300ms processing period), MPEG decoders (26ms for mp3), etc. As long as it is on the same core that rest of the pipeline, there's no point at all to use DP for it DP processing is much heavier in fact (a thread!)

If we have MP3 decoder + SRC, this entire chain does not need to be low-latency. It'll have extremely variable processing requirements depending on the source encoding.

Likewise if we have a 'media' pipeline with PCM deep-buffer + SRC, what is the point of the SRC being in the LL domain? At this point you've on-purpose increased the buffering to reduce power, latency was never a concern.

I don't see what benefits we get here by using LL... in my experience low-latency can only be done by moving all the cruft in the background - in other words do few things and do them well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fail to see how an SRC qualifiers for a LL component? It's a heavy duty processing that is used to match rates that are precisely not aligned with the LL processing, so is this not going to introduce variability in the LL domain?

We want to keep the low-latency domain with as few heavy-duty processing as possible, precisely meet strict latency requirements.

Nothing prevents the SRC from being executed in a background task with lower priority and larger buffers.

Lost on this one.

SRC can be well used e.g. to connect voice call processing to 48 kHz audio DAI in/out streams. Such processing need to be very low latency, and our 8/16/32 <-> 8/16/32 conversions are such. Even the 44.1 kHz <-> 48 kHz conversion is quite low latency due to the 2-stage polyphase matrix.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but the 'media' usages are not really low-latency, as measured by the latency added by the decoding process or the deep-buffer.

The problem is that we have one module used in completely different use cases. So if we do MP3 decode -> SRC -> 3rd party processing, the SRC would be tagged as LL even though the source and sink are not.

The point is that the arbitrary decision on the module domain does not make sense here. If you have a processing chain, going back and forth between LL and DP is questionable at best.

Copy link
Contributor Author

@singalsu singalsu Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, now it's not possible to load a topology with SRC at all. And TGL that I use for development won't get DP support so this config/tgl*-cavs.toml should be LL. MTL can have DP when the feature is ready but this PR is not covering it.

And we don't have DP versions of other components like peakvolume, so there will be a mix of components. The cSOF framework should in load time change the type if mix needs to be avoided.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart Your review is blocking merge of this (I re-asked review from you yesterday that cleared the red x). Can you reconsider?

@singalsu singalsu requested a review from plbossart April 26, 2023 12:58
@plbossart plbossart dismissed their stale review April 26, 2023 14:59

disagree and commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants