Support multiple compress stream types with same topology

[dbaluta]

With this changes we can play AAC/MP3 streams with the same topology. This brings the advantage that we do not need to reboot/or change topology in order to playd diffferent stream types.

[dbaluta]

@lgirdwood no need for any kernel updates.

[lgirdwood]

@dbaluta do you have an example of the command line that would be used in this use case ? I'm guessing it's just

cplay file.mp3
cplay file.aac

i.e. there is no kcontrol to switch between aac and mp3 ?

[dbaluta]

Quoting @plbossart

Is this saying that the parameters in topology need to be formatted so as to match the format expected by a specific implementation? The whole point was to try and push this sort of knowledge in the firmware, userspace and kernel shouldn't depend on what the library requires.

Yes, unfortunately this was the initial assumption done when we introduced topology setup bytes.

# Codec Adapter setup config control bytes (little endian)
#  : bytes "abi_header, ca_config, [codec_param0, codec_param1...]"
#  - 32 bytes abi_header: you could get by command "sof-ctl -t 0 -g <payload_size> -b"
#    - [0:3]: magic number 0x00464f53
#    - [4:7]: type 0
#    - [8:11]: payload size in bytes (not including abi header bytes)
#    - [12:15]: abi 3.1.0
#    - [16:31]: reserved 0s
#  - 20 bytes ca_config: codec adapter setup config parameters, for more details please refer
#                        struct ca_config under audio/codec_adapter/codec/generic.h
#    - [0]: API ID, e.g. 0x01
#    - [1:3]: codec ID, e.g. 0xd03311
#    - [4:7]: reserved 0s
#    - [8:11]: sample rate, e.g. 48000
#    - [12:15]: sample width in bits, e.g. 32
#    - [16:19]: channels, e.g. 2
# - (optional) 12+ bytes codec_param: codec TLV parameters container, for more details please refer
#                                     struct codec_param under audio/codec_adapter/codec/generic.h
#    - [0:3]: param ID
#    - [4:7]: size in bytes (ID + size + data)
#    - [8:n-1]: data[], the param data

I think your idea, to push the inner details in FW , kernel or userspace is good at first glance, but because we have dozens of parameters would be very hard to hide this from topology right now.

What we can really do in the future is to set most of the parameters dynamically at runtime via an ioctl. For now, I will try to do as little damage as possible to existing implementation and move to dynamic approach in a next patch.

Actually, just right now we need a way to specify PCM bit width size from topology. This information is nowhere to be found in compress API parameters structs (e.g snd_codec) and we don't have any way to pass it at runtime.

So, it needs to stay in topology. But to make things worse, the actual param_id for PCM bit width size is different from codec to codec.

So, if we want a topology to support multiple codecs we need to have multiple entries in the topology setup bytes.

[plbossart]

@dbaluta the premise of the ioctl was to standardize on common parameters. If we abuse this ioctl and make it dependent on the fimware implementation, it's a very large deviation from the initial idea.

Also I don't get why the codec PCM width cannot be adjusted by the module interface itself. We shouldn't manage this at the topology level, this is an implementation detail that needs to be accounted for by the module interface proper.

[lgirdwood]

@dbaluta the premise of the ioctl was to standardize on common parameters. If we abuse this ioctl and make it dependent on the fimware implementation, it's a very large deviation from the initial idea.

Also I don't get why the codec PCM width cannot be adjusted by the module interface itself. We shouldn't manage this at the topology level, this is an implementation detail that needs to be accounted for by the module interface proper.

@plbossart are you suggesting some wrapping of the ioctl in alsa-lib with an alsa API ?? i.e. to force the standardization and help application code with config ?

[dbaluta]

@dbaluta the premise of the ioctl was to standardize on common parameters. If we abuse this ioctl and make it dependent on the fimware implementation, it's a very large deviation from the initial idea.

Also I don't get why the codec PCM width cannot be adjusted by the module interface itself. We shouldn't manage this at the topology level, this is an implementation detail that needs to be accounted for by the module interface proper.

I wasn't suggesting to abuse the ioctl interface.

My intention is like this:

1. • use ioctl parameters for everything it already supports. Don't add any new field.
2. • use topology for any non-standard parameter OR not-supported-by ioctl.

Module interface cannot make decision by itself. e.g use 32bit width PCM or 24bit width PCM. It needs someone externally to instruct it.

@plbossart @lgirdwood @ranj063 do you have any 30-45 minutes next week to setup a call?

[lgirdwood]

@plbossart @lgirdwood @ranj063 do you have any 30-45 minutes next week to setup a call?

Good for me Wednesday onwards, but @ranj063 is out.

[plbossart]

Module interface cannot make decision by itself. e.g use 32bit width PCM or 24bit width PCM. It needs someone externally to instruct it.

Why not? don't we have information on component sink and source buffer that could be used to infer what the desired bit depth is?

@plbossart @lgirdwood @ranj063 do you have any 30-45 minutes next week to setup a call?

Fine with me. it wouldn't hurt to review all the known issues with the compressed support, it seems we have a few still.

[dbaluta]

@plbossart as discussed yesterday this PR is as good as it gets.

I find @ujfalusi proposals to group parameter by codec id, e.g

[codec_id1, (param_id, value), (param_id, value)], [codec_id2, (param_id, value), param_id, value)]

very interesting, so I will hold this PR a little bit more to get a chance to discuss with Ranjani. @ranj063

The drawback for @ujfalusi proposal is that we won't be able to keep backward compatibility between older SOF version and new topology. I'm not sure though we really want that.

[lgirdwood]

@plbossart @dbaluta sorry, I missed the call - what is the update/plan here ?

[gkbldcig]

Can one of the admins verify this patch?

[lgirdwood]

@dbaluta any update ?

[dbaluta]

will prepare a simpler PR for this.