Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix hdmi audio: switch audio HAL module from tinyalsa to alsa #1

Merged
merged 3 commits into from Mar 6, 2023
Merged

fix hdmi audio: switch audio HAL module from tinyalsa to alsa #1

merged 3 commits into from Mar 6, 2023

Conversation

nename0
Copy link
Contributor

@nename0 nename0 commented Feb 28, 2023

Hi @KonstaT

I read what you said about, why HDMI audio was not properly working, so I started digging a bit.
The current alsa_loop solution uses alsa-lib to do the IEC958 subframe conversion (via pcm_iec958.c).

However, the audio HAL module in audio_hw.c already uses tinyalsa, so I though why not do it directly.
The reason why android seems to use tinyalsa instead of full alsa seems to be GPL license stuff and maybe size(?).
But thanks to raspberry-vanilla/android_external_alsa-lib we already have alsa-lib in our build.

The only big task was to port audio_hw.c from tinyalsa to alsa.
I had to revert the audio_hw.c from soong back to a makefile, because the alsa-lib is also still makefile and a soong module cannot depend on a makefile one.

Of course a few things are still hacked together like the fixed channels and rates. If someone with more time and knowledge comes along he/she might be able to add surround sound and passthrough (if raspberry drivers even support that, but LibreELEC seems to be able to) like here.

Also on your original vendor image /vendor/lib/libasound.so does not exits cause all alsa_utils are 64 bit, but now it is needed and thus created.
So to test this on an existing vendor image you need to update both /vendor/lib/libasound.so and /vendor/lib/hw/audio.primary.rpi.so
And also remove the asla_loop from /vendor/etc/init/hw/init.rpi4.rc.

TODO: remove the alsa_loop from init.rpi4.rc otherwise multiple processes try to open the alsa device and fail...

@KonstaT
Copy link
Member

KonstaT commented Mar 1, 2023

Thank you for this PR. :) This is actually one of the few options I've considered. This looks fine for most parts.

Few quick thoughts/ideas:

  • I'd probably prefer to have this built as separate HAL (e.g. audio.primary.rpi_hdmi or whatever). There would be one that uses tinyalsa for 3.5mm jack/audio DACs and another one that uses alsa-lib for HDMI audio. There's a property (ro.hardware.audio.primary) that can be used to set which of the two HALs is being used.
  • I'd also prefer to look into if the alsa makefiles can be converted to blueprint instead. There's some generated files there so it might not be that simple, though. I'd rather not have any legacy .mk makefiles in the device tree if not mandatory.
  • Loopback device should be removed from the kernel as well once it's no longer used so the HAL can't depend on that device number either.

I have some other things going on so it might take a while until get around to all this but I'm sure I will eventually. Thanks again for your contribution!

@nename0
Copy link
Contributor Author

nename0 commented Mar 1, 2023

I'd probably prefer to have this built as separate HAL

I'll look into that if I find time in the next days. Thanks for the pointer to ro.hardware.audio.primary. However if you change that property (e.g. via your (non-public?) settings app) you might have to reboot or kill the audio binder to make it load the other library, right?.

There's some generated files there so it might not be that simple, though.

Yes that's the part that scared me off.

In alsa-lib the asoundlib.h is generated: https://github.com/raspberry-vanilla/android_external_alsa-lib/blob/c793569fc8f598e32bcafeddbb44edb411b486f9/android/Android.mk#L39-L48

No idea how one would port that so soong. Soong does support generated headers somehow: see

Loopback device should be removed from the kernel

Either you do that yourself when merging this PR or you need give me a pointer to where that happens. Sometimes it's really hard to find all these inter-dependencies...

@KonstaT
Copy link
Member

KonstaT commented Mar 1, 2023

I'll look into that if I find time in the next days. Thanks for the pointer to ro.hardware.audio.primary. However if you change that property (e.g. via your (non-public?) settings app) you might have to reboot or kill the audio binder to make it load the other library, right?.

Yeah, it will require a reboot. I don't consider that too much of an issue as I don't think people generally need to switch between audio devices that often.

In alsa-lib the asoundlib.h is generated

Also probably not too much of an issue to generate it once and just stick the generated header somewhere in the source. I doubt I'll be updating the alsa-lib anytime soon so it won't be changing either.

Either you do that yourself when merging this PR or you need give me a pointer to where that happens. Sometimes it's really hard to find all these inter-dependencies...

https://github.com/raspberry-vanilla/android_kernel_brcm_rpi4/blob/android-13.0/arch/arm64/configs/lineageos_rpi4_defconfig#L4030 to # CONFIG_SND_ALOOP is not set. Not a big deal in any case.

@nename0
Copy link
Contributor Author

nename0 commented Mar 3, 2023

Also probably not too much of an issue to generate it once and just stick the generated header somewhere in the source. I doubt I'll be updating the alsa-lib anytime soon so it won't be changing either.

I tried using a cc_genrule first but after I found out that most examples in android actually generate zip files cause it seems like genrules can only output one single file, I though fuck it and just commited the headers in, see the PR on the alsa-lib repo raspberry-vanilla/android_external_alsa-lib#1.
Also it doesn't help that soong needs like 12 min to regenerate if just one blueprint file changed...

I'd probably prefer to have this built as separate HAL

Implemented that now. Here are the properties (like in the wiki) you would need to set (in your settings app) when switching audio outputs:

audio jack:

ro.hardware.audio.primary=rpi
persist.audio.pcm.card=0

HDMI0:

ro.hardware.audio.primary=rpi_hdmi
persist.audio.hdmi.device=vc4hdmi0

HDMI1:

ro.hardware.audio.primary=rpi_hdmi
persist.audio.hdmi.device=vc4hdmi1

DAC:

ro.hardware.audio.primary=rpi
persist.audio.pcm.card=4 # 3 if loopback removed

To set the ro properties I used:

source /vendor/bin/rpi4-utils.sh
mount_rw /vendor
switch_property ro.hardware.audio.primary rpi rpi_hdmi /vendor/build.prop

As you said requires a reboot afterwards...

Also you still need to toggle that CONFIG_SND_ALOOP in the kernel repo ;-)

@KonstaT
Copy link
Member

KonstaT commented Mar 4, 2023

Thank you. :)

I updated the PR with few of my nitpicks:

  • moved the HDMI HAL back to the same audio directory
  • minor whitespace/tabs cleanups
  • properties in alphabetical order and added default hdmi prop

I tested this and this seems to work well. If this still looks fine for you, I'll merge this sometime next week.

Still some TODOs but I can manage this:

  • disable CONFIG_SND_ALOOP in kernel as no longer used
  • look into converting makefile in alsa-utils to blueprint as well or drop the alsa-utils (alsa_amixer, alsa_aplay, alsa_arecord, alsa_loop) altogether as no longer needed (could be still useful for testing/development)

@nename0
Copy link
Contributor Author

nename0 commented Mar 4, 2023

Just tested it - LGTM

@KonstaT KonstaT merged commit cac7486 into raspberry-vanilla:android-13.0 Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants