Repository navigation
Conversation
marcinszkudlinski
left a comment
marcinszkudlinski
left a comment
There was a problem hiding this comment.
007 branch is a frozen release "stable" branch for MTL, please don't merge this commit.
| CONFIG_COMP_GOOGLE_RTC_AUDIO_PROCESSING=y | ||
| CONFIG_COMP_GOOGLE_RTC_AUDIO_PROCESSING_NUM_CHANNELS=2 | ||
| CONFIG_GOOGLE_RTC_AUDIO_PROCESSING_MIC_BITS=32 | ||
|
|
There was a problem hiding this comment.
Please review in #8571 . This PR is just a place to deliver the same code against mtl-007-drop-stable.
There was a problem hiding this comment.
Oh, actually this patch got dropped from that branch somehow! Will fix. But when I do please review there :)
| struct sof_source *data_src = dp_queue_get_source(dp_queue); | ||
| size_t dp_data_available = source_get_data_available(data_src); | ||
|
|
||
| if (!dp_data_available) |
There was a problem hiding this comment.
this error is a real error - if DP processing is working it will never appear, even during pipelie startup
kv2019i
changed the title
| Object.Control.bytes."1" { | ||
| access [ | ||
| tlv_write | ||
| tlv_callback |
There was a problem hiding this comment.
is callback a kernel or userspace permission?
There was a problem hiding this comment.
This is a cherry pick from main. And "access" in this context refers (I think!) to the API variant used to access the control's data. "callback" is what needs to be used for large TLV blocks because they can't be sent in a single IPC command.
| #define CHAN_MAX CONFIG_COMP_GOOGLE_RTC_AUDIO_PROCESSING_CHANNEL_MAX | ||
|
|
||
| static __aligned(PLATFORM_DCACHE_ALIGN) | ||
| float refoutbuf[CHAN_MAX][NUM_FRAMES]; |
There was a problem hiding this comment.
not a fan of having these here long term. Should we want to start supporting >1 AEC instance (e.g. on HS) this will not end well.
There was a problem hiding this comment.
Please review the main PR. This is just a rebase on top of the device branch for testing. But to answer: Trying to grab half the heap all at once (which is literally what we were doing here) doesn't really scale either. It's just not a great situation, but static allocation in firmware is widely held to be the more reliable. SOF is a little unique in being heavily dynamic, which works fine when you're looking at allocating individual 4-8k buffers. AEC wants 200k by default and last I heard Lionel was saying 390k would be better.
Longer term we could try to integrate the AEC internal allocation with the external heap better (there are hooks already but they only work for overflow use), which might be the best choice overall.
| struct sof_source *ref, | ||
| struct sof_source *mic, | ||
| struct sof_sink *out) | ||
| static ALWAYS_INLINE float s16_to_float(const char *ptr) |
There was a problem hiding this comment.
(Again, please continue review in the main PR, but answering here):
This was discussed earlier: Those generate horrifyingly pessimal code (runtime division!) and can't be fixed without breaking the ABI (because as specified they work for 64 bit values). These seem like they're intended for generating compile time constants for fixed point use. There's also a pcm_convert API elsewhere in SOF, but that's designed to work in a block on linear arrays of samples (and don't expose inlinable single-sample conversions) where here we need to unify de/interleaving with the conversion.
Basically: SOF just doesn't have an API for this. I could move this somewhere else and try to unify with an existing library.
|
Updated with current versions of all my in-review PRs, unifying the AEC work in main with the MTL branch. |
|
Close this one. Main has now caught up with 007-drop-stable and this works directly on top of it. No need to maintain the separate branch anymore. |
Squashed changes from #8571 , rebased on top of the mtl-007-drop-stable branch so we can fix the divergences in this code.
Review there. Not here.