Bug 192755 - snd_uaudio incorrect handling of version 1 audio format type descriptor
Summary: snd_uaudio incorrect handling of version 1 audio format type descriptor
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: usb (show other bugs)
Version: 10.0-STABLE
Hardware: amd64 Any
: --- Affects Only Me
Assignee: freebsd-usb (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-17 17:24 UTC by juanmasf2
Modified: 2014-08-27 14:25 UTC (History)
2 users (show)

See Also:


Attachments
USB audio patch (1.13 KB, patch)
2014-08-17 20:57 UTC, Hans Petter Selasky
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description juanmasf2 2014-08-17 17:24:56 UTC
For USB audio 1.0 formats the code in uaudio.c uses the value of bBitResolution from the descriptor directly to choose and internal audio format from the array uaudio10_formats. It should use the value from bSubFrameSize instead.
It is clearly specified in the official USB document for audio data formats, page 9 under the section "2.2.2 Audio Subframe" that the endpoint must be handled using bSubFramSize*8 bits per sample from which the device will take the number of bits it actually uses.
In my particular case I own a device reporting bBitResolution of 24 and bSubFrameSize of 4. An unpatched driver will be sending 3-byte subframes to the endpoint resulting in pure noise :)

For USB 2.0 formats the code already does some conversion to avoid this same problem, using the equivalent bSubslotSize in the function uaudio_chan_fill_info_sub:

                 /* Map 4-byte aligned 24-bit samples into 32-bit */
                        if (bBitResolution == 24 && bSubslotSize == 4)
                                bBitResolution = 32;
        
                        if (bBitResolution != (bSubslotSize * 8)) {
                                DPRINTF("Invalid bSubslotSize\n");
                                goto next_ep;
                        }
Comment 1 Hans Petter Selasky freebsd_committer freebsd_triage 2014-08-17 20:57:00 UTC
Created attachment 145942 [details]
USB audio patch

Can you test the attached patch?

--HPS
Comment 2 juanmasf2 2014-08-18 08:51:21 UTC
After the patch the format seems to be correct, showing the following output in dmesg:
uaudio0: <Roland SonicCell, rev 2.00/0.01, addr 2> on usbus7
uaudio0: Play: 44100 Hz, 2 ch, 32-bit S-LE PCM format, 2x8ms buffer.
uaudio0: Record: 44100 Hz, 2 ch, 32-bit S-LE PCM format, 2x8ms buffer.
uaudio0: MIDI sequencer.

Unfortunately this card also needs other code changes and quirks to work, but I hope your patch can help other people with devices using non-typical bit resolutions.
Comment 3 commit-hook freebsd_committer freebsd_triage 2014-08-18 14:31:14 UTC
A commit references this bug:

Author: hselasky
Date: Mon Aug 18 14:30:44 UTC 2014
New revision: 270134
URL: http://svnweb.freebsd.org/changeset/base/270134

Log:
  Use the "bSubslotSize" and "bSubFrameSize" fields to obtain the actual
  sample size. According to the USB audio frame format specification
  from USB.org, the value in the "bBitResolution" field can be less than
  the actual sample size, depending on the actual hardware, and should
  not be used for this computation.

  PR:		192755
  MFC after:	1 week

Changes:
  head/sys/dev/sound/usb/uaudio.c
Comment 4 Hans Petter Selasky freebsd_committer freebsd_triage 2014-08-18 14:32:00 UTC
Patch will be MFC'ed to 10-stable shortly.

Thank you!

--HPS
Comment 5 commit-hook freebsd_committer freebsd_triage 2014-08-27 14:23:22 UTC
A commit references this bug:

Author: hselasky
Date: Wed Aug 27 14:22:40 UTC 2014
New revision: 270717
URL: http://svnweb.freebsd.org/changeset/base/270717

Log:
  MFC r270134:
  Use the "bSubslotSize" and "bSubFrameSize" fields to obtain the actual
  sample size. According to the USB audio frame format specification
  from USB.org, the value in the "bBitResolution" field can be less than
  the actual sample size, depending on the actual hardware, and should
  not be used for this computation.

  PR:		192755

Changes:
_U  stable/10/
  stable/10/sys/dev/sound/usb/uaudio.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2014-08-27 14:24:26 UTC
A commit references this bug:

Author: hselasky
Date: Wed Aug 27 14:24:01 UTC 2014
New revision: 270718
URL: http://svnweb.freebsd.org/changeset/base/270718

Log:
  MFC r270134:
  Use the "bSubslotSize" and "bSubFrameSize" fields to obtain the actual
  sample size. According to the USB audio frame format specification
  from USB.org, the value in the "bBitResolution" field can be less than
  the actual sample size, depending on the actual hardware, and should
  not be used for this computation.

  PR:		192755

Changes:
_U  stable/9/sys/
_U  stable/9/sys/dev/
  stable/9/sys/dev/sound/usb/uaudio.c
Comment 7 commit-hook freebsd_committer freebsd_triage 2014-08-27 14:25:27 UTC
A commit references this bug:

Author: hselasky
Date: Wed Aug 27 14:25:18 UTC 2014
New revision: 270719
URL: http://svnweb.freebsd.org/changeset/base/270719

Log:
  MFC r270134:
  Use the "bSubslotSize" and "bSubFrameSize" fields to obtain the actual
  sample size. According to the USB audio frame format specification
  from USB.org, the value in the "bBitResolution" field can be less than
  the actual sample size, depending on the actual hardware, and should
  not be used for this computation.

  PR:		192755

Changes:
_U  stable/8/sys/
_U  stable/8/sys/dev/
_U  stable/8/sys/dev/sound/
_U  stable/8/sys/dev/sound/usb/
  stable/8/sys/dev/sound/usb/uaudio.c