Codec Adapter: Use the passed output buffers in process() #5559

ranj063 · 2022-03-16T20:34:16Z

No description provided.

remove init for ret. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Remove init for source. It is not needed anymore. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

dbaluta · 2022-03-17T12:41:37Z

First there is a compilation error that needs to be fixed:

In file included from /home/daniel/work/sof_dir/sof/src/include/sof/audio/buffer.h:20,
                 from /home/daniel/work/sof_dir/sof/src/audio/codec_adapter/codec_adapter.c:16:
/home/daniel/work/sof_dir/sof/src/audio/codec_adapter/codec_adapter.c: In function 'codec_adapter_copy':
/home/daniel/work/sof_dir/sof/src/include/sof/coherent.h:274:10: error: 'src_buffer' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  274 |   c->key = k_spin_lock(&c->lock);
/home/daniel/work/sof_dir/sof/src/audio/codec_adapter/codec_adapter.c:505:23: note: 'src_buffer' was declared here
  505 |   struct comp_buffer *src_buffer;
      |                       ^~~~~~~~~~
[ 80%] Building C object CMakeFiles/sof.dir/src/lib/notifier.c.o
[ 82%] Building C object CMakeFiles/sof.dir/src/lib/pm_runtime.c.o
[ 82%] Building C object CMakeFiles/sof.dir/src/lib/clk.c.o
[ 83%] Building C object CMakeFiles/sof.dir/src/lib/dma.c.o

src/audio/codec_adapter/codec_adapter.c

cujomalainey · 2022-03-17T20:37:45Z

src/audio/codec_adapter/codec_adapter.c

does this mean it is no longer up to the IPC messages to allocate the buffers?

@cujomalainey sorry not following the question. Which buffers are you alluding to? This list of buffers is a replacement for mod->local_buff which assumes there's only one sink for a module. So we expand it to a list to support multiple sinks

what does mean "support multiple sinks".?

@lenghonglin imagine a demux where you can have a single source split and copied to multiple sinks. Thats is what I mean by "support multiple sinks"

Add a sink_buffer_list to struct processing_module to support multiple output buffers for modules. Allocate sink buffers for all sinks during prepare and copy the produced samples from the output_buffers into the respective sink buffers during copy. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Add a helper function to copy from local buffer to the sink buffer. This will be used to copy from all output buffers. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Modify all codec implementations to copy the produced samples to the output_buffer. Copy the produced samples from all output buffers in the sink_buffer_list. Remove local_buff from struct processing_module as it is no longer needed. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 · 2022-03-18T04:05:37Z

@markbartonxperi can you please check if this PR works for you?

markbartonxperi · 2022-03-18T08:58:15Z

@ranj063 Looks good. Thanks!

dbaluta

Thanks @ranj063 this works fine for i.MX. Also, incremental patches compile fine.

lgirdwood · 2022-03-18T14:47:55Z

CI looks good.

hlleng · 2022-03-20T07:46:54Z

src/audio/codec_adapter/codec_adapter.c

+
+copy_period:
+	comp_get_copy_limits_with_lock(src_buffer, sink_buffer, &cl);
+	copy_bytes = cl.frames * cl.source_frame_bytes;


What if copy_bytes is zero?

I dont think you can get here is copy_bytes is 0. we get to this point only if audio_stream_get_avail_bytes(&src_buffer->stream) conditions above are met

for dai comp, if comsumer(codec) slow than producter(codec_adapter), the dai buffer will be fulled, so in this condition, copy_bytes should be 0.

@lenghonglin even if copy_bytes is 0, the operations below will return without doing anything in that case. But sure, I can add a check for this in a follow up PR

hlleng · 2022-03-22T08:52:55Z

There is another thing that i wanna disscuss.
For example, the mp3 is mpeg2 layer 3 16k 16bit . so codec_adapter input size is 72 bytes, output size is 1152 bytes. and the comp buffer size is 4k.
when host send TRIGER_STOP. there is 4k buffer in sof is not to be comused

So what time for host to send TRIGER_STOP?

dbaluta · 2022-03-22T08:55:34Z

So, you say that we don't play the entire mp3 file till the end right? I think the solution here is to implement DRAIN. Patches are welcomed if you can implement this.

I have this on my todo list but with low prio this quarter.

hlleng · 2022-03-22T09:13:58Z

ok, i will add this thing in my todolist

ranj063 requested review from cujomalainey, dbaluta, lbetlej, lgirdwood, mmaka1, mrajwa and plbossart as code owners March 16, 2022 20:34

ranj063 added 2 commits March 16, 2022 13:34

codec_adapter: no need to init ret

aee0495

remove init for ret. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

codec_adapter: no need to init source

fc44362

Remove init for source. It is not needed anymore. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 force-pushed the fix/codec_bytes_to_process branch 2 times, most recently from 9f57812 to 92e3a22 Compare March 16, 2022 22:17

dbaluta mentioned this pull request Mar 17, 2022

[SOF CODEC]how can i use sof/cadence decoder or encoder? #5381

Open

ranj063 force-pushed the fix/codec_bytes_to_process branch from 8b5e036 to 8874273 Compare March 17, 2022 13:32

dbaluta reviewed Mar 17, 2022

View reviewed changes

src/audio/codec_adapter/codec_adapter.c Outdated Show resolved Hide resolved

ranj063 force-pushed the fix/codec_bytes_to_process branch 2 times, most recently from 1d0af51 to aa185f3 Compare March 17, 2022 18:06

cujomalainey reviewed Mar 17, 2022

View reviewed changes

ranj063 force-pushed the fix/codec_bytes_to_process branch 6 times, most recently from 74165b9 to a11a065 Compare March 18, 2022 01:47

ranj063 added 3 commits March 17, 2022 20:40

codec_adapter: Add a helper function to copy from local buffer to sink

1cf316a

Add a helper function to copy from local buffer to the sink buffer. This will be used to copy from all output buffers. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 force-pushed the fix/codec_bytes_to_process branch from 51358e0 to e562ecc Compare March 18, 2022 03:41

dbaluta approved these changes Mar 18, 2022

View reviewed changes

lgirdwood approved these changes Mar 18, 2022

View reviewed changes

ranj063 requested a review from cujomalainey March 18, 2022 16:08

hlleng reviewed Mar 20, 2022

View reviewed changes

dbaluta merged commit 4f8a70d into thesofproject:main Mar 21, 2022

ranj063 deleted the fix/codec_bytes_to_process branch March 22, 2022 16:20

Codec Adapter: Use the passed output buffers in process() #5559

Codec Adapter: Use the passed output buffers in process() #5559

Uh oh!

Conversation

ranj063 commented Mar 16, 2022

Uh oh!

dbaluta commented Mar 17, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranj063 commented Mar 18, 2022

Uh oh!

markbartonxperi commented Mar 18, 2022

Uh oh!

dbaluta left a comment

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Mar 18, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranj063 Mar 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hlleng commented Mar 22, 2022

Uh oh!

dbaluta commented Mar 22, 2022

Uh oh!

hlleng commented Mar 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ranj063 Mar 20, 2022 •

edited

Loading