Date: Thu, 17 Feb 2011 22:12:16 +0100 From: Raoul Mégélas <rmgls@free.fr> To: freebsd-multimedia@freebsd.org Subject: USB MIDI patch for FreeBSD 8/9 Message-ID: <20110217211213.F1DC8A6284@smtp3-g21.free.fr>
next in thread | raw e-mail | index | archive | help
on Thu, 17 Feb 2011 11:18:40 +0100 Hans Petter Selasky <hselasky@c2i.net> wrote: ... > cd /usr/src/sys/dev/sound/usb/ > cat umidi_patch_001.txt | patch > > A short summary of what the changes are about: > > - Make the USB MIDI driver more OSS compliant by implementing the > correct data format for the /dev/umidiX.Y character devices. Also > implement some missing OSS MIDI ioctls. > - Use the USB stack's builtin clear-stall feature. > - Wrap some long lines. > - Use memcpy() instead of bcopy(). > > --HPS > > Some demo tunes: > > fluidsynth + zynaddsubfx + audacity + midipp: > > http://www.selasky.org/hans_petter/synth_test_new.mp3 > > zynaddsubfx + midipp: > > http://www.selasky.org/hans_petter/synth_test.mp3 > > --HPS > > --Boundary-00=_AYPXNgVCRizMd8n > Content-Type: text/plain; > charset="us-ascii"; > name="umidi_patch_001.txt" > Content-Transfer-Encoding: 7bit > Content-Disposition: inline; > filename="umidi_patch_001.txt" > > === uaudio.c > ================================================================== > --- uaudio.c (revision 218769) > +++ uaudio.c (local) > @@ -191,10 +191,15 @@ > uint8_t iface_alt_index; > }; > > -#define UMIDI_N_TRANSFER 4 /* units */ > #define UMIDI_CABLES_MAX 16 /* units */ > #define UMIDI_BULK_SIZE 1024 /* bytes */ > > +enum { > + UMIDI_TX_TRANSFER, > + UMIDI_RX_TRANSFER, > + UMIDI_N_TRANSFER, > +}; > + > struct umidi_sub_chan { > struct usb_fifo_sc fifo; > uint8_t *temp_cmd; > @@ -224,10 +229,6 @@ > uint8_t iface_index; > uint8_t iface_alt_index; > > - uint8_t flags; > -#define UMIDI_FLAG_READ_STALL 0x01 > -#define UMIDI_FLAG_WRITE_STALL 0x02 > - > uint8_t read_open_refcount; > uint8_t write_open_refcount; > > @@ -336,9 +337,7 @@ > static usb_callback_t uaudio_chan_play_callback; > static usb_callback_t uaudio_chan_record_callback; > static usb_callback_t uaudio_mixer_write_cfg_callback; > -static usb_callback_t umidi_read_clear_stall_callback; > static usb_callback_t umidi_bulk_read_callback; > -static usb_callback_t umidi_write_clear_stall_callback; > static usb_callback_t umidi_bulk_write_callback; > > static void uaudio_chan_fill_info_sub(struct uaudio_softc *, > @@ -493,7 +492,7 @@ > > static const struct usb_config > umidi_config[UMIDI_N_TRANSFER] = { > - [0] = { > + [UMIDI_TX_TRANSFER] = { > .type = UE_BULK, > .endpoint = UE_ADDR_ANY, > .direction = UE_DIR_OUT, > @@ -502,7 +501,7 @@ > .callback = &umidi_bulk_write_callback, > }, > > - [1] = { > + [UMIDI_RX_TRANSFER] = { > .type = UE_BULK, > .endpoint = UE_ADDR_ANY, > .direction = UE_DIR_IN, > @@ -510,26 +509,6 @@ > .flags = {.pipe_bof = 1,.short_xfer_ok = 1,.proxy_buffer = 1,}, > .callback = &umidi_bulk_read_callback, > }, > - > - [2] = { > - .type = UE_CONTROL, > - .endpoint = 0x00, /* Control pipe */ > - .direction = UE_DIR_ANY, > - .bufsize = sizeof(struct usb_device_request), > - .callback = &umidi_write_clear_stall_callback, > - .timeout = 1000, /* 1 second */ > - .interval = 50, /* 50ms */ > - }, > - > - [3] = { > - .type = UE_CONTROL, > - .endpoint = 0x00, /* Control pipe */ > - .direction = UE_DIR_ANY, > - .bufsize = sizeof(struct usb_device_request), > - .callback = &umidi_read_clear_stall_callback, > - .timeout = 1000, /* 1 second */ > - .interval = 50, /* 50ms */ > - }, > }; > > static devclass_t uaudio_devclass; > @@ -1577,10 +1556,10 @@ > uaudio_mixer_add_ctl_sub(struct uaudio_softc *sc, struct uaudio_mixer_node *mc) > { > struct uaudio_mixer_node *p_mc_new = > - malloc(sizeof(*p_mc_new), M_USBDEV, M_WAITOK); > + malloc(sizeof(*p_mc_new), M_USBDEV, M_WAITOK); > > - if (p_mc_new) { > - bcopy(mc, p_mc_new, sizeof(*p_mc_new)); > + if (p_mc_new != NULL) { > + memcpy(p_mc_new, mc, sizeof(*p_mc_new)); > p_mc_new->next = sc->sc_mixer_root; > sc->sc_mixer_root = p_mc_new; > sc->sc_mixer_count++; > @@ -1722,7 +1701,7 @@ > > DPRINTFN(3, "ichs=%d ochs=%d\n", ichs, ochs); > > - bzero(&mix, sizeof(mix)); > + memset(&mix, 0, sizeof(mix)); > > mix.wIndex = MAKE_WORD(d0->bUnitId, sc->sc_mixer_iface_no); > uaudio_mixer_determine_class(&iot[id], &mix); > @@ -1782,7 +1761,7 @@ > if (d->bNrInPins == 0) { > return; > } > - bzero(&mix, sizeof(mix)); > + memset(&mix, 0, sizeof(mix)); > > mix.wIndex = MAKE_WORD(d->bUnitId, sc->sc_mixer_iface_no); > mix.wValue[0] = MAKE_WORD(0, 0); > @@ -1852,7 +1831,7 @@ > if (d->bControlSize == 0) { > return; > } > - bzero(&mix, sizeof(mix)); > + memset(&mix, 0, sizeof(mix)); > > nchan = (d->bLength - 7) / d->bControlSize; > mmask = uaudio_mixer_feature_get_bmaControls(d, 0); > @@ -1986,7 +1965,7 @@ > DPRINTF("no mode select\n"); > return; > } > - bzero(&mix, sizeof(mix)); > + memset(&mix, 0, sizeof(mix)); > > mix.wIndex = MAKE_WORD(d0->bUnitId, sc->sc_mixer_iface_no); > mix.nchan = 1; > @@ -2012,7 +1991,7 @@ > struct uaudio_mixer_node mix; > uint16_t ptype; > > - bzero(&mix, sizeof(mix)); > + memset(&mix, 0, sizeof(mix)); > > ptype = UGETW(d0->wProcessType); > > @@ -2067,7 +2046,7 @@ > } > if (d1->bmControls[0] & UA_EXT_ENABLE_MASK) { > > - bzero(&mix, sizeof(mix)); > + memset(&mix, 0, sizeof(mix)); > > mix.wIndex = MAKE_WORD(d0->bUnitId, sc->sc_mixer_iface_no); > mix.nchan = 1; > @@ -2294,7 +2273,7 @@ > } > error: > DPRINTF("bad data\n"); > - bzero(&r, sizeof(r)); > + memset(&r, 0, sizeof(r)); > done: > return (r); > } > @@ -3284,25 +3263,12 @@ > *========================================================================*/ > > static void > -umidi_read_clear_stall_callback(struct usb_xfer *xfer, usb_error_t error) > -{ > - struct umidi_chan *chan = usbd_xfer_softc(xfer); > - struct usb_xfer *xfer_other = chan->xfer[1]; > - > - if (usbd_clear_stall_callback(xfer, xfer_other)) { > - DPRINTF("stall cleared\n"); > - chan->flags &= ~UMIDI_FLAG_READ_STALL; > - usbd_transfer_start(xfer_other); > - } > -} > - > -static void > umidi_bulk_read_callback(struct usb_xfer *xfer, usb_error_t error) > { > struct umidi_chan *chan = usbd_xfer_softc(xfer); > struct umidi_sub_chan *sub; > struct usb_page_cache *pc; > - uint8_t buf[1]; > + uint8_t buf[5]; > uint8_t cmd_len; > uint8_t cn; > uint16_t pos; > @@ -3320,60 +3286,66 @@ > > while (actlen >= 4) { > > - usbd_copy_out(pc, pos, buf, 1); > - > - cmd_len = umidi_cmd_to_len[buf[0] & 0xF]; /* command length */ > - cn = buf[0] >> 4; /* cable number */ > + /* copy out the MIDI data */ > + usbd_copy_out(pc, pos, buf, 4); > + /* command length */ > + cmd_len = umidi_cmd_to_len[buf[0] & 0xF]; > + /* cable number */ > + cn = buf[0] >> 4; > + /* > + * Lookup sub-channel. The index is range > + * checked below. > + */ > sub = &chan->sub[cn]; > > - if (cmd_len && (cn < chan->max_cable) && sub->read_open) { > - usb_fifo_put_data(sub->fifo.fp[USB_FIFO_RX], pc, > - pos + 1, cmd_len, 1); > - } else { > - /* ignore the command */ > - } > + if ((cmd_len != 0) && > + (cn < chan->max_cable) && > + (sub->read_open != 0)) { > > + /* re-pack into a 4-byte array */ > + /* zero unused bytes */ > + if (cmd_len == 1) { > + buf[2] = 0; > + buf[3] = 0; > + buf[4] = 0; > + } else if (cmd_len == 2) { > + buf[3] = 0; > + buf[4] = 0; > + } else { > + buf[4] = 0; > + } > + > + /* > + * Send data in 4-byte chunks to the > + * application: > + */ > + usb_fifo_put_data_linear( > + sub->fifo.fp[USB_FIFO_RX], > + buf + 1, 4, 1); > + } > actlen -= 4; > pos += 4; > } > > case USB_ST_SETUP: > DPRINTF("start\n"); > - > - if (chan->flags & UMIDI_FLAG_READ_STALL) { > - usbd_transfer_start(chan->xfer[3]); > - return; > - } > +tr_setup: > usbd_xfer_set_frame_len(xfer, 0, usbd_xfer_max_len(xfer)); > usbd_transfer_submit(xfer); > - return; > + break; > > default: > DPRINTF("error=%s\n", usbd_errstr(error)); > > if (error != USB_ERR_CANCELLED) { > /* try to clear stall first */ > - chan->flags |= UMIDI_FLAG_READ_STALL; > - usbd_transfer_start(chan->xfer[3]); > + usbd_xfer_set_stall(xfer); > + goto tr_setup; > } > - return; > - > + break; > } > } > > -static void > -umidi_write_clear_stall_callback(struct usb_xfer *xfer, usb_error_t error) > -{ > - struct umidi_chan *chan = usbd_xfer_softc(xfer); > - struct usb_xfer *xfer_other = chan->xfer[0]; > - > - if (usbd_clear_stall_callback(xfer, xfer_other)) { > - DPRINTF("stall cleared\n"); > - chan->flags &= ~UMIDI_FLAG_WRITE_STALL; > - usbd_transfer_start(xfer_other); > - } > -} > - > /* > * The following statemachine, that converts MIDI commands to > * USB MIDI packets, derives from Linux's usbmidi.c, which > @@ -3502,6 +3474,8 @@ > sub->temp_cmd = sub->temp_1; > sub->state = UMIDI_ST_SYSEX_0; > return (1); > + default: > + break; > } > } > return (0); > @@ -3527,13 +3501,9 @@ > DPRINTF("actlen=%d bytes\n", len); > > case USB_ST_SETUP: > - > +tr_setup: > DPRINTF("start\n"); > > - if (chan->flags & UMIDI_FLAG_WRITE_STALL) { > - usbd_transfer_start(chan->xfer[2]); > - return; > - } > total_length = 0; /* reset */ > start_cable = chan->curr_cable; > tr_any = 0; > @@ -3593,7 +3563,7 @@ > usbd_xfer_set_frame_len(xfer, 0, total_length); > usbd_transfer_submit(xfer); > } > - return; > + break; > > default: /* Error */ > > @@ -3601,11 +3571,10 @@ > > if (error != USB_ERR_CANCELLED) { > /* try to clear stall first */ > - chan->flags |= UMIDI_FLAG_WRITE_STALL; > - usbd_transfer_start(chan->xfer[2]); > + usbd_xfer_set_stall(xfer); > + goto tr_setup; > } > - return; > - > + break; > } > } > > @@ -3635,7 +3604,7 @@ > { > struct umidi_chan *chan = usb_fifo_softc(fifo); > > - usbd_transfer_start(chan->xfer[1]); > + usbd_transfer_start(chan->xfer[UMIDI_RX_TRANSFER]); > } > > static void > @@ -3662,7 +3631,7 @@ > { > struct umidi_chan *chan = usb_fifo_softc(fifo); > > - usbd_transfer_start(chan->xfer[0]); > + usbd_transfer_start(chan->xfer[UMIDI_TX_TRANSFER]); > } > > static void > @@ -3677,8 +3646,7 @@ > > if (--(chan->write_open_refcount) == 0) { > DPRINTF("(stopping write transfer)\n"); > - usbd_transfer_stop(chan->xfer[2]); > - usbd_transfer_stop(chan->xfer[0]); > + usbd_transfer_stop(chan->xfer[UMIDI_TX_TRANSFER]); > } > } > > @@ -3703,7 +3671,7 @@ > } > /* clear stall first */ > mtx_lock(&chan->mtx); > - chan->flags |= UMIDI_FLAG_WRITE_STALL; > + usbd_xfer_set_stall(chan->xfer[UMIDI_TX_TRANSFER]); > chan->write_open_refcount++; > sub->write_open = 1; > > @@ -3769,7 +3737,8 @@ > DPRINTF("setting of alternate index failed!\n"); > goto detach; > } > - usbd_set_parent_iface(sc->sc_udev, chan->iface_index, sc->sc_mixer_iface_index); > + usbd_set_parent_iface(sc->sc_udev, chan->iface_index, > + sc->sc_mixer_iface_index); > > error = usbd_transfer_setup(uaa->device, &chan->iface_index, > chan->xfer, umidi_config, UMIDI_N_TRANSFER, > @@ -3799,13 +3768,15 @@ > mtx_lock(&chan->mtx); > > /* clear stall first */ > - chan->flags |= UMIDI_FLAG_READ_STALL; > + usbd_xfer_set_stall(chan->xfer[UMIDI_RX_TRANSFER]); > > /* > - * NOTE: at least one device will not work properly unless > - * the BULK pipe is open all the time. > + * NOTE: At least one device will not work properly unless the > + * BULK IN pipe is open all the time. This might have to do > + * about that the internal queues of the device overflow if we > + * don't read them regularly. > */ > - usbd_transfer_start(chan->xfer[1]); > + usbd_transfer_start(chan->xfer[UMIDI_RX_TRANSFER]); > > mtx_unlock(&chan->mtx); > > @@ -3828,8 +3799,7 @@ > > mtx_lock(&chan->mtx); > > - usbd_transfer_stop(chan->xfer[3]); > - usbd_transfer_stop(chan->xfer[1]); > + usbd_transfer_stop(chan->xfer[UMIDI_RX_TRANSFER]); > > mtx_unlock(&chan->mtx); Hi Hans Peter, On my laptop The first patch coredump, but not the second. without the patch, my little soft runs fine, i.e. using the mutexes part of code from ssynth. With the second patch: in the same conditions of testing, all notes stick, and make some jam. It seems that the performances are degraded here. what i do is simple: the master keyboard sends the notes, the soft does some transformations and resends the notes to the two modules. Do you need something else? feel free to ask, of course. Best Regards Raoul rmgls@free.fr
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110217211213.F1DC8A6284>