[alsa-devel] bootup message "Control name ... truncated to ..." - what to fix?
Hi all,
On my i8k, I'm getting this on bootup (2.6.30-rc5):
ALSA sound/core/control.c:232: Control name 'Sigmatel Surround Phase Inversion Playback Switch' truncated to 'Sigmatel Surround Phase Inversion Playback '
I would quickly have submitted a patch for this (and similar items), problem is: - what to fix: control name or struct definition? - "Sigmatel Surround" might be a relatively important prefix, thus removing "Sigmatel " from relevant names possibly not a good idea
andi@note:/usr/src/linux-2.6.30-rc5/sound$ find . -name "*.c" -o -name "*.h"|xargs grep "Sigmatel Surround" ./pci/ac97/ac97_patch.c:AC97_SINGLE("Sigmatel Surround Phase Inversion Playback Switch", AC97_SIGMATEL_DAC2INVERT, 3, 1, 0); ./pci/ac97/ac97_patch.c: snd_ac97_rename_vol_ctl(ac97, "Headphone Playback", "Sigmatel Surround Playback"); ./pci/ca0106/ca0106_mixer.c: "Sigmatel Surround Phase Inversion Playback ", ./pci/emu10k1/emu10k1x.c: * the PCM Playback Volume, Sigmatel Surround Playback Volume and
sound/core/control.c/snd_ctl_new1(): if (ncontrol->name) { strlcpy(kctl.id.name, ncontrol->name, sizeof(kctl.id.name)); if (strcmp(ncontrol->name, kctl.id.name) != 0) snd_printk(KERN_WARNING "Control name '%s' truncated to '%s'\n", ncontrol->name, kctl.id.name); }
include/sound/asound.h: struct snd_ctl_elem_id { unsigned int numid; /* numeric identifier, zero = invalid */ snd_ctl_elem_iface_t iface; /* interface identifier */ unsigned int device; /* device/client number */ unsigned int subdevice; /* subdevice (substream) number */ unsigned char name[44]; /* ASCII name of item */ unsigned int index; /* index of item */ };
I suppose (well, DAMN sure) that name is only 44 bytes due to exactly 64-byte cacheline-compatible struct alignment status.
Note that status quo is pretty bad, since in another file already: ./pci/ca0106/ca0106_mixer.c: "Sigmatel Surround Phase Inversion Playback ", .
IOW we already violate official mixer control name specs (by omitting "Switch" - see ControlNames.txt) simply in order to obey the 44-byte limit.
Given that snd_ctl_elem_id 64 byte is a rather hard and arguably important limit, I'd suggest changing the names, but how?
Unfortunately Sigmatel Surround PhaseInvert Playback Switch doesn't fit the bill either, since it's one char too long. Thus it will probably be Surround Phase Inversion Playback Switch (and thus changing "Sigmatel Surround" at all affected controls)
Thanks,
Andreas Mohr
At Thu, 21 May 2009 20:25:53 +0200, Andreas Mohr wrote:
Hi all,
On my i8k, I'm getting this on bootup (2.6.30-rc5):
ALSA sound/core/control.c:232: Control name 'Sigmatel Surround Phase Inversion Playback Switch' truncated to 'Sigmatel Surround Phase Inversion Playback '
I would quickly have submitted a patch for this (and similar items), problem is:
- what to fix: control name or struct definition?
- "Sigmatel Surround" might be a relatively important prefix, thus removing "Sigmatel " from relevant names possibly not a good idea
The word "Sigmatel" is superfluous, IMO. Just removing it should suffice...
(snip)
struct snd_ctl_elem_id { unsigned int numid; /* numeric identifier, zero = invalid */ snd_ctl_elem_iface_t iface; /* interface identifier */ unsigned int device; /* device/client number */ unsigned int subdevice; /* subdevice (substream) number */ unsigned char name[44]; /* ASCII name of item */ unsigned int index; /* index of item */ };
I suppose (well, DAMN sure) that name is only 44 bytes due to exactly 64-byte cacheline-compatible struct alignment status.
Not due to cacheline. It's just an initial definition, supposed to be large enough (but not obviously for some stupid case like this).
Given that snd_ctl_elem_id 64 byte is a rather hard and arguably important limit, I'd suggest changing the names, but how?
Unfortunately Sigmatel Surround PhaseInvert Playback Switch doesn't fit the bill either, since it's one char too long. Thus it will probably be Surround Phase Inversion Playback Switch (and thus changing "Sigmatel Surround" at all affected controls)
"Sigmatel Surround" is used just because "Surround" already exists. No really need to sync with the phase inversion switch, IMO.
The overall "Sigmatel" there looks really weird. But, I don't want to screw up things at this stage just for "looks ugly"...
thanks,
Takashi
Hi,
ChangeLog:
Avoid ALSA sound/core/control.c:232: Control name 'Sigmatel Surround Phase Inversion Playback Switch' truncated to 'Sigmatel Surround Phase Inversion Playback ' bootup message by omitting weird Sigmatel prefix in this case; also fix up the related ca0106 mixer control removal part by using identical naming there.
Patch runtime-tested on -rc6, then diffed, run through checkpatch.pl and readjusted accordingly.
I guess that this is more or less what one should have done after your very fast reply...
BTW, the "3D Control Sigmatel - Depth" vs. "3D Control - Switch" is even weirder naming, methinks.
Thanks!!
Signed-off-by: Andreas Mohr andi@lisas.de
diff -uprN linux-2.6.30-rc6.orig/sound/pci/ac97/ac97_patch.c linux-2.6.30-rc6.patched/sound/pci/ac97/ac97_patch.c --- linux-2.6.30-rc6.orig/sound/pci/ac97/ac97_patch.c 2009-05-22 17:26:44.000000000 +0200 +++ linux-2.6.30-rc6.patched/sound/pci/ac97/ac97_patch.c 2009-05-22 17:35:20.000000000 +0200 @@ -958,10 +958,13 @@ static int patch_sigmatel_stac9708_3d(st }
static const struct snd_kcontrol_new snd_ac97_sigmatel_4speaker = -AC97_SINGLE("Sigmatel 4-Speaker Stereo Playback Switch", AC97_SIGMATEL_DAC2INVERT, 2, 1, 0); +AC97_SINGLE("Sigmatel 4-Speaker Stereo Playback Switch", + AC97_SIGMATEL_DAC2INVERT, 2, 1, 0);
+/* "Sigmatel " removed due to excessive name length: */ static const struct snd_kcontrol_new snd_ac97_sigmatel_phaseinvert = -AC97_SINGLE("Sigmatel Surround Phase Inversion Playback Switch", AC97_SIGMATEL_DAC2INVERT, 3, 1, 0); +AC97_SINGLE("Surround Phase Inversion Playback Switch", + AC97_SIGMATEL_DAC2INVERT, 3, 1, 0);
static const struct snd_kcontrol_new snd_ac97_sigmatel_controls[] = { AC97_SINGLE("Sigmatel DAC 6dB Attenuate", AC97_SIGMATEL_ANALOG, 1, 1, 0), diff -uprN linux-2.6.30-rc6.orig/sound/pci/ca0106/ca0106_mixer.c linux-2.6.30-rc6.patched/sound/pci/ca0106/ca0106_mixer.c --- linux-2.6.30-rc6.orig/sound/pci/ca0106/ca0106_mixer.c 2009-05-22 17:26:59.000000000 +0200 +++ linux-2.6.30-rc6.patched/sound/pci/ca0106/ca0106_mixer.c 2009-05-22 17:11:57.000000000 +0200 @@ -800,7 +800,7 @@ int __devinit snd_ca0106_mixer(struct sn "Capture Volume", "External Amplifier", "Sigmatel 4-Speaker Stereo Playback Switch", - "Sigmatel Surround Phase Inversion Playback ", + "Surround Phase Inversion Playback Switch", NULL }; static char *ca0106_rename_ctls[] = {
At Fri, 22 May 2009 17:48:58 +0200, Andreas Mohr wrote:
Hi,
ChangeLog:
Avoid ALSA sound/core/control.c:232: Control name 'Sigmatel Surround Phase Inversion Playback Switch' truncated to 'Sigmatel Surround Phase Inversion Playback ' bootup message by omitting weird Sigmatel prefix in this case; also fix up the related ca0106 mixer control removal part by using identical naming there.
Patch runtime-tested on -rc6, then diffed, run through checkpatch.pl and readjusted accordingly.
I guess that this is more or less what one should have done after your very fast reply...
BTW, the "3D Control Sigmatel - Depth" vs. "3D Control - Switch" is even weirder naming, methinks.
Thanks!!
Signed-off-by: Andreas Mohr andi@lisas.de
Thanks, applied now.
Takashi
participants (2)
-
Andreas Mohr
-
Takashi Iwai