Re: [PATCH 24/36] ALSA: seq: Add UMP support

[Takashi Iwai <tiwai@suse.de>]

On Mon, 22 May 2023 10:00:40 +0200,
Takashi Iwai wrote:
> 
> On Mon, 22 May 2023 09:34:45 +0200,
> Jaroslav Kysela wrote:
> > 
> > On 19. 05. 23 11:31, Takashi Iwai wrote:
> > > Starting from this commit, we add the basic support of UMP (Universal
> > > MIDI Packet) events on ALSA sequencer infrastructure.  The biggest
> > > change here is that, for transferring UMP packets that are up to 128
> > > bits, we extend the data payload of ALSA sequencer event record when
> > > the client is declared to support for the new UMP events.
> > > 
> > > A new event flag bit, SNDRV_SEQ_EVENT_UMP, is defined and it shall be
> > > set for the UMP packet events that have the larger payload of 128
> > > bits, defined as struct snd_seq_ump_event.
> > > 
> > > For applications that want to access the UMP packets, first of all, a
> > > sequencer client has to declare the user-protocol to match with the
> > > latest one via the new SNDRV_SEQ_IOCTL_USER_PVERSION; otherwise it's
> > > treated as if a legacy client without UMP support.
> > > 
> > > Then the client can switch to the new UMP mode (MIDI 1.0 or MIDI 2.0)
> > > with a new field, midi_version, in snd_seq_client_info.  When switched
> > > to UMP mode (midi_version = 1 or 2), the client can write the UMP
> > > events with SNDRV_SEQ_EVENT_UMP flag.  For reads, the alignment size
> > > is changed from snd_seq_event (28 bytes) to snd_seq_ump_event (32
> > > bytes).  When a UMP sequencer event is delivered to a legacy sequencer
> > > client, it's ignored or handled as an error.
> > 
> > The internal struct snd_seq_event_cell is also extended by 4 bytes. I
> > think that it is worth to note this in this commit (the memory
> > footprint is slightly growing).
> 
> Yes, will do that.  But note that the size growth of
> snd_seq_event_cell happens only for 32bit archs (there was already
> padding on 64bit archs).
> 
> > Maybe handle !SND_UMP here, but we can
> > wait, if someone really requires this optimization.
> 
> Right, I began with that, but I threw it away in the end as it becomes
> unnecessarily complex.

Actually the optimization with CONFIG_SND_SEQ_UMP was already done;
unless CONFIG_SND_SEQ_UMP is set, the old size of snd_seq_event_cell
is still kept.  The definition of union __snd_seq_event in
seq_memory.c takes care of the conditional builds.

What I meant was to adjust the cell size dynamically depending on the
supported midi_version, and this will make things too flaky.


Takashi