RFC: Add final support for compress audio #3959

dbaluta · 2021-03-25T19:03:17Z

This patch series adds the final support patches for compress audio playback.
It mostly touches the codec_adapter() copy function which was initially designed only for PCM frames.

This patches are sent for an early review. They will eventually get merged after I will sort out @cujomalainey PR: #3881

@mrajwa @paulstelian97 @iuliana-prodan @cujomalainey @stolx please have a look.

@cujomalainey I will start working on sorting out #3881 from now on.

init_process API is used to initialize the processing phase. (e.g for Cadence: search for valid header, decode header in order to get parameters). init_process needs to have access to the input frames so it will be called by codec adapter copy() function. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

local buffer is used to store the output of processing function. So, we need to compute its size based on codec_adapter output buffer not input buffer. For non-compressed processing the current code works because input and output buffer have equal sizes. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

processed field keeps the total number of processed bytes from the input buffer. We need to update this field also for the process_init operation. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

src/audio/codec_adapter/codec_adapter.c

paulstelian97 · 2021-03-26T08:36:35Z

tools/topology/sof/pipe-codec-adapter-playback.m4

We need to figure out what's with this hardcoding.

Yes, yes! Will fix it asap.

tools/topology/sof-imx8-processing-wm8960.m4

src/audio/codec_adapter/codec_adapter.c

iuliana-prodan · 2021-03-26T09:41:33Z

src/audio/codec_adapter/codec_adapter.c

I would put this magic number in a define.
Also, maybe, it will be good to specify this limitation in commit message: "so far we support mp3 at 192khz and 24 bits".

@thanks Iulia for the comment. What I actually plan to do is to create a generic function that given a codec and its parameters it will return the maximum amount of the output data.

Something like code_adapter_get_max_bytes? I just couldn't find a good name. This work is still RFC. We won't submit the code like that.

A generic function sounds good.
Regarding the naming, I think it's ok, but I'm not good with names either :)

Fwiw the 9212 should also be a topology 2.0 attribute (as this can also be enforced in any MP3 topology)

@lgirdwood this could be an option. It looks like at least for mp3 the number of samples resulted after decoding one frame is always the same 1152 (9217 = 1152 samples * 2 channels * 4 bytes / sample + 1). If this holds true for all the decoding algorithms then getting this via topology could be an option.

I think it would be a grave mistake to use a single period.

if the scheduling is constrained by the output size, then it may happen that you have to decode multiple frames in the allocated time. In our past integration, the decoder was part of a different pipeline and scheduled with a lower priority, so that there were no peaks in the MCPS usage. I really think you have to decouple the codec_adapter from the dai scheduling....

@plbossart Definitely! That was my next step. I try to do it gradually and also have something that's working for our releases. Customers want to look at the DSP even if it isn't the final code.
I will try to see how we satisfy both requirements. In time! :)

the only thing that's definitely out of place right now is the m4 topology.

For the rest of the code:

existing DAP support (based on PCM frames, should still work)

no other functionality is impacted.

Next steps:
(1) decouple codec adapter from DAI scheduling. In fact here there is no clear line where should I do the decoupling.

Because codec_adapter has an output buffer that holds the decoded samples and DMA needs to copy based on DAI pace
each period "period_bytes" of data.
So, each DAI period I need to call codec_adapter_copy() to push data from codec_adapter local buffer to DMA buffer.

Also, the Host need to push data to codec_adapter based on its own pace. So, Host should copy one "compress period" of data each time the DAI has rendered one mp3 frame.

So, in simple terms:

Host should copy one mp3 compressed frame each 0.026 sec (compress period will be 26ms).

DAI should copy SAMPLE_RATE/1000 sampes each 1ms (compress period will be 1ms).

Also, we noticed that we could process around 3 to 4 MP3 frames within 1ms , so there is enough room for our simple sceario to work.

I think you should really have 3 pipelines
a) dai with mixer
b) PCM for notifications/beeps that may occur at any time
c) compressed. That pipe may run at a lower priority or larger granularity than the DAI period. The last thing you want it to try and complete one decode at a 26x speed, you have to spread the work across multiple dai periods otherwise you will underrun when your system is loaded.

IIRC in all implementations, the DAI typically works with O(ms) buffers, and the compressed pipeline can work with much larger frames. In fact if you look at newer codecs they will generate 4k or 8k samples in one frame. So you need some sort of ring buffer to adjust between codec and dai.

And I forget that you need an SRC on your compressed pipeline if you want to support all frequencies.

Nice suggestion. I wonder if compress and PCM can be supported on the sample sound card device on the Linux side.

src/audio/codec_adapter/codec_adapter.c

tools/topology/sof-imx8-processing-wm8960.m4

With compress audio the number of output bytes is usually greater then the number of the input bytes. Because of this, local buffer might have "valid" bytes even if the current process operation didn't produce anything. So, in this period we did not produce anything but we can still copy some bytes from local_buffer to sink buffer. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

…sing We need to make sure that the local buffer has enough room to hold the output of the processing. This means that for each type of decoding algorithm we need to know the maximum possible output size and skip processing if local buffer size is less than that. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

Processing loop inside codec_adapter copy() function will take the input buffer and try to process all the data available. We need to 'consume' the source bytes and 'produce' the bytes to local_buffer after each iteration not just at the end. Failing to do so, will put wrong data for processing function starting with second iteration. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

Add topology to support compress playback for MP3 on i.MX8/i.MX8X with wm8960 codec. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

lgirdwood

I assume we will support full passthrough mode for this work so this can be validated by CI without binary codecs ?

lgirdwood · 2021-03-26T13:09:40Z

src/audio/codec_adapter/codec_adapter.c

Fwiw the 9212 should also be a topology 2.0 attribute (as this can also be enforced in any MP3 topology)

dbaluta · 2021-03-26T16:26:35Z

@lgirdwood yes we will support pass through. This is one of our tiny hopes to test the codec_adapter publicly without private binary blobs.

We need to clarify some things about pass through:

currently there is a simple pass through family of codecs [1]. This is already merged and I will take care that current patches do not break it.
there is a proposal to add a pass through codec type inside Cadence family of codecs [2]

@johnylin76 is there any update for [2]?

[1] https://github.com/thesofproject/sof/blob/master/src/audio/codec_adapter/codec/passthrough.c
[2] #3766

plbossart · 2021-03-26T16:31:28Z

src/audio/codec_adapter/codec_adapter.c


