[BUG]DEMUX comp not work normally with channel 4 to 2 with format conversion

Describe the bug
The DEMUX should copy the data from source to sink without any error, but now with DEMXU comp, the copy out data is broken.

To Reproduce
Use demux tplg sof-apl-demux-pcm512x.tplg (need #2524 ) to replace the sof-apl-pcm512x.tplg on UP2 PCM512X

aplay -Dhw:0,0 -c 2 -f s32_le -r 48000 32le-2ch-sin-100-200hz.wav -vvv
arecord -Dhw:0,4 -f dat 2ch_0312_04.wav -vvv

Reproduction Rate
100%

Expected behavior
The record wav should be same as input wav

Impact
This will be a stopper for echo reference and smart amplifier

Environment

Branch name and commit hash of the 2 repositories: sof (firmware/topology) and linux (kernel driver).
- Kernel: sof-dev d05959d
- SOF: master 8e5ab9c (need topology: fix apl-demux-pcm512x HDMI dai id #2524 )
Name of the topology file
- Topology: sof-apl-demux-pcm512x.tplg rename to sof-apl-pcm512x.tplg
Name of the platform(s) on which the bug is observed.
- Platform: APL

Screenshots or console output

@ClarexZhou

Also test with #2525 on TGL
wav files attached
2ch_0312_04.wav test on APL with DEMUX and 32le-2ch-sin-100-200hz.wav
4ch_0312_01.wav test on TGL with DEMUX and 32le-8ch-sin-100-800hz.wav
8ch_0312_02.wav test on TGL with DEMUX and 32le-8ch-sin-100-800hz.wav
8ch_0312_03.wav test on TGL with nocodec loopback and 32le-8ch-sin-100-800hz.wav
demux_test.zip

@ClarexZhou Could you try to use the sof-apl-demux-pcm512x.tplg on an UP2 to check if the output from codec also have noise?

@xiulipan

@xiulipan Applied #2524 and rename sof-apl-demux-pcm512x.tplg to sof-apl-pcm512x.tplg on platform APL-UP2-PCM512X. Then tried below scenarios:

Aplay via HW:0,0(headset) is no output
Aplay via HW:0,1(HDMI) can heard the output and no noise.
Seems different with yours, did i do something wrong here?
dmesg.log
amixer.txt

@dengyangchao

@dengyangchao Interesting. I only care about the 0,0 headset codec output. Can you check amixer setting with normal UP2 PCM512X.

@juimonen

I would like to see demux/mux fixed to remove the binary blobs. this is related to poor understaning and usage complexity. @juimonen fyi.

@lgirdwood

@lgirdwood ok, it is now very clear what you want, thanks. I've been little bit hesitant to work on the demux on fw side (as it is not done by me), but I'll see what I can do. I'll make some RFC PR suggestions about the structs and topology.

@juimonen

@juimonen thanks - if we look at the config structures used by the binary kcontrol (Ive added comments inline)

struct mux_stream_data {
	uint32_t pipeline_id;
	uint8_t num_channels;
	uint8_t mask[PLATFORM_MAX_CHANNELS];

	uint8_t reserved[(20 - PLATFORM_MAX_CHANNELS - 1) % 4]; // padding to ensure proper alignment of following instances
};

struct sof_mux_config {
	uint16_t frame_format;   /* not sure why needed - come from hw_params */
	uint16_t num_channels; /* diito, from hw_params */
	uint16_t num_streams; /* unclear if this is source or sink or total */

	uint16_t reserved; // padding to ensure proper alignment

	struct mux_stream_data streams[];
};

They need documented for sure alongside the mux code that sets the data.

We either need to represent a mux (N streams -> 1 stream) or a demux (1 stream -> N streams).

For every MUX and DEMUX output channel we create an enum kcontrol that contains all possible input stream channel choices. i.e. pipeline1 left, pipeline 1 right, pipeline 2 left, pipeline 2 right and so on
The enum will only permit 1:1 mapping between. i.e. we cannot mix channels
Channel duplication is allowed i.e. we can route input pipeline1 left to more than 1 output channel.
topology should only create the minimum enums required based on actual channels/streams and not pre-defined max channels and streams.
MUX will create a small number of large enums whilst DEMUX will create a larger number of small enums.

@lgirdwood

@lgirdwood Do you mean that we need a refine for MUX/DEMUX? I agree that the binary blob is very hard to understand and config. But how long would this take?

@juimonen Do you have any idea why we got this issue with following binary blob?

sof/tools/topology/m4/demux_coef_default.m4

Lines 1 to 11 in 366aa67

    
           CONTROLBYTES_PRIV(DEMUX_priv, 
        
           `       bytes "0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00,' 
        
           `       0x28,0x00,0x00,0x00,0x00,0x60,0x00,0x03,' 
        
           `       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,' 
        
           `       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,' 
        
           `       0x02,0x00,0x02,0x00,0x02,0x00,0x00,0x00,' 
        
           `       0x01,0x00,0x00,0x00,0x02,0x01,0x02,0x04,' 
        
           `       0x08,0x10,0x20,0x40,0x80,0x00,0x00,0x00,' 
        
           `       0x05,0x00,0x00,0x00,0x02,0x01,0x02,0x04,' 
        
           `       0x08,0x10,0x20,0x40,0x80,0x00,0x00,0x00"' 
        
           )

It should work with the sof-apl-demux-pcm512x.tplg but I think we may got some issue with the total copy function.

@xiulipan

@xiulipan demux has never worked perfectly. The first version we did was only working with branched demux pipe directly connected to scheduler (pcm) i.e. you could not add any components to branched pipeline. This was because our fw scheduling didn't support this kind of pipe.

So I only recently tried to change the interface to use tokens and faced the same situation as you, so idisp id's we're wrong and the whole topology didn't load. I did not test it further, I just wanted to get to tokens (so loading). if you want to play with the blob, check #2536, there are examples for demux with -h option.

I'll try the demux myself, but I'm little bit in "slow mode" now as I'm working from home.

@lgirdwood

@lgirdwood Do you mean that we need a refine for MUX/DEMUX? I agree that the binary blob is very hard to understand and config. But how long would this take?

Yes, we only refine the topology so that we create enum kcontrols instead of this undecipherable blob. The ABI between FW and kernel remains the same.The FW will still support the blob method, but should also be able to use the enums.

Kernel needs some helper functions to store a local blob for each mux/demux component in topology. It then allows the enum kcontrol to change the internal blobs before updating FW.
FW - may not need any update
Topology needs new macros for constructing enum text arrays and attaching to demux/mux components.

@xiulipan

@xiulipan What is the final statement? I'm not sure whether it's worth looking into or maybe your configuration is wrong?

@tlauda

@tlauda Still testing, not sure if it is some configure issue or comp issue.

and yes sorry I had my up2 dusting for a while, so no idea what he old firmware was. I was updating everything to latest and greatest.

@juimonen

@juimonen Ack, demux topology doesn't work with today's FW master. I tried going back to 1.4.2 and there it still works as expected. So something has broke.

See #2687 lets simplify.

@juimonen

@juimonen Ack, demux topology doesn't work with today's FW master. I tried going back to 1.4.2 and there it still works as expected. So something has broke.

@juimonen @kv2019i can you try the latest master as @akloniex 's #2600 was merged yesterday?

Tested it with latest sof-dev (commit:fce7f93) + master (commit: d7ad0c1), issue can still be reproduced on APL-UP2-PCM512X.
tplg: same as fw branch: rename sof-apl-demux-pcm512x.tplg to sof-apl-pcm512x.tplg

Reproduce steps:

aplay -Dhw:0,0 -c 2 -f s32_le -r 48000 32le-2ch-sin-100-200hz.wav -vvv
arecord -Dhw:0,4 -f dat 2ch_0312_04.wav -vvv

Screenshot:

wav_files_and_demsg_sof_trace.zip

@akloniex

@akloniex mind taking a look at it?

latest firmware af38b20 and latest topology sof-apl-demux-pcm512x.tplg I can get proper sound from echoref pipe!

terminal 1:
aplay test_play.wav

Terminal 2:
arecord -Dhw:0,4 -f S16_LE -c 2 -r 48000 -d 10 test.wav

So maybe someone else can also verify?

@akloniex

@akloniex any idea why we didn't use the "caps" in ipc_buffer for underrun/overrun flags and defined new flags?

struct sof_ipc_buffer {
struct sof_ipc_comp comp;
uint32_t size; /< buffer size in bytes */
uint32_t caps; /< SOF_MEM_CAPS_ */
uint32_t flags; /**< SOF_BUF_ flags defined above */

I think it is also 32bit bit value which is using only couple of bits for memory related flags...

Move to new #2959 with new test and new issue.

Navigation Menu

Repository navigation

[BUG]DEMUX comp not work normally with channel 4 to 2 with format conversion #2533

25 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	CONTROLBYTES_PRIV(DEMUX_priv,
	` bytes "0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00,'
	` 0x28,0x00,0x00,0x00,0x00,0x60,0x00,0x03,'
	` 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
	` 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
	` 0x02,0x00,0x02,0x00,0x02,0x00,0x00,0x00,'
	` 0x01,0x00,0x00,0x00,0x02,0x01,0x02,0x04,'
	` 0x08,0x10,0x20,0x40,0x80,0x00,0x00,0x00,'
	` 0x05,0x00,0x00,0x00,0x02,0x01,0x02,0x04,'
	` 0x08,0x10,0x20,0x40,0x80,0x00,0x00,0x00"'
	)

[BUG]DEMUX comp not work normally with channel 4 to 2 with format conversion #2533

Description

Activity

xiulipan commented on Mar 12, 2020

dengyangchao commented on Mar 12, 2020

xiulipan commented on Mar 12, 2020

lgirdwood commented on Mar 12, 2020

juimonen commented on Mar 12, 2020

lgirdwood commented on Mar 12, 2020

xiulipan commented on Mar 13, 2020

juimonen commented on Mar 13, 2020

lgirdwood commented on Mar 13, 2020

tlauda commented on Mar 16, 2020

xiulipan commented on Mar 17, 2020

25 remaining items

juimonen commented on Apr 1, 2020

kv2019i commented on Apr 1, 2020

lgirdwood commented on Apr 1, 2020

keyonjie commented on Apr 3, 2020

YvonneYang2 commented on Apr 3, 2020

slawblauciak commented on Apr 28, 2020

juimonen commented on Apr 29, 2020

juimonen commented on Apr 30, 2020

xiulipan commented on May 20, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions