Dne 30. 01. 24 v 13:30 Jaroslav Kysela napsal(a):
On 29. 01. 24 18:55, Pavel Hofman wrote:
Dne 29. 01. 24 v 18:27 Jaroslav Kysela napsal(a):
On 29. 01. 24 18:06, Pavel Hofman wrote:
I was wondering if it made sense to add support for the IEC958 formats (using the iec958 plugin methods) to the plug plugin.
It may be complicated with the hdmi_mode hint though, I do not know if there is any API to recognize HDMI vs. SPDIF. Maybe a different format specifically for HDMI could have been perhaps useful but it may be too late for that.
Thank you very much for consideration.
Perhaps something trivial like this could do (header file changes not included):
=================================================================== diff --git a/src/pcm/pcm_plug.c b/src/pcm/pcm_plug.c --- a/src/pcm/pcm_plug.c (revision ffed4f342678c31bf0b9edfe184be5f3de41603a) +++ b/src/pcm/pcm_plug.c (date 1706546414549) @@ -490,6 +490,14 @@ } #endif
+int snd_pcm_plug_generic_iec958_open(snd_pcm_t **pcmp, const char *name, snd_pcm_format_t sformat, snd_pcm_t *slave, int close_slave) { + unsigned char preamble_vals[3] = { + 0x08, 0x02, 0x04 /* Z, X, Y */ + }; + int hdmi_mode; // ???? + return snd_pcm_iec958_open(pcmp, name, sformat, slave, close_slave, NULL, preamble_vals, hdmi_mode); +}
static int snd_pcm_plug_change_format(snd_pcm_t *pcm, snd_pcm_t **new, snd_pcm_plug_params_t *clt, snd_pcm_plug_params_t *slv) { snd_pcm_plug_t *plug = pcm->private_data; @@ -567,6 +575,10 @@ f = snd_pcm_adpcm_open; break; #endif + case SND_PCM_FORMAT_IEC958_SUBFRAME_LE: + case SND_PCM_FORMAT_IEC958_SUBFRAME_BE: + f = pcm_snd_general_iec958_open; + break; default: return -EINVAL; }
===================================================================
IMO preamble_vals could be left at the same default as defined in pcm_iec958.c:_snd_pcm_iec958_open().
But the correct hdmi_mode parameter is necessary. Alsa configs specify it explicitly (like vc4-hdmi.conf). But the plug plugin has no such information. Adding a parameter like that to the plug plugin would cancel the effect of this change (to support hardcoded plughw devices e.g. in java, with no support for custom PCM devices).
Maybe an environment variable could be defined for the whole application which could be used for the hdmi_mode, an ugly hack though...
Thanks a lot for any hints and suggestions.
It looks like a way to go. The hdmi_mode can be set using ALSA configuration use snd_config_search (with global config - snd_config pointer) and snd_config_get_bool functions for that. The variable may be named like 'defaults.pcm.plug.iec958.hdmi_mode' or so (see alsa.conf).
That's interesting. IIUC such parameter would globally switch all plugs instances to the hdmi_mode. Would a user-based ~/.asoundrc with such variable be applied for the hard-coded plughw:XX device? Maybe it would be enough for most use cases, eliminating the need for an app-specific environment variable.
Or maybe a prioritized sequence getenv('ALSA_PCM_PLUG_IEC958_HDMI_MODE') -> snd_config_search('defaults.pcm.plug.iec958.hdmi_mode') could be used.
We usually don't use getenv hacks for alsa-lib features. The global configuration path can be redirected using ALSA_CONFIG_DIR and per user (~/.asoundrc or $XDG_CONFIG_HOME/alsa/asoundrc) and per card configurations (/var/lib/alsa/card#.conf.d) are also loaded from the global alsa.conf file.
Perphaps, the iec958 plug code may include card number / card driver name to the configuration tree lookup - like 'defaults.pcm.plug.iec958.0.hdmi_mode' or 'defaults.pcm.plug.iec958.vc4-hdmi.hdmi_mode' . With this extension, this value can be set in global src/conf/cards/vc4-hdmi.conf for this hw.
Actually, looking at pcm_iec958.c and the commit which introduced the hdmi_mode param I am not sure the the hdmi_mode is of any value for the plug plugin.
IIUC the hdmi_mode value gets used only if the status is IEC958_AES0_NONAUDIO:
int single_stream = iec->hdmi_mode && (iec->status[0] & IEC958_AES0_NONAUDIO) && (channels == 8);
But the plug plugin would pass NULL as status_bits which in snd_pcm_iec958_open will be replaced with default_status_bits:
static const unsigned char default_status_bits[] = { IEC958_AES0_CON_EMPHASIS_NONE, IEC958_AES1_CON_ORIGINAL | IEC958_AES1_CON_PCM_CODER, 0, IEC958_AES3_CON_FS_NOTID, /* will be set in hwparams */ IEC958_AES4_CON_WORDLEN_NOTID /* will be set in hwparams */ };
Logically no IEC958_AES0_NONAUDIO bit is set in the default status bits.
IMO we can safely pass hdmi_mode=false to snd_pcm_iec958_open because using plug for non-audio stream would not make sense anyway.