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

I2S support for using a Hifiberry audio DAC (16-bit) #17

Closed
dcoredump opened this issue Feb 20, 2022 · 7 comments
Closed

I2S support for using a Hifiberry audio DAC (16-bit) #17

dcoredump opened this issue Feb 20, 2022 · 7 comments
Labels
enhancement New feature or request

Comments

@dcoredump
Copy link
Contributor

Using I2S sound boards would be very nice, because real 16bit/44.1 KHz audio sounds much better than 12(?)bit PWM.

@probonopd
Copy link
Owner

Indeed.

According to the README, Circle supports

I2S sound device using external hardware (tested with PCM5102A and PCM5122 DACs)

https://github.com/rsta2/circle/blob/75bc0890490f4c38f8cdf0eee83a2bad40942566/sample/34-sounddevices/kernel.cpp#L129-L137

shows how to initialize it. Unfortunately I don't have any PCM5102A or PCM5122 devices to test with. I do have a WM8960 I2S hat, though.

@rsta2
Copy link
Contributor

rsta2 commented Feb 21, 2022

Here comes a patch for the I2S (tested with a PiFi DAC+ v2.0 HAT) and HDMI (also tested) modes. You have to add one of the following lines to the file circle-stdlib/libs/circle/Config.mk after entering configure to generate a respective version of MiniDexed:

DEFINE += -DUSE_I2S
or
DEFINE += -DUSE_HDMI

In the .zip file there is a configure script, which can be used to call circle-stdlib/configure and automatically add these lines, when one of the following options is given in the first place:

--i2s
or
--hdmi

All other options are passed along to circle-stdlib/configure. This script is only a suggestion and has to go to MiniDexed project root.

There is a good chance, that the Hifiberry DAC works too. Unfortunately the WM8960 is not supported in Circle.

i2s_hdmi_patch.zip

@probonopd probonopd added the enhancement New feature or request label Feb 21, 2022
@probonopd
Copy link
Owner

Hello @rsta2, thanks for this patch!

You have to add one of the following lines to the file circle-stdlib/libs/circle/Config.mk after entering configure to generate a respective version of MiniDexed

Is it also possible to enable both and have no bad side effects when the respective hardware is not available? Ideally, we would have one binary that fits all supported hardware configurations. Could the output be changed from the headphone jack to one of the other outputs by using a txt file on the microSD card?

@rsta2
Copy link
Contributor

rsta2 commented Feb 21, 2022

The hardware cannot detect, if there is something connected to the headphone jack (PWM mode). Some I2S HATs could be detected, but not all. This is also not implemented in the Circle drivers. I think, to enable more then one interface is not the way to go. The recommended way, to handle different audio interfaces and to select the one to use, is implemented in the Circle sample/34-sounddevices. You have to create a file cmdline.txt on the µSD card with this contents to select an interface:

sounddev=sndpwm
or
sounddev=sndi2s
or
sounddev=sndhdmi

There is also the option to use VCHIQ sound, which is done by the Raspberry Pi firmware via PWM or HDMI, but I wouldn't use this, because it is difficult to handle (needs tasks, difficult timing).

Unfortunately this method to select the audio interface using cmdline.txt, as it is implemented in the sample, requires a different approach for the communication with the audio drivers, different from that, which is currently implemented in the Synth_Dexed project. You must call CSoundBaseDevice::Write() and do not have to override CSoundBaseDevice::GetChunk() as before. So this would require some modifications to the existing and already working code.

@probonopd
Copy link
Owner

cmdline.txt seems like the way to go to me! How much rework would it be to get it going?

What would the advantage of VCHIQ sound be? Better quality? "difficult to handle (needs tasks, difficult timing)" sounds to me like we should probably not use it.

@rsta2
Copy link
Contributor

rsta2 commented Feb 22, 2022

Yes, I would also prefer the cmdline.txt solution. I found an implementation now, which works without big modifications and without changing from GetChunk() to Write(). The patch is attached. It is based on the current master of both projects, so the previous patch above is obsolete. With these modifications you need to build only one kernel image for each target and can select the sound interface using sounddev=sndpwm | sndi2s | sndhdmi in the file cmdline.txt. The default is PWM, when cmdline.txt is missing or does not contain a valid sounddev= option.

sound_devices_patch.zip

Using the VCHIQ sound may have the following advantages:

  • Better sound quality over the headphone jack (PWM). I think, they doing additional processing in the firmware there.

  • The HDMI1 connector can (probably) be used on the Raspberry Pi 4/400. This is not supported by the Circle HDMI hardware driver (HDMI0 only).

  • The firmware can detect, if a HDMI display with audio support is connected. Hence in this case an automatic selection between PWM and HDMI would be possible.

On the other hand using VCHIQ sound generates some problems:

  • Adds additional overhead for using tasks.

  • Adds additional latency, because all sound data has to be transferred to the firmware first.

  • My impression of some Circle projects, that I had some insight in, was that the authors often had problems with getting the timing right (to eliminate drops in the audio signal).

I would not use VCHIQ sound, at least for the moment.

probonopd added a commit to probonopd/Synth_Dexed that referenced this issue Feb 22, 2022
Using sounddev=sndpwm | sndi2s | sndhdmi in the file cmdline.txt
probonopd/MiniDexed#17 (comment)
probonopd added a commit that referenced this issue Feb 22, 2022
Using sounddev=sndpwm | sndi2s | sndhdmi in the file cmdline.txt
#17 (comment)
@probonopd
Copy link
Owner

Thanks for the patch and for the explanation @rsta2.
Agree about VCHIQ, seems not worth the effort at this point. Who wants quality will probably use a i2c DAC anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants