Re: [PATCH 03/36] ALSA: rawmidi: UMP support

[Takashi Iwai <tiwai@suse.de>]

On Mon, 22 May 2023 10:08:24 +0200,
Jaroslav Kysela wrote:
> 
> On 22. 05. 23 9:21, Takashi Iwai wrote:
> > On Mon, 22 May 2023 08:34:20 +0200,
> > Jaroslav Kysela wrote:
> >> 
> >> On 19. 05. 23 11:30, Takashi Iwai wrote:
> >>> This patch adds the support helpers for UMP (Universal MIDI Packet) in
> >>> ALSA core.
> >>> 
> >>> The basic design is that a rawmidi instance is assigned to each UMP
> >>> Endpoint.  A UMP Endpoint provides a UMP stream, typically
> >>> bidirectional (but can be also uni-directional, too), which may hold
> >>> up to 16 UMP Groups, where each UMP (input/output) Group corresponds
> >>> to the traditional MIDI I/O Endpoint.
> >>> 
> >>> Additionally, the ALSA UMP abstraction provides the multiple UMP
> >>> Blocks that can be assigned to each UMP Endpoint.  A UMP Block is a
> >>> metadata to hold the UMP Group clusters, and can represent the
> >>> functions assigned to each UMP Group.  A typical implementation of UMP
> >>> Block is the Group Terminal Blocks of USB MIDI 2.0 specification.
> >>> 
> >>> For distinguishing from the legacy byte-stream MIDI device, a new
> >>> device "umpC*D*" will be created, instead of the standard (MIDI 1.0)
> >>> devices "midiC*D*".  The UMP instance can be identified by the new
> >>> rawmidi info bit SNDRV_RAWMIDI_INFO_UMP, too.
> >>> 
> >>> A UMP rawmidi device reads/writes only in 4-bytes words alignment,
> >>> stored in CPU native endianness.
> >>> 
> >>> The transmit and receive functions take care of the input/out data
> >>> alignment, and may return zero or aligned size, and the params ioctl
> >>> may return -EINVAL when the given input/output buffer size isn't
> >>> aligned.
> >>> 
> >>> A few new UMP-specific ioctls are added for obtaining the new UMP
> >>> endpoint and block information.
> >>> 
> >>> As of this commit, no ALSA sequencer instance is attached to UMP
> >>> devices yet.  They will be supported by later patches.
> >>> 
> >>> Along with those changes, the protocol version for rawmidi is bumped
> >>> to 2.0.3.
> >>> 
> >>> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> >> 
> >> Reviewed-by: Jaroslav Kysela
> >> 
> >> except:
> >> 
> >>> +/* UMP Endpoint information */
> >>> +struct snd_ump_endpoint_info {
> >>> +	int card;			/* card number */
> >>> +	int device;			/* device number */
> >> 
> >> I suspect that those two fields were added to enumerate devices in the
> >> control API. But this extension seems to be missing in your
> >> patches. There is only SNDRV_CTL_IOCTL_UMP_NEXT_DEVICE
> >> implemented. Otherwise those two fields are not useful.
> > 
> > The SNDRV_CTL_IOCTL_UMP_NEXT_DEVICE is looping over rawmidi, and
> > snd_rawmidi_info is provided for (kernel) UMP implementation.
> 
> Right. My point was that an application may be able to evaluate the
> other UMP specific information from those new structures before the
> rawmidi device is opened. So the CTL API extension may make sense.

Point taken, and indeed it might make more sense to change the ioctl
for looking at snd_ump_endpoint_info.  Will try to cook with it.

> > I took over those fields from snd_rawmidi_info, and they are supposed
> > to be referred rather from sequencer clients/ports (as well as from
> > the UMP rawmidi).  Then it could be useful when a user-space sequencer
> > client implements the UMP in future, and distinguish its identity.
> > 
> > But, it's no big deal to drop those, too.
> 
> Ok, keep them. Although this information seems to be redundant, it's
> harmless for now.

OK.


Takashi