Src tplg fix #8412
Src tplg fix #8412
Conversation
| @@ -27,7 +27,7 @@ | |||
| ] | |||
| out_bit_depth [ 32 ] | |||
| out_valid_bit_depth [ 32 ] | |||
| obs "$[($out_channels * (($[($out_rate + 999)] / 1000) + 4)) * ($out_bit_depth / 8)]" | |||
| obs "$[$out_channels * (($out_rate / 1000) + 1) * ($out_bit_depth / 8)]" | |||
There was a problem hiding this comment.
I think this would benefit from more explanation and examples with some values. So the base class version of obs calculation is not enough (output_audio_format.conf), but why is 1 extra word enough in this case. I think a similar documentation note as in output_audio_format.conf would be in order here.
There was a problem hiding this comment.
@btian1 Have you been able to run e.g. the SRC capture and playback in sof-nocodec topology with clean audio with this patched topology with any of these impacted rates?
There was a problem hiding this comment.
Can't listen audio in real time with nocodec, but the topology is arranged such that you can capture from PCM the playback to PCM via SSP loopback. Launch arecord from one shell window and aplay from other. Then copy the recording to you PC and listen if it's clean and with correct rate/duration.
We don't have SRC tests coverage with topology in CI so this is do-it-yourself.
|
@btian1 can you check IPC3 build test result, it looks like not related to this PR. Thanks |
it is smart amp error, should already be fixed with latest, let me force push again to get pass result. |
|
SOFCI TEST |
22050Hz obs also need a separate setting, this was missed when first create this file, now move it to the group with obs setting. Signed-off-by: Baofeng Tian <[email protected]>
From 48k input, the obs for 11025/22050/44100/88200/176400 setting need take care, previous implementation left 5 words redundancy for output, this change removed redundancy, since actual output buffer size always use double obs size. Signed-off-by: Baofeng Tian <[email protected]>
There was a problem hiding this comment.
Not sure why we need to override "obs" anymore if there is no change to baseclass definition, but the comment might be useful to remind why extra space is not required.
I'm not 100% sure how wise it is to rely on 2*obs definition. To me, if we have a field for output block size, modules should specify what they actually need, and not assume that the user of this information will anyways double the allocation. But seems others are happy, so let's go with this and leave the comment in place.
| @@ -27,7 +27,8 @@ | |||
| ] | |||
| out_bit_depth [ 32 ] | |||
| out_valid_bit_depth [ 32 ] | |||
| obs "$[($out_channels * (($[($out_rate + 999)] / 1000) + 4)) * ($out_bit_depth / 8)]" | |||
| # actual buffer size in sof is obs * 2, so here simply set to its period size | |||
| obs "$[$out_channels * ($out_bit_depth / 8) * (($out_rate / 1000)) * ($period / 1000)]" | |||
There was a problem hiding this comment.
@btian1 But isn't this the same definition we have in the baseline? So if this doesn't require custom obs logic, we can remove this altogether?
There was a problem hiding this comment.
obs "$[(
this adding period into count, however, this may change further, a lot of related changes are happening.
let's wait and see.
No description provided.