| Message ID | 1462568920-19891-1-git-send-email-joerg.krause@embedded.rocks (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Jörg Krause wrote: > For the softvol plugin the TLV data are written wrong, e.g. for the default values > of min_dB = -51 dB and max_dB = 0 dB, alsactl generates the following state: > > tlv '00000001000000080000000000000014' > > As both min_dB and max_dB can be negative numbers, the tlv type must not be > unsigned. TLVs always are unsigned. But they can contain bit patterns that are to be interpreted as signed. > - unsigned int tlv[4]; > + int tlv[4]; > tlv[0] = SND_CTL_TLVT_DB_SCALE; > tlv[1] = 2 * sizeof(int); > tlv[2] = svol->min_dB * 100; The problem is that conversion of a negative floating-point number into an unsigned integer results in undefined behaviour, and on your architecture (whatever it is), this results in zero. The correct way to handle this would be something like this: tlv[i] = (int)(...); Regards, Clemens
Hello, On Sa, 2016-05-07 at 09:06 +0200, Clemens Ladisch wrote: > Jörg Krause wrote: > > For the softvol plugin the TLV data are written wrong, e.g. for the > > default values > > of min_dB = -51 dB and max_dB = 0 dB, alsactl generates the > > following state: > > > > tlv '00000001000000080000000000000014' > > > > As both min_dB and max_dB can be negative numbers, the tlv type > > must not be > > unsigned. > > TLVs always are unsigned. But they can contain bit patterns that are > to > be interpreted as signed. I see! > > - unsigned int tlv[4]; > > + int tlv[4]; > > tlv[0] = SND_CTL_TLVT_DB_SCALE; > > tlv[1] = 2 * sizeof(int); > > tlv[2] = svol->min_dB * 100; > > The problem is that conversion of a negative floating-point number > into > an unsigned integer results in undefined behaviour, and on your > architecture (whatever it is), this results in zero. > > The correct way to handle this would be something like this: > > tlv[i] = (int)(...); This fixes the issue for my ARM target, many thanks! I'll send an updated version. Many thanks for the review! Best regards, Jörg Krause
diff --git a/src/pcm/pcm_softvol.c b/src/pcm/pcm_softvol.c index 802aa4b..1748667 100644 --- a/src/pcm/pcm_softvol.c +++ b/src/pcm/pcm_softvol.c @@ -655,7 +655,7 @@ static void snd_pcm_softvol_dump(snd_pcm_t *pcm, snd_output_t *out) static int add_tlv_info(snd_pcm_softvol_t *svol, snd_ctl_elem_info_t *cinfo) { - unsigned int tlv[4]; + int tlv[4]; tlv[0] = SND_CTL_TLVT_DB_SCALE; tlv[1] = 2 * sizeof(int); tlv[2] = svol->min_dB * 100; @@ -768,7 +768,7 @@ static int softvol_load_control(snd_pcm_t *pcm, snd_pcm_softvol_t *svol, } } else if (svol->max_val > 1) { /* check TLV availability */ - unsigned int tlv[4]; + int tlv[4]; err = snd_ctl_elem_tlv_read(svol->ctl, &cinfo->id, tlv, sizeof(tlv)); if (err < 0) add_tlv_info(svol, cinfo);
For the softvol plugin the TLV data are written wrong, e.g. for the default values of min_dB = -51 dB and max_dB = 0 dB, alsactl generates the following state: control.1 { iface MIXER name Master value.0 255 value.1 255 comment { access 'read write user' type INTEGER count 2 range '0 - 255' tlv '00000001000000080000000000000014' dbmin 0 dbmax 5100 dbvalue.0 5100 dbvalue.1 5100 } } As both min_dB and max_dB can be negative numbers, the tlv type must not be unsigned. This fixes the TLV and the correct state is generated: control.1 { iface MIXER name Master value.0 255 value.1 255 comment { access 'read write user' type INTEGER count 2 range '0 - 255' tlv '0000000100000008ffffec1400000014' dbmin -5100 dbmax 0 dbvalue.0 0 dbvalue.1 0 } } Also tested for different combinations of min_dB and max_dB other than the default values. Signed-off-by: Jörg Krause <joerg.krause@embedded.rocks> --- src/pcm/pcm_softvol.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)