+		/* TODO: remove magic number and use possible maximum output per
+		 * decoding algorithm type
+		 * 9216 is maximum frame length bytes for mp3 at 192Khz, 24bits,


that makes no sense. mp3 never supported 192kHz and perceptual coders don't care about bit depth.

I don't recall what the max value was but this was something at 320 kbits/s

@plbossart It is clear that I still need to read upon mp3 stuff and all. I'm still confused about all the version / etc.

I started with a sample mp3 file:

root@imx8qxpc0mek:~# file samples/lb.mp3 samples/lb.mp3: MPEG ADTS, layer III, v1, 192 kbps, 48 kHz, Stereo

From the internet I have this:

// Bitrates - use [version][layer][bitrate] const uint16_t mpeg_bitrates[4][4][16] = { { // Version 2.5 { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, // Reserved { 0, 8, 16, 24, 32, 40, 48, 56, 64, 80, 96, 112, 128, 144, 160, 0 }, // Layer 3 { 0, 8, 16, 24, 32, 40, 48, 56, 64, 80, 96, 112, 128, 144, 160, 0 }, // Layer 2 { 0, 32, 48, 56, 64, 80, 96, 112, 128, 144, 160, 176, 192, 224, 256, 0 } // Layer 1 }, { // Reserved { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, // Invalid { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, // Invalid { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, // Invalid { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 } // Invalid }, { // Version 2 { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, // Reserved { 0, 8, 16, 24, 32, 40, 48, 56, 64, 80, 96, 112, 128, 144, 160, 0 }, // Layer 3 { 0, 8, 16, 24, 32, 40, 48, 56, 64, 80, 96, 112, 128, 144, 160, 0 }, // Layer 2 { 0, 32, 48, 56, 64, 80, 96, 112, 128, 144, 160, 176, 192, 224, 256, 0 } // Layer 1 }, { // Version 1 { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, // Reserved { 0, 32, 40, 48, 56, 64, 80, 96, 112, 128, 160, 192, 224, 256, 320, 0 }, // Layer 3 { 0, 32, 48, 56, 64, 80, 96, 112, 128, 160, 192, 224, 256, 320, 384, 0 }, // Layer 2 { 0, 32, 64, 96, 128, 160, 192, 224, 256, 288, 320, 352, 384, 416, 448, 0 }, // Layer 1 } };

If my understanding is correct MP3 is MPEG-1 Audio Layer III or MPEG-2 Audio Layer III.

So, based on the table above mp3 should also support 192kHZ. But I admit my comment there is just bad! I will remove it.

you may be confusing kbs and kHz :-)

the rates are
version 1: 48, 44.1 and 32kHz
version 2: 24, 22.05 and 16 kHz
version 2.5: 12, 11.025 and 8 kHz (this is actually not a standard but an extension by FhG).

https://en.wikipedia.org/wiki/MP3 has most of the needed information.

Yes, you are right the comment should be read 192kbps.

plbossart · 2021-03-26T16:33:15Z

tools/topology/sof-imx8-processing-wm8960.m4

+	1000, 0, 0, SCHEDULE_TIME_DOMAIN_DMA)
+
+
+# PCM Low Latency, id 0


@plbossart copy paste of course.

plbossart · 2021-03-26T16:33:18Z

tools/topology/sof-imx8-processing-wm8960.m4

+# PCM Low Latency, id 0
+
+dnl PCM_DUPLEX_ADD(name, pcm_id, playback, capture)
+COMPR_PLAYBACK_ADD(Port0, 0, PIPELINE_PCM_1)


CompressedPort0?

Makes sense.

lgirdwood · 2021-03-28T13:47:51Z

Please use "main" as PR target.

dbaluta requested review from lbetlej, lgirdwood, mmaka1, plbossart and ranj063 as code owners March 25, 2021 19:03

dbaluta changed the title ~~RFC: Add support for compress audio~~ RFC: Add final support for compress audio Mar 25, 2021

dbaluta added 2 commits March 25, 2021 21:06

audio: codec_adapter: Updated processed bytes

14ad9c2

processed field keeps the total number of processed bytes from the input buffer. We need to update this field also for the process_init operation. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

dbaluta force-pushed the compress branch 2 times, most recently from 2f800d3 to 66d0e4e Compare March 25, 2021 19:19

paulstelian97 reviewed Mar 26, 2021

View reviewed changes

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

paulstelian97 reviewed Mar 26, 2021

View reviewed changes

tools/topology/sof-imx8-processing-wm8960.m4 Outdated Show resolved Hide resolved

iuliana-prodan requested changes Mar 26, 2021

View reviewed changes

dbaluta added 4 commits March 26, 2021 13:28

topology: imx8: Add processing pipeline for i.MX8/i.MX8X

ac6c89b

Add topology to support compress playback for MP3 on i.MX8/i.MX8X with wm8960 codec. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

lgirdwood reviewed Mar 26, 2021

View reviewed changes

dbaluta force-pushed the compress branch from 66d0e4e to ac6c89b Compare March 26, 2021 16:10

plbossart reviewed Mar 26, 2021

View reviewed changes

lgirdwood closed this Mar 28, 2021

lgirdwood deleted the branch thesofproject:master March 28, 2021 13:44

		1000, 0, 0, SCHEDULE_TIME_DOMAIN_DMA)


		# PCM Low Latency, id 0

RFC: Add final support for compress audio #3959

RFC: Add final support for compress audio #3959

Uh oh!

Conversation

dbaluta commented Mar 25, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

plbossart Mar 26, 2021 • 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!

dbaluta Mar 26, 2021 • 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!

dbaluta Mar 26, 2021 • 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!

Uh oh!

Uh oh!

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dbaluta commented Mar 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

lgirdwood commented Mar 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

plbossart Mar 26, 2021 •

edited

Loading

dbaluta Mar 26, 2021 •

edited

Loading

dbaluta Mar 26, 2021 •

edited

Loading

dbaluta commented Mar 26, 2021 •

edited

Loading