On 04. 02. 24 14:59, Pavel Hofman wrote:
Dne 02. 02. 24 v 9:00 Pavel Hofman napsal(a):
Dne 30. 01. 24 v 20:22 Pavel Hofman napsal(a):
Dne 30. 01. 24 v 13:30 Jaroslav Kysela napsal(a):
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.
A patch related to the conversion could look like this, IMO.
=================================================================== 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 1706857992580) @@ -490,6 +490,20 @@ } #endif
+#ifdef BUILD_PCM_PLUGIN_IEC958 +static int snd_pcm_plug_iec958_open(snd_pcm_t **pcmp, const char *name, snd_pcm_format_t sformat, snd_pcm_t *slave, int close_slave) +{ + return snd_pcm_iec958_open(pcmp, name, sformat, slave, close_slave, + /* using default status bits defined in the iec958 plugin*/ + NULL, + /* default preamble values as used in the iec958 plugin */ + {0x08, 0x02, 0x04 /* Z, X, Y */}, + /* hdmi_mode=0 because it is applied only for IEC958_AES0_NONAUDIO which is not in the default status bits */ + 0 + ); +} +#endif
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; @@ -565,6 +579,12 @@ #ifdef BUILD_PCM_PLUGIN_ADPCM case SND_PCM_FORMAT_IMA_ADPCM: f = snd_pcm_adpcm_open; + break; +#endif +#ifdef BUILD_PCM_PLUGIN_IEC958 + case SND_PCM_FORMAT_IEC958_SUBFRAME_LE: + case SND_PCM_FORMAT_IEC958_SUBFRAME_BE: + f = snd_pcm_plug_iec958_open; break; #endif default:
===================================================================
Unfortunately I am afraid I do not understand fully the code in snd_pcm_plug_hw_refine_schange which calls method snd_pcm_plug_slave_format where the IEC958 formats should also be checked.
I am trying to understand the code logic in snd_pcm_plug_hw_refine_cchange and snd_pcm_plug_hw_refine_schange as there are no comments at all, to no avail.
Please what is the meaning of local variable f in this snippet of snd_pcm_plug_hw_refine_cchange https://github.com/alsa-project/alsa-lib/blob/2a736a0d2543f206fd2653aaae8a08... ?
for (format = 0; format <= SND_PCM_FORMAT_LAST; format++) { snd_pcm_format_t f; if (!snd_pcm_format_mask_test(format_mask, format)) continue; if (snd_pcm_format_mask_test(sformat_mask, format)) f = format; else { f = snd_pcm_plug_slave_format(format, sformat_mask); if (f == SND_PCM_FORMAT_UNKNOWN) continue; } snd_pcm_format_mask_set(&fmt_mask, format); }
The snd_pcm_plug_hw_refine_schange method sets the f to the fmt_mask in https://github.com/alsa-project/alsa-lib/blob/2a736a0d2543f206fd2653aaae8a08... , but not in https://github.com/alsa-project/alsa-lib/blob/2a736a0d2543f206fd2653aaae8a08...
Please what is the meaning/contract of the method snd_pcm_plug_slave_format https://github.com/alsa-project/alsa-lib/blob/2a736a0d2543f206fd2653aaae8a08... ? What is the relation between input format and output format?
IIUC, the existing code in snd_pcm_plug_hw_refine_cchange():
for (format = 0; format <= SND_PCM_FORMAT_LAST; format++) { snd_pcm_format_t f; if (!snd_pcm_format_mask_test(format_mask, format)) continue; if (snd_pcm_format_mask_test(sformat_mask, format)) f = format; else { f = snd_pcm_plug_slave_format(format, sformat_mask); if (f == SND_PCM_FORMAT_UNKNOWN) continue; } snd_pcm_format_mask_set(&fmt_mask, format); }
can be simplified to:
for (format = 0; format <= SND_PCM_FORMAT_LAST; format++) { if (!snd_pcm_format_mask_test(format_mask, format)) continue; if (!snd_pcm_format_mask_test(sformat_mask, format) && snd_pcm_plug_slave_format(format, sformat_mask) == SND_PCM_FORMAT_UNKNOWN) continue; snd_pcm_format_mask_set(&fmt_mask, format); }
But honestly I still do not understand what it actually does and what is the goal of snd_pcm_plug_slave_format().
Without that I cannot modify snd_pcm_plug_slave_format() correctly to incorporate support for IEC958.