CirrusLogic / tinyhal

Configurable audio HAL for Android
Apache License 2.0
32 stars 17 forks source link

Provide support the mixer_paths xml format for configuring audio routing #8

Closed MaksymPrymierov closed 4 years ago

MaksymPrymierov commented 4 years ago

Many AOSP projects use the mixer_paths xml format to configure audio routing. Support for this format will help use tinyhal in projects using the mixer_paths xml format.

Add parser for the mixer_paths files using the audioroute library. Add the element for parsing, which takes the name of the mixer_paths.xml file. Update audio.example.xml with an example of using to configure audio routing.

rfvirgil commented 4 years ago

I don't understand (a) how this can be useful or (b) why it is necessary.

This change only invokes the initialization section from mixer_paths.xml, so the rest of the file (the actual use-cases) is ignored. This means any realistic mixer_paths.xml would still have to be converted to tinyhal format to run the use-cases.

Also this gives you all the overhead of audio_route that tinyhal avoids. When you initialize audio_route it will start another instance of libtinyalsa, which walks all the controls to build a list of controls, and then audio_route walks all those controls to build a list of initial values. This can take a very long time on systems with large numbers of ALSA controls (some hardware has >3000 ALSA controls and takes several seconds to walk).

Why use two different UCMs anyway?

If the critical requirement is to use a mixer_paths.xml, then it is appropriate to use audio_route.

If the critical requirement is to make use of tinyhal features, then the xml should be converted to tinyhal format. The conversion of the init section is easy because the format is almost identical. Only binary controls need a bit more work because mixer_paths.xml can only set one byte per but tinyhal defines the entire array values in one . But binary controls are not often used.

audio_route/mixer_paths.xml works in a fundamentally different way to audio_route so I don't think it would ever make sense for tinyhal to act as a front-end to audio_route.

It wasn't the intention for tinyhal to be a drop-in replacement for AOSP projects that were designed to use audio_route. The intention is as an alternative UCM for new projects that want an alternative to audio_route/mixer_paths.xml.

MaksymPrymierov commented 4 years ago

@rfvirgil Thank you for your reply, this solution appeared after I tried to convert mixer_paths.xml to the tinyhal format and ran into the problem of integer arrays. Integer arrays are not supported. I suggest implementing integer array support instead of mixer_paths.xml support.

rfvirgil commented 4 years ago

Integer arrays are not supported.

I think it should work if you do it the same way as mixer_paths.xml, with a new entry for each index position, like this:

<ctl name="VOLUMES ARRAY" index="0" val="42" />
<ctl name="VOLUMES ARRAY" index="1" val="37" />
<ctl name="VOLUMES ARRAY" index="2" val="94" />

This was definitely intended to be supported so that tinyhal could handle stereo volume and switch controls where you want to give a different value to left and right. It relies on the tinyalsa mixer_ctl_set_value() correctly doing a read-modify-write of the control.

Obviously that is inefficient of you need to set a very large integer array. I think the assumption was that integer and bool arrays usually are only seen for stereo controls, so only two array entries. Therefore no need to optimise it to support setting the entire array in a single . Unlike byte arrays which could be large blocks of coefficient data.

MaksymPrymierov commented 4 years ago

@rfvirgil Thank you, I understand what you mean. With this, I think I can close this pull request